Change in vdsm[ovirt-3.2]: domain: select the cluster lock using makeClusterLock

fsimonce at redhat.com fsimonce at redhat.com
Mon Jan 28 14:03:15 UTC 2013


Hello Allon Mureinik, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/11464

to review the following change.

Change subject: domain: select the cluster lock using makeClusterLock
......................................................................

domain: select the cluster lock using makeClusterLock

In order to support different locking mechanisms (not only per-domain
format but also per-domain type) a new makeClusterLock method has been
introduced to select the appropriate cluster lock.

Change-Id: I78072254441335a420292af642985840e9b2ac68
Signed-off-by: Federico Simoncelli <fsimonce at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/10281
Reviewed-by: Allon Mureinik <amureini at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/storage/imageRepository/formatConverter.py
M vdsm/storage/sd.py
2 files changed, 39 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/11464/1

diff --git a/vdsm/storage/imageRepository/formatConverter.py b/vdsm/storage/imageRepository/formatConverter.py
index 0742560..95a77d1 100644
--- a/vdsm/storage/imageRepository/formatConverter.py
+++ b/vdsm/storage/imageRepository/formatConverter.py
@@ -26,7 +26,6 @@
 from storage import sd
 from storage import blockSD
 from storage import image
-from storage import clusterlock
 from storage import volume
 from storage import blockVolume
 from storage import storage_exception as se
@@ -91,7 +90,12 @@
 
 def v3DomainConverter(repoPath, hostId, domain, isMsd):
     log = logging.getLogger('Storage.v3DomainConverter')
-    log.debug("Starting conversion for domain %s", domain.sdUUID)
+
+    targetVersion = 3
+    currentVersion = domain.getVersion()
+
+    log.debug("Starting conversion for domain %s from version %s "
+              "to version %s", domain.sdUUID, currentVersion, targetVersion)
 
     targetVersion = 3
     currentVersion = domain.getVersion()
@@ -115,8 +119,7 @@
         domain.setMetadataPermissions()
 
     log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID)
-    newClusterLock = clusterlock.SANLock(
-        domain.sdUUID, domain.getIdsFilePath(), domain.getLeasesFilePath())
+    newClusterLock = domain._makeClusterLock(targetVersion)
     newClusterLock.initLock()
 
     log.debug("Acquiring the host id %s for domain %s", hostId, domain.sdUUID)
diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py
index dbc1beb..a55ce06 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -101,10 +101,6 @@
 DOMAIN_CLASSES = {DATA_DOMAIN: 'Data', ISO_DOMAIN: 'Iso',
                   BACKUP_DOMAIN: 'Backup'}
 
-# Lock Version
-DOM_SAFELEASE_VERS = (0, 2)
-DOM_SANLOCK_VERS = (3,)
-
 # Metadata keys
 DMDK_VERSION = "VERSION"
 DMDK_SDUUID = "SDUUID"
@@ -292,29 +288,20 @@
     mdBackupVersions = config.get('irs', 'md_backup_versions')
     mdBackupDir = config.get('irs', 'md_backup_dir')
 
+    # version: (clusterLockClass, hasVolumeLeases)
+    _clusterLockTable = {
+        0: (clusterlock.SafeLease, False),
+        2: (clusterlock.SafeLease, False),
+        3: (clusterlock.SANLock, True),
+    }
+
     def __init__(self, sdUUID, domaindir, metadata):
         self.sdUUID = sdUUID
         self.domaindir = domaindir
         self._metadata = metadata
         self._lock = threading.Lock()
         self.stat = None
-
-        domversion = self.getVersion()
-
-        if domversion in DOM_SAFELEASE_VERS:
-            leaseParams = (
-                DEFAULT_LEASE_PARAMS[DMDK_LOCK_RENEWAL_INTERVAL_SEC],
-                DEFAULT_LEASE_PARAMS[DMDK_LEASE_TIME_SEC],
-                DEFAULT_LEASE_PARAMS[DMDK_LEASE_RETRIES],
-                DEFAULT_LEASE_PARAMS[DMDK_IO_OP_TIMEOUT_SEC])
-            self._clusterLock = clusterlock.SafeLease(
-                self.sdUUID, self.getIdsFilePath(), self.getLeasesFilePath(),
-                *leaseParams)
-        elif domversion in DOM_SANLOCK_VERS:
-            self._clusterLock = clusterlock.SANLock(
-                self.sdUUID, self.getIdsFilePath(), self.getLeasesFilePath())
-        else:
-            raise se.UnsupportedDomainVersion(domversion)
+        self._clusterLock = self._makeClusterLock()
 
     def __del__(self):
         if self.stat:
@@ -327,6 +314,25 @@
     @property
     def oop(self):
         return oop.getProcessPool(self.sdUUID)
+
+    def _makeClusterLock(self, domVersion=None):
+        if not domVersion:
+            domVersion = self.getVersion()
+
+        leaseParams = (
+            DEFAULT_LEASE_PARAMS[DMDK_LOCK_RENEWAL_INTERVAL_SEC],
+            DEFAULT_LEASE_PARAMS[DMDK_LEASE_TIME_SEC],
+            DEFAULT_LEASE_PARAMS[DMDK_LEASE_RETRIES],
+            DEFAULT_LEASE_PARAMS[DMDK_IO_OP_TIMEOUT_SEC],
+        )
+
+        try:
+            clusterLockClass = self._clusterLockTable[domVersion][0]
+        except KeyError:
+            raise se.UnsupportedDomainVersion(domVersion)
+
+        return clusterLockClass(self.sdUUID, self.getIdsFilePath(),
+                                self.getLeasesFilePath(), *leaseParams)
 
     @classmethod
     def create(cls, sdUUID, domainName, domClass, typeSpecificArg, version):
@@ -436,7 +442,11 @@
         return self._clusterLock.hasHostId(hostId)
 
     def hasVolumeLeases(self):
-        return self.getVersion() in DOM_SANLOCK_VERS
+        domVersion = self.getVersion()
+        try:
+            return self._clusterLockTable[domVersion][1]
+        except KeyError:
+            raise se.UnsupportedDomainVersion(domVersion)
 
     def getVolumeLease(self, volUUID):
         """


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I78072254441335a420292af642985840e9b2ac68
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.2
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list