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

Yotam Oron yoron at redhat.com
Sun Aug 7 15:23:09 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 75f3a8102d4dad02727ea160f4afb71ae0c9c880
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 d9d8e21..f7ec4cc 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -170,10 +170,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.public_connectStoragePool(spUUID, None, None, None, None, setDefaulException=False)
+                        #TODO Once we support simultaneous connection to multiple pools, remove following line (break)
+                        break
                 except Exception:
                     self.log.error("Unexpected error", exc_info=True)
 
@@ -394,15 +393,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.
@@ -484,7 +474,8 @@ class HSM:
 
         return sp.StoragePool(spUUID).create(poolName, masterDom, domList, masterVersion, safeLease)
 
-    def public_connectStoragePool(self, spUUID, hostID, scsiKey, msdUUID, masterVersion, options = None):
+    def public_connectStoragePool(self, spUUID, hostID, scsiKey, msdUUID, masterVersion, options = None,
+            setDefaulException=True):
         """
         Connect a Host to a specific storage pool.
 
@@ -504,13 +495,13 @@ class HSM:
 
         :raises: :exc:`storage_exception.ConnotConnectMultiplePools` when storage pool is not connected to the system.
         """
-        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))
+        if setDefaulException:
+            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')
         misc.validateUUID(spUUID, 'spUUID')
 
         # TBD: To support multiple pool connection on single host,
@@ -523,25 +514,32 @@ 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()
+                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 8d2eebb..9461de0 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -400,31 +400,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