Ayal Baron has posted comments on this change.
Change subject: sp: improve masterMigrate safety
......................................................................
Patch Set 1:
(6 comments)
....................................................
File vdsm/storage/sp.py
Line 879: curmsd = sdCache.produce(sdUUID)
Line 880: newmsd = sdCache.produce(msdUUID)
Line 881: self._refreshDomainLinks(newmsd)
Line 882: curmsd.invalidateMetadata()
Line 883: self._convertDomain(newmsd, curmsd.getFormat())
is it ok to call convertDomain before acquiring the lease?
Line 884: self._copyLeaseParameters(curmsd, newmsd)
Line 885:
Line 886: # new 'master' should be in 'active' status
Line 887: domList = self.getDomains()
Line 880: newmsd = sdCache.produce(msdUUID)
Line 881: self._refreshDomainLinks(newmsd)
Line 882: curmsd.invalidateMetadata()
Line 883: self._convertDomain(newmsd, curmsd.getFormat())
Line 884: self._copyLeaseParameters(curmsd, newmsd)
doesn't switch master cover this?
Line 885:
Line 886: # new 'master' should be in 'active' status
Line 887: domList = self.getDomains()
Line 888: if msdUUID not in domList:
Line 893: raise se.StorageDomainNotActive(msdUUID)
Line 894: if newmsd.isISO():
Line 895: raise se.IsoCannotBeMasterDomain(msdUUID)
Line 896: if newmsd.isBackup():
Line 897: raise se.BackupCannotBeMasterDomain(msdUUID)
shouldn't we perform all of these checks *before* alling _convertDomain,
copyLeaseParameters, etc?
Line 898:
Line 899: # If the new master domain is using safelease (version < 3)
then
Line 900: # we can speed up the cluster lock acquirement by resetting
the
Line 901: # SPM lease.
Line 927: src = os.path.join(curmsd.domaindir, sd.MASTER_FS_DIR)
Line 928: dst = os.path.join(newmsd.domaindir, sd.MASTER_FS_DIR)
Line 929: fileUtils.tarCopy(src, dst, exclude=('./lost+found',))
Line 930:
Line 931: if not newmsd.getMDPath():
shouldn't this check be performed before call to mountMaster, together with all
the above validations? (before any change actually)
Line 932: raise se.StorageDomainLayoutError("domain", msdUUID)
Line 933:
Line 934: newmsd.changeRole(sd.MASTER_DOMAIN)
Line 935: self.switchMasterDomain(curmsd, newmsd, masterVersion)
Line 932: raise se.StorageDomainLayoutError("domain", msdUUID)
Line 933:
Line 934: newmsd.changeRole(sd.MASTER_DOMAIN)
Line 935: self.switchMasterDomain(curmsd, newmsd, masterVersion)
Line 936: except Exception:
no finally with release?
Line 937: self.log.exception('migration to new master failed')
Line 938: try:
Line 939: newmsd.unmountMaster()
Line 940: except: # logging only
Line 944: raise
Line 945:
Line 946: # From this point on we have a new master and should not fail
Line 947: try:
Line 948: self.refresh(msdUUID, masterVersion)
move all code within try...except into a separate method? (same thing above)
Line 949: self._saveReconnectInformation(
Line 950: self.id, self.scsiKey, newmsd.sdUUID, masterVersion)
Line 951:
Line 952: # From this point on there is a new master domain in the
pool
--
To view, visit http://gerrit.ovirt.org/22419
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I155b36d41df43230f99ba610699bba05bee3f0c0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
