[NEW PATCH] [WIP] BZ#719330 - No need to spew unmounted masters to the log. (via gerrit-bot)

New patch submitted by Saggi Mizrahi (smizrahi(a)redhat.com)

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

commit d334c99c34d3a922a89ff75280e927235b0984db
Author: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Date:   Mon Jul 11 12:15:48 2011 +0300

    [WIP] BZ#719330 - No need to spew unmounted masters to the log.
    
    Change-Id: Ib0044075f10171bd37613e17b46b28cd7e81dff7

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index b0d5d21..623b09f 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -607,7 +607,7 @@ class HSM:
         for sdUUID in domList:
             vars.task.getExclusiveLock(STORAGE, sdUUID)
 
-        self.spm.detachAllDomains(pool)
+        self.spm.destroyPool(pool)
         return self._disconnectPool(pool, hostID, scsiKey, remove=True)
 
 
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 8aed3e6..e9f5c90 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -595,31 +595,11 @@ class StoragePool:
             self.log.error("Unexpected error", exc_info=True)
 
 
-    def __unmountLastMaster(self, sdUUID):
-        curmsd = SDF.produce(sdUUID)
-        # Check if it's last domain and allow it detaching
-        dl = self.getDomains(activeOnly=True)
-        domList = dl.keys()
-        if curmsd.sdUUID in domList:
-            domList.remove(curmsd.sdUUID)
-        for item in domList:
-            domain = SDF.produce(item)
-            if domain.isData():
-                # Failure, we have at least one more data domain
-                # in the pool and one which can become 'master'
-                raise se.StoragePoolHasPotentialMaster(item)
-        curmsd.unmountMaster()
-
     def masterMigrate(self, sdUUID, msdUUID, masterVersion):
         self.log.info("sdUUID=%s spUUID=%s msdUUID=%s", sdUUID,  self.spUUID,
msdUUID)
 
-        # Check if we are migrating to or just unmounting last master
-        if msdUUID != sd.BLANK_UUID:
-            self.__masterMigrate(sdUUID, msdUUID, masterVersion)
-            return False    # not last master
-
-        self.__unmountLastMaster(sdUUID)
-        return True     # last master
+        self.__masterMigrate(sdUUID, msdUUID, masterVersion)
+        return False    # not last master
 
     def attachSD(self, sdUUID):
         """
@@ -651,6 +631,21 @@ class StoragePool:
             dom.releaseClusterLock()
         self.updateMonitoringThreads()
 
+    def destroy(self):
+        curmsd = self.getMasterDomain()
+        # Check if there is only 1 domain and allow it detaching
+        domList = self.getDomains()
+        if len(domList) > 1:
+            raise se.StoragePoolDestroyingError()
+
+        with self._metadata.transaction():
+            spUUID = self.spUUID
+            self._metadata[PMDK_DOMAINS] = {}
+            self._metadata[PMDK_MASTER_VER] = 0
+            curmsd.detachSD(spUUID)
+
+        curmsd.unmountMaster()
+
 
     def forcedDetachSD(self, sdUUID):
         self.log.warn("Force detaching domain `%s`", sdUUID)
diff --git a/vdsm/storage/spm.py b/vdsm/storage/spm.py
index b186d39..a80d03b 100644
--- a/vdsm/storage/spm.py
+++ b/vdsm/storage/spm.py
@@ -201,8 +201,6 @@ class SPM:
                     blockSD.BlockStorageDomain.doUnmountMaster(master)
                 except se.StorageDomainMasterUnmountError, e:
                     misc.panic("unmount %s failed - %s" % (master, e))
-            else:
-                cls.log.debug("master `%s` is not mounted, skipping", master)
 
     @classmethod
     def __cleanupSPMLinks(cls):
@@ -1070,7 +1068,7 @@ class SPM:
         pool.detachSD(sdUUID, msdUUID, masterVersion)
 
 
-    def detachAllDomains(self, pool):
+    def destroyPool(self, pool):
         """
         Detach all domains from pool before destroying pool
         """
@@ -1086,7 +1084,8 @@ class SPM:
             pool.detachSD(sdUUID=sdUUID, msdUUID=sd.BLANK_UUID, masterVersion=0)
         self._stop(pool.spUUID)
         # Forced detach 'master' domain after stopping SPM
-        pool.detachSD(mDom.sdUUID, sd.BLANK_UUID, 0)
+        pool.destroy()
+        return True     # last master
 
 
     def public_attachStorageDomain(self, sdUUID, spUUID, options = None):

Change in vdsm[master]: [WIP] BZ#719330 - No need to spew unmounted masters to the l...

Saggi Mizrahi has abandoned this change.

Change subject: [WIP] BZ#719330 - No need to spew unmounted masters to the log.
......................................................................


Patch Set 2: Abandoned

Bug is non issue and refactoring can wait

--
To view, visit http://gerrit.usersys.redhat.com/683
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ib0044075f10171bd37613e17b46b28cd7e81dff7
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: [WIP] BZ#719330 - No need to spew unmounted masters to the l...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: [WIP] BZ#719330 - No need to spew unmounted masters to the log.
......................................................................


Patch Set 2: I would prefer that you didn't submit this

(3 inline comments)

....................................................
Commit Message
Line 7: [WIP] BZ#719330 - No need to spew unmounted masters to the log.
Is it still [WIP]?

....................................................
File vdsm/storage/sp.py
Line 601:         self.__masterMigrate(sdUUID, msdUUID, masterVersion)
wouldn't this explode when passed msdUUID=BLANK

Line 642:             spUUID = self.spUUID
I miss the point of this early assignment.

--
To view, visit http://gerrit.usersys.redhat.com/683
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0044075f10171bd37613e17b46b28cd7e81dff7
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: [WIP] BZ#719330 - No need to spew unmounted masters to the l...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: [WIP] BZ#719330 - No need to spew unmounted masters to the log.
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

1-2-3 testing

....................................................
File vdsm/storage/sp.py
Line 601:         self.__masterMigrate(sdUUID, msdUUID, masterVersion)
all these changes are just to clean the logs?

--
To view, visit http://gerrit.usersys.redhat.com/683
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0044075f10171bd37613e17b46b28cd7e81dff7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;