Change in vdsm[master]: sp: encapsulate spm status in StoragePool

Federico Simoncelli fsimonce at redhat.com
Tue Dec 17 17:13:23 UTC 2013


Federico Simoncelli has uploaded a new change for review.

Change subject: sp: encapsulate spm status in StoragePool
......................................................................

sp: encapsulate spm status in StoragePool

The scope of this patch is to group the spm status values and access
them consistently in StoragePool and HSM.

Change-Id: I3b71349e2a0dfc453b68cd2d9ca6c563b1bee90c
Signed-off-by: Federico Simoncelli <fsimonce at redhat.com>
---
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
2 files changed, 34 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/27/21527/8

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 622065e..975407b 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -616,12 +616,17 @@
         pool = self.getPool(spUUID)
         pool.stopSpm()
 
+    @staticmethod
+    def _getSpmStatusInfo(pool):
+        return dict(
+            zip(('spmStatus', 'spmLver', 'spmId'),
+                (pool.spmRole,) + pool.getSpmStatus()))
+
     @public
     def getSpmStatus(self, spUUID, options=None):
         pool = self.getPool(spUUID)
         try:
-            status = {'spmStatus': pool.spmRole, 'spmLver': pool.getSpmLver(),
-                      'spmId': pool.getSpmId()}
+            status = self._getSpmStatusInfo(pool)
         except (se.LogicalVolumeRefreshError, IOError):
             # This happens when we cannot read the MD LV
             self.log.error("Can't read LV based metadata", exc_info=True)
@@ -3556,9 +3561,7 @@
         pool.invalidateMetadata()
         vars.task.getExclusiveLock(STORAGE, spUUID)
         pool.forceFreeSpm()
-        st = {'spmStatus': pool.spmRole, 'spmLver': pool.getSpmLver(),
-              'spmId': pool.getSpmId()}
-        return dict(spm_st=st)
+        return dict(spm_st=self._getSpmStatusInfo(pool))
 
     @public
     def upgradeStoragePool(self, spUUID, targetDomVersion):
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 77c3016..fdf6d42 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -133,19 +133,35 @@
                                         proxy(self))
 
     @unsecured
-    def getSpmLver(self):
-        return self.getMetaParam(PMDK_LVER)
-
-    @unsecured
     def getSpmStatus(self):
-        return self.spmRole, self.getSpmLver(), self.getSpmId()
+        poolMeta = self._getPoolMD(self.masterDomain)
+
+        # if we claim that we were the SPM (but we're currently not) we
+        # have to make sure that we're not returning stale data
+        if (poolMeta[PMDK_SPM_ID] == self.id
+                and not self.spmRole == SPM_ACQUIRED):
+            self.invalidateMetadata()
+            poolMeta = self._getPoolMD(self.masterDomain)
+
+        return poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID]
+
+    def setSpmStatus(self, lVer=None, spmId=None):
+        self.invalidateMetadata()
+        metaParams = dict(filter(lambda (k, v): v is not None,
+                          ((PMDK_LVER, lVer), (PMDK_SPM_ID, spmId))))
+        # this method must be secured (as it changes the pool metadata),
+        # but since it is also used during the SPM status transition by
+        # default we override the security for setMetaParams.
+        # NOTE: this introduces a race when the method is used in the
+        # secured mode, but generally you shouldn't need to call this at
+        # any time.
+        self.setMetaParams(metaParams, __securityOverride=True)
 
     @unsecured
     def forceFreeSpm(self):
         # DO NOT USE, STUPID, HERE ONLY FOR BC
         # TODO: SCSI Fence the 'lastOwner'
-        self.setMetaParams({PMDK_SPM_ID: SPM_ID_FREE, PMDK_LVER: LVER_INVALID},
-                           __securityOverride=True)
+        self.setSpmStatus(LVER_INVALID, SPM_ID_FREE, __securityOverride=True)
         self.spmRole = SPM_FREE
 
     def _upgradePoolDomain(self, sdUUID, isValid):
@@ -239,9 +255,7 @@
                 raise se.OperationInProgress("spm start %s" % self.spUUID)
 
             self.updateMonitoringThreads()
-            self.invalidateMetadata()
-            oldlver = self.getSpmLver()
-            oldid = self.getSpmId()
+            oldlver, oldid = self.getSpmStatus()
             masterDomVersion = self.getVersion()
             # If no specific domain version was specified use current master
             # domain version
@@ -273,10 +287,7 @@
             try:
                 self.lver = int(oldlver) + 1
 
-                self.invalidateMetadata()
-                self.setMetaParams({
-                    PMDK_LVER: self.lver,
-                    PMDK_SPM_ID: self.id}, __securityOverride=True)
+                self.setSpmStatus(self.lver, self.id, __securityOverride=True)
                 self._maxHostID = maxHostID
 
                 # Upgrade the master domain now if needed
@@ -411,7 +422,7 @@
 
             if not stopFailed:
                 try:
-                    self.setMetaParam(PMDK_SPM_ID, SPM_ID_FREE,
+                    self.setSpmStatus(spmId=SPM_ID_FREE,
                                       __securityOverride=True)
                 except:
                     pass  # The system can handle this inconsistency
@@ -1415,16 +1426,6 @@
     @unsecured
     def getVersion(self):
         return self.masterDomain.getVersion()
-
-    @unsecured
-    def getSpmId(self):
-        spmid = self.getMetaParam(PMDK_SPM_ID)
-        if spmid != self.id or self.spmRole != SPM_FREE:
-            return spmid
-
-        # If we claim to be the SPM we have to be really sure we are
-        self.invalidateMetadata()
-        return self.getMetaParam(PMDK_SPM_ID)
 
     @unsecured
     def getInfo(self):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b71349e2a0dfc453b68cd2d9ca6c563b1bee90c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron 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