Change in vdsm[ovirt-3.4]: hsm: move domainStateCallback to HSM

nsoffer at redhat.com nsoffer at redhat.com
Wed May 7 15:53:36 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: hsm: move domainStateCallback to HSM
......................................................................


Patch Set 2:

(2 comments)

Looks good expect thread safety when accessing HSM.pools dict.

http://gerrit.ovirt.org/#/c/27478/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 418:         self.domainStateChangeCallbacks.add(callbackFunc)
Line 419: 
Line 420:     def _onDomainStateChange(self, sdUUID, isValid):
Line 421:         for pool in self.pools.values():
Line 422:             pool.domainStateChangeEvent(sdUUID, isValid)
This happens from an event thread - I'm sure that this is not thread safe as is.

Don't we use locking when accessing the pools dict?
Line 423: 
Line 424:     def _hsmSchedule(self, name, func, *args):
Line 425:         self.taskMng.scheduleJob("hsm", self.tasksDir, vars.task,
Line 426:                                  name, func, *args)


http://gerrit.ovirt.org/#/c/27478/2/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 136:     @unsecured
Line 137:     def getBackend(self):
Line 138:         return self._backend
Line 139: 
Line 140:     def domainStateChangeEvent(self, sdUUID, isValid):
Do we have this pattern - xxxEvent? If we don't I prefer something like "domainStateChanged" or "domainStateDidChange" (if you have time and like this direction).
Line 141:         if isValid and sdUUID in self.getDomains():
Line 142:             self._refreshDomainLinks(sdCache.produce(sdUUID))
Line 143: 
Line 144:     def _upgradePoolDomain(self, sdUUID, isValid):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7509aced432dfa77dbec727d57f507d1a9c550b7
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list