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

nsoffer at redhat.com nsoffer at redhat.com
Tue Jan 12 09:43:37 UTC 2016


Nir Soffer has posted comments on this change.

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


Patch Set 2:

(2 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
> I know, we need to add our protection code to all virConnect and virDomain 
The problem is fixing all the code checking for libvrit.libvirtError - all this code should be modified to this something like this:

    except libvirt.libvirtError as e:
        if utils.is_unrecoverable(e):
            raise UnrecoverableLibvirtError(e)

And the top level handler can do what should be done (maybe killing vdsm).


Line 107:     with __connectionLock:
Line 108:         __connections.clear()
Line 109: 
Line 110: 
Line 111: def _weakmethod(method):
> will update as you suggested once I'm confident enough this really solves t
It does not matter if this change solve the "leak", the current code is wrong, and we must fix it. This change (with some simplifications and tests) looks like the best way to get a minimal fix that will be easy to backport to 3.5.

If you can get minimal fix using __getattr__ it can be nicer, but I don't have any problem taking this for now, and improving it later.

I would make a simple patch with tests, and let Eldad run the nest test with it.
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


-- 
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: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek 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