Change in vdsm[master]: core: return lver/spm id from pool metadata

Liron Ar has uploaded a new change for review.

Change subject: core: return lver/spm id from pool metadata
......................................................................

core: return lver/spm id from pool metadata

Currently the lver/spm id are being returned as they appear in the
cluster lock if supported.

The fenceSpmStorage verb is currently used to manually confirm that
there's no current spm on the responsibillity of the caller. This verb
updates the lver/spm id in the pool metadata to indicate that the spm
role is free.

This won't help us currently as getSpmStatus will return the info from
the cluster lock (if supported) which wasn't edited at all. As currently
we can't edit the information return from cluster lock, we can return
the same results as in case it wasn't supported by the cluster lock.

Right now the solution is implemented in StoragePoolDiskBackend only as
a bug fix, as StoragePoolMemoryBackend isn't being used at the moment.

Change-Id: I460801329a9a1c5ee940bce22566ad3d29b351de
Signed-off-by: Liron Aravot 
---
M vdsm/storage/spbackends.py
1 file changed, 8 insertions(+), 19 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/26/27226/1

diff --git a/vdsm/storage/spbackends.py b/vdsm/storage/spbackends.py
index d090d5a..86714a3 100644
--- a/vdsm/storage/spbackends.py
+++ b/vdsm/storage/spbackends.py
@@ -26,7 +26,6 @@
 import sd
 import storage_exception as se

-from clusterlock import InquireNotSupportedError
 from persistentDict import DictValidator
 from persistentDict import unicodeDecoder
 from persistentDict import unicodeEncoder
@@ -217,26 +216,16 @@

@unsecured
     def getSpmStatus(self):
-        try:
-            # If the cluster lock implements inquire (e.g. sanlock) then we
-            # can fetch the spmId and the lVer from it.
-            lVer, spmId = self.masterDomain.inquireClusterLock()
-            lVer, spmId = lVer or LVER_INVALID, spmId or SPM_ID_FREE
-        except InquireNotSupportedError:
-            # Legacy implementation for cluster locks that are not able to
-            # return the spmId and the lVer.
+        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)

-            # 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)
-
-            lVer, spmId = poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID]
-
-        return lVer, spmId
+        return poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID]

def setSpmStatus(self, lVer=None, spmId=None):
         self.invalidateMetadata()

Liron Ar has posted comments on this change.

Change subject: core: return lver/spm id from pool metadata
......................................................................

Patch Set 2:

I ran quick verification on the patch (fence scenario) and it seemed fine on that scenario.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: core: return lver/spm id from pool metadata
......................................................................

Patch Set 1:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8411/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7621/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8531/ : FAILURE

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: core: return lver/spm id from pool metadata
......................................................................

Patch Set 2:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8412/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7622/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8532/ : FAILURE

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: core: return lver/spm id from pool metadata
......................................................................

Patch Set 2: Code-Review+1

Looks ok, as this was the previous behavior of the system, and we don't use yet the new memory backend.

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: core: return lver/spm id from pool metadata
......................................................................

Patch Set 2: Code-Review+2

It's sad that for BC we have to maintain this ugliness.

Tal Nisan has posted comments on this change.

Change subject: core: return lver/spm id from pool metadata
......................................................................

Patch Set 2:

In V4 it will alllllllll go away :)

Liron Ar has posted comments on this change.

Change subject: core: return lver/spm id from pool metadata
......................................................................

Patch Set 2: Verified+1

marking as verified as i tested fence scenario and the system recovered.

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: core: return lver/spm id from pool metadata
......................................................................

core: return lver/spm id from pool metadata

Currently the lver/spm id are being returned as they appear in the
cluster lock if supported.

The fenceSpmStorage verb is currently used to manually confirm that
there's no current spm on the responsibillity of the caller. This verb
updates the lver/spm id in the pool metadata to indicate that the spm
role is free.

This won't help us currently as getSpmStatus will return the info from
the cluster lock (if supported) which wasn't edited at all. As currently
we can't edit the information return from cluster lock, we can return
the same results as in case it wasn't supported by the cluster lock.

Right now the solution is implemented in StoragePoolDiskBackend only as
a bug fix, as StoragePoolMemoryBackend isn't being used at the moment.

Change-Id: I460801329a9a1c5ee940bce22566ad3d29b351de
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1092631
Signed-off-by: Liron Aravot 
Reviewed-on: http://gerrit.ovirt.org/27226
Reviewed-by: Nir Soffer 
Reviewed-by: Federico Simoncelli 
---
M vdsm/storage/spbackends.py
1 file changed, 8 insertions(+), 19 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Federico Simoncelli: Looks good to me, approved
  Liron Ar: Verified

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: core: return lver/spm id from pool metadata
......................................................................

Patch Set 3:

Liron, can you please backport this?