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@redhat.com Gerrit-Reviewer: Allon Mureinik amureini@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com