Federico Simoncelli has posted comments on this change.
Change subject: Fix attachSD and masterMigrate for domain version 3
......................................................................
Patch Set 9: (2 inline comments)
....................................................
File vdsm/storage/sp.py
Line 859: if not newmsd.hasVolumeLeases():
I don't understand what you're proposing here. Do you want to get rid of
initSPMlease() in all cases?
Line 949: dom.releaseHostId(self.id)
It's out of the scope of this method to maintain the host id. In fact it was acquired
for the sole purpose of temporarily taking the SPM resource to avoid races with other DCs.
Keeping the host acquired here is an optimization that doesn't buy you much in terms
of performance and instead it's adding a possible mismatch if the domainMonitor thread
is not properly started.
I'd keep the code as it is now but if we really want to maintain the host id acquired
here then we might want to make sure to start the domainMonitor as well:
try:
dom.acquireClusterLock(self.id)
try:
[...]
finally:
dom.releaseClusterLock()
self.domainMonitor.startMonitoring(sdCache.produce(sdUUID), self.id)
except:
dom.releaseHostId(self.id)
And get rid of the updateMonitoringThreads() at the end.
--
To view, visit
http://gerrit.ovirt.org/4037
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe26891b2b9d5c4a34876115d49f14725b476f99
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>