[NEW PATCH] BZ#705058 - Don't create a new pool object on storage refresh (via gerrit-bot)

Yotam Oron yoron at redhat.com
Wed Aug 31 09:13:14 UTC 2011


New patch submitted by Yotam Oron (yoron at redhat.com)

You can review this change at: http://gerrit.usersys.redhat.com/796

commit 5a513a193f95eb83330503aaab2b6b8baa07bc00
Author: Yotam Oron <yoron at redhat.com>
Date:   Sun Aug 7 18:22:10 2011 +0300

    BZ#705058 - Don't create a new pool object on storage refresh
    
    Use the standard interface for pool connection and avoid double pool creation
    
    Change-Id: I40d86257c239736c74d715775bcc27cab4613125

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index f83316c..a77be87 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -182,10 +182,9 @@ class HSM:
                 poolPath = os.path.join(self.storage_repository, spUUID)
                 try:
                     if os.path.exists(poolPath):
-                        with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive):
-                            self._restorePool(spUUID)
-                            #TODO Once we support simultaneous connection to multiple pools, remove following line (break)
-                            break
+                        self._connectStoragePool(spUUID, None, None, None, None)
+                        #TODO Once we support simultaneous connection to multiple pools, remove following line (break)
+                        break
                 except Exception:
                     self.log.error("Unexpected error", exc_info=True)
 
@@ -406,15 +405,6 @@ class HSM:
             pool.hsmMailer.flushMessages()
 
 
-    def _restorePool(self, spUUID):
-        self.log.info("RESTOREPOOL: %s", spUUID)
-        pool = sp.StoragePool(spUUID)
-        if pool.reconnect():
-            self.pools[spUUID] = pool
-            return True
-        self.log.info("RESTOREPOOL: %s reconnect failed", spUUID)
-
-
     def public_createStoragePool(self, poolType, spUUID, poolName, masterDom, domList, masterVersion, lockPolicy=None, lockRenewalIntervalSec=None, leaseTimeSec=None, ioOpTimeoutSec=None, leaseRetries=None, options = None):
         """
         Create new storage pool with single/multiple image data domain.
@@ -519,10 +509,10 @@ class HSM:
         vars.task.setDefaultException(
             se.StoragePoolConnectionError("spUUID=%s, msdUUID=%s, masterVersion=%s, " \
                                           "hostID=%s, scsiKey=%s" % (str(spUUID), str(msdUUID),
-                                          str(masterVersion), str(hostID), str(scsiKey))
-            )
-        )
-        misc.validateN(hostID, 'hostID')
+                                          str(masterVersion), str(hostID), str(scsiKey))))
+        return self._connectStoragePool(spUUID, hostID, scsiKey, msdUUID, masterVersion, options)
+
+    def _connectStoragePool(self, spUUID, hostID, scsiKey, msdUUID, masterVersion, options=None):
         misc.validateUUID(spUUID, 'spUUID')
 
         # TBD: To support multiple pool connection on single host,
@@ -535,25 +525,33 @@ class HSM:
         except se.StoragePoolUnknown:
             pass #pool not connected yet
         else:
-            vars.task.getSharedLock(STORAGE, spUUID)
-            pool = self.getPool(spUUID)
-            pool.verifyMasterDomain(msdUUID=msdUUID, masterVersion=masterVersion)
-            return
-
-        vars.task.getExclusiveLock(STORAGE, spUUID)
-        try:
-            pool = self.getPool(spUUID)
-        except se.StoragePoolUnknown:
-            pass #pool not connected yet
-        else:
-            pool.verifyMasterDomain(msdUUID=msdUUID, masterVersion=masterVersion)
-            return
+            with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.shared):
+                pool = self.getPool(spUUID)
+                if not msdUUID or not masterVersion:
+                    hostID, scsiKey, msdUUID, masterVersion = pool.getPoolParams()
+                misc.validateN(hostID, 'hostID')
+                pool.verifyMasterDomain(msdUUID=msdUUID, masterVersion=masterVersion)
+                return
+
+        with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive):
+            try:
+                pool = self.getPool(spUUID)
+            except se.StoragePoolUnknown:
+                pass #pool not connected yet
+            else:
+                if not msdUUID or not masterVersion:
+                    hostID, scsiKey, msdUUID, masterVersion = pool.getPoolParams()
+                misc.validateN(hostID, 'hostID')
+                pool.verifyMasterDomain(msdUUID=msdUUID, masterVersion=masterVersion)
+                return
 
-        pool = sp.StoragePool(spUUID)
-        res = pool.connect(hostID, scsiKey, msdUUID, masterVersion)
-        if res:
-            self.pools[spUUID] = pool
-        return res
+            pool = sp.StoragePool(spUUID)
+            if not hostID or not scsiKey or not msdUUID or not masterVersion:
+                hostID, scsiKey, msdUUID, masterVersion = pool.getPoolParams()
+            res = pool.connect(hostID, scsiKey, msdUUID, masterVersion)
+            if res:
+                self.pools[spUUID] = pool
+            return res
 
     def public_disconnectStoragePool(self, spUUID, hostID, scsiKey, remove=False, options = None):
         """
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 4fbf95f..0de7be6 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -410,31 +410,22 @@ class StoragePool:
         return True
 
 
-    def reconnect(self):
-        self.log.info("Trying to reconnect to pool: %s" % self.spUUID)
-        try:
-            file = open(self._poolFile, "r")
-            for line in file:
-                pair = line.strip().split("=")
-                if len(pair) == 2:
-                    if pair[0] == "id":
-                        hostId = int(pair[1])
-                    elif pair[0] == "scsiKey":
-                        scsiKey = pair[1]
-                    elif pair[0] == "sdUUID":
-                        msdUUID = pair[1]
-                    elif pair[0] == "version":
-                        masterVersion = pair[1]
-            file.close()
-            if not (hostId and scsiKey and msdUUID and masterVersion):
-                os.unlink(self._poolFile)
-                return False
-            if self.connect(hostId, scsiKey, msdUUID, masterVersion):
-                return True
-        except:
-            self.log.error("RECONNECT: Failed: %s", self.spUUID, exc_info=True)
-            os.unlink(self._poolFile)
-            raise
+    def getPoolParams(self):
+        file = open(self._poolFile, "r")
+        for line in file:
+            pair = line.strip().split("=")
+            if len(pair) == 2:
+                if pair[0] == "id":
+                    hostId = int(pair[1])
+                elif pair[0] == "scsiKey":
+                    scsiKey = pair[1]
+                elif pair[0] == "sdUUID":
+                    msdUUID = pair[1]
+                elif pair[0] == "version":
+                    masterVersion = pair[1]
+        file.close()
+
+        return hostId, scsiKey, msdUUID, masterVersion
 
 
     def createMaster(self, poolName, domain, masterVersion, leaseParams):




More information about the vdsm-patches mailing list