Change in vdsm[master]: sp: consolidate getMasterVersion in one method

fsimonce at redhat.com fsimonce at redhat.com
Fri Dec 6 23:09:15 UTC 2013


Federico Simoncelli has uploaded a new change for review.

Change subject: sp: consolidate getMasterVersion in one method
......................................................................

sp: consolidate getMasterVersion in one method

Change-Id: I8a055c9eb21cc5681d1f9afde5e7eab899aa65c0
Signed-off-by: Federico Simoncelli <fsimonce at redhat.com>
---
M vdsm/storage/sp.py
1 file changed, 20 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/22130/1

diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 31e0bcd..f35ea4b 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -488,8 +488,20 @@
         self.refresh(msdUUID, masterVersion)
 
     @unsecured
-    def getMasterVersion(self):
-        return self.getMetaParam(PMDK_MASTER_VER)
+    def getMasterVersion(self, useMasterDomain=None):
+        domain = (self.masterDomain
+                  if useMasterDomain is None else useMasterDomain)
+        return self._getPoolMD(domain)[PMDK_MASTER_VER]
+
+    def setDomainMasterRole(self, domain, role, masterVersion):
+        poolMeta = self._getPoolMD(domain)
+        # NOTE: the transaction here does not ensure the consistency between
+        # the domain and pool metadata. For example if the role on the domain
+        # has been changed and the pool metadata transaction fails then the
+        # domain role is not reverted to the previous value.
+        with poolMeta.transaction():
+            poolMeta[PMDK_MASTER_VER] = masterVersion
+            domain.changeRole(role)
 
     # TODO: Remove or rename this function.
     @unsecured
@@ -511,23 +523,6 @@
             if self.spUUID not in dom.getPools():
                 raise se.StorageDomainNotInPool(self.spUUID, dom.sdUUID)
         return True
-
-    @unsecured
-    def validatePoolMVerHigher(self, masterVersion):
-        """
-        Make sure the masterVersion higher than that of the pool.
-
-        :param masterVersion: the master version you want to validate
-        :type masterVersion: int
-
-        :raises: :exc:`storage_exception.StoragePoolWrongMasterVersion`
-            exception if masterVersion doesn't follow the rules
-
-        """
-        mver = self.getMasterVersion()
-        if not int(masterVersion) > mver:
-            raise se.StoragePoolWrongMaster(self.spUUID,
-                                            self.masterDomain.sdUUID)
 
     @unsecured
     def getMaximumSupportedDomains(self):
@@ -856,7 +851,9 @@
                       msdUUID)
 
         # TODO: is this check still relevant?
-        self.validatePoolMVerHigher(masterVersion)
+        if not masterVersion > self.getMasterVersion():
+            raise se.StoragePoolWrongMaster(self.spUUID,
+                                            self.masterDomain.sdUUID)
 
         curmsd = sdCache.produce(sdUUID)
         newmsd = sdCache.produce(msdUUID)
@@ -929,12 +926,7 @@
         self.log.debug("masterMigrate - lease acquired successfully")
 
         try:
-            # Now mark new domain as 'master'
-            # if things break down here move the master back pronto
-            newPoolMD = self._getPoolMD(newmsd)
-            with newPoolMD.transaction():
-                newPoolMD[PMDK_MASTER_VER] = masterVersion
-                newmsd.changeRole(sd.MASTER_DOMAIN)
+            self.setDomainMasterRole(newmsd, sd.MASTER_DOMAIN, masterVersion)
             self._saveReconnectInformation(self.id, self.scsiKey,
                                            newmsd.sdUUID, masterVersion)
         except Exception:
@@ -1226,10 +1218,7 @@
             masterName = os.path.join(self.poolPath, POOL_MASTER_DOMAIN)
             self._linkStorageDomain(domain.domaindir, masterName)
         else:
-            domPoolMD = self._getPoolMD(domain)
-            with domPoolMD.transaction():
-                domain.changeRole(sd.REGULAR_DOMAIN)
-                domPoolMD[PMDK_MASTER_VER] = 0
+            self.setDomainMasterRole(domain, sd.MASTER_DOMAIN, 0)
 
     @unsecured
     def __rebuild(self, msdUUID, masterVersion):
@@ -1521,7 +1510,7 @@
                            " %s", msdUUID, self.spUUID)
             raise se.StoragePoolWrongMaster(self.spUUID, msdUUID)
 
-        version = self._getPoolMD(domain)[PMDK_MASTER_VER]
+        version = self.getMasterVersion(useMasterDomain=domain)
         if version != int(masterVersion):
             self.log.error("Requested master domain %s does not have expected "
                            "version %s it is version %s",


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8a055c9eb21cc5681d1f9afde5e7eab899aa65c0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list