Change in vdsm[master]: sp: Fix stopping domain monitors

fsimonce at redhat.com fsimonce at redhat.com
Thu Dec 5 13:41:49 UTC 2013


Federico Simoncelli has posted comments on this change.

Change subject: sp: Fix stopping domain monitors
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

Code is fine but the commit message should be improved.

....................................................
Commit Message
Line 12: delaying stopping of domain monitors for unlimited time. In case of a memory
Line 13: leak, the pool may never be deleted and domain monitors would never stop.
Line 14: 
Line 15: Fortunately, this fix did seem to work for couple of years, until commit
Line 16: 7b1cc6a20f made the domain monitor pool independent. Instead of one domain
I don't think you should mention 2671777c69. At its time it was maybe overzealous but correct. (Reading these two paragraphs it gives the impression that it was working by accident).
Line 17: monitor object per pool, there is now a single shared domain monitor object
Line 18: used by all pool objects. Having a shared domain monitor, the pool __del__
Line 19: method became deadly.
Line 20: 


-- 
To view, visit http://gerrit.ovirt.org/22058
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad80984ab44dd4a4e36b92330eb9320cf68f1580
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list