Change in vdsm[master]: lib: avoid circular references due to wrapping

nsoffer at redhat.com nsoffer at redhat.com
Tue Jan 12 06:53:01 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: lib: avoid circular references due to wrapping
......................................................................


Patch Set 2:

(4 comments)

https://gerrit.ovirt.org/#/c/51630/2/lib/vdsm/libvirtconnection.py
File lib/vdsm/libvirtconnection.py:

Line 120
Line 121
Line 122
Line 123
Line 124
This wrapping is not correct. Method that return list of virDomain objects (e.g. listAllDomains used v2v) will return unwrapped virDomain. Not related to this patch, just an example of brokenness of this wrapping.


Line 109: 
Line 110: 
Line 111: def _weakmethod(method):
Line 112:     cls = method.im_class
Line 113:     func = method.im_func
You never use this - this must be called when you call the method.
Line 114:     instance_ref = weakref.ref(method.im_self)
Line 115:     del method
Line 116: 
Line 117:     def inner(*args, **kwargs):


Line 111: def _weakmethod(method):
Line 112:     cls = method.im_class
Line 113:     func = method.im_func
Line 114:     instance_ref = weakref.ref(method.im_self)
Line 115:     del method
Can be simpler if we pass the instance instead of the method.

What we want is to create a proxy of the instance, and call class.method(instance, args...)
Line 116: 
Line 117:     def inner(*args, **kwargs):
Line 118:         instance = instance_ref()
Line 119: 


Line 119: 
Line 120:         if instance is None:
Line 121:             raise ValueError("Cannot call weak method with dead instance")
Line 122: 
Line 123:         return func.__get__(instance, cls)(*args, **kwargs)
Are you sure this is correct?
Line 124: 
Line 125:     return inner
Line 126: 
Line 127: 


-- 
To view, visit https://gerrit.ovirt.org/51630
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I06607e63d222bf3f8037f2bda115b48feb94cb1c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list