Change in vdsm[master]: clusterlock: Remove uneeded workaround

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: clusterlock: Remove uneeded workaround
......................................................................

clusterlock: Remove uneeded workaround

Sanlock version XXXX had a off-by-one bug when calling get_hosts with a
host id, returning info for the next host. This bug is fixed in version
XXX. Now we can use the hostId parameter, making the call more efficient
and simpligying clusterlock code.

Change-Id: Ide75e749fbc2916540c2b526b78fedc247b5c6f9
Signed-off-by: Nir Soffer 
---
M vdsm.spec.in
M vdsm/storage/clusterlock.py
2 files changed, 5 insertions(+), 16 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/62/31162/1

diff --git a/vdsm.spec.in b/vdsm.spec.in
index 5ba6fc6..fc39eb9 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -167,7 +167,7 @@
 Requires: iscsi-initiator-utils >= 6.2.0.873-21
 %endif

-Requires: sanlock >= 2.8, sanlock-python
+Requires: sanlock >= XXX, sanlock-python

%if 0%{?rhel}
 Requires: python-ethtool >= 0.6-3
diff --git a/vdsm/storage/clusterlock.py b/vdsm/storage/clusterlock.py
index 24a5d81..17cdd53 100644
--- a/vdsm/storage/clusterlock.py
+++ b/vdsm/storage/clusterlock.py
@@ -265,26 +265,15 @@
                 return False

def getHostStatus(self, hostId):
-        # Note: get_hosts has off-by-one bug when asking for particular host
-        # id, so get all hosts info and filter.
-        # See https://bugzilla.redhat.com/1111210
         try:
-            hosts = sanlock.get_hosts(self._sdUUID)
+            hosts = sanlock.get_hosts(self._sdUUID, hostId)
         except sanlock.SanlockException as e:
             self.log.debug("Unable to get host %d status in lockspace %s: %s",
                            hostId, self._sdUUID, e)
             return HOST_STATUS_UNAVAILABLE
-
-        for info in hosts:
-            if info['host_id'] == hostId:
-                status = info['flags']
-                return self.STATUS_NAME[status]
-
-        # get_hosts with host_id=0 returns only hosts with timestamp != 0,
-        # which means that no host is using this host id now. If there a
-        # timestamp, sanlock will return HOST_UNKNOWN and then HOST_LIVE or
-        # HOST_FAIL.
-        return HOST_STATUS_FREE
+        else:
+            status = hosts[0]['flags']
+            return self.STATUS_NAME[status]

# The hostId parameter is maintained here only for compatibility with
     # ClusterLock. We could consider to remove it in the future but keeping it

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: clusterlock: Remove uneeded workaround
......................................................................

Patch Set 1:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/212/ : SUCCESS

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

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/202... : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11637/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/234... : FAILURE

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

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: clusterlock: Remove uneeded workaround
......................................................................

Patch Set 1: Code-Review+1

Great. Now we just need to wait for sanlock to be released.

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: clusterlock: Remove uneeded workaround
......................................................................

Patch Set 1: Verified-1 Code-Review+1

+1 on the code, obviously can't be merged without sanlock's fix to BZ#1111210

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[e-mail] has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[e-mail] has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 3:

* Update tracker::#1131192::OK
* Check Bug-Url::OK
* Check Public Bug::#1131192::OK, public bug
* Check Product::#1131192::OK, Correct product oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 3: Verified+1

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 3: Verified-1

Package is not available yet on el6.6. 2.8-1 is available, need to check if this version includes the fix.

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 2:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc21_created/397... : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15541/ : FAILURE

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

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

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/944... : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el7_created/404/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/962/ : FAILURE

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 3:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc21_created/398... : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15543/ : FAILURE

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

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

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/945... : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el7_created/405/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/963/ : FAILURE

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 3:

We don't support el6, so we can just require the sanlock version on el7/fedora.

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[e-mail] has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 4:

* #1131192::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1131192::OK, public bug
* Check Product::#1131192::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Change in vdsm[master]: clusterlock: Remove unneeded workaround

Idan Shaby has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 4: Verified+1

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 4: Code-Review+1 -Verified

Idan, can you describe (in details) how you verified this patch?

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 4: Code-Review+1

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 4: Continuous-Integration+1

unrelated failure

17:26:54 Last metadata expiration check performed 0:00:02 ago on Tue Nov 10 17:26:47 2015.
17:26:54 Error: nothing provides ovirt-vmconsole >= 1.0.0-0 needed by vdsm-4.17.999-118.git71e8041.fc23.noarch.

Change in vdsm[master]: clusterlock: Remove unneeded workaround

Idan Shaby has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 4:

Sure:
I had an environment of three hosts.
I blocked the outgoing packets from one of the hosts (id=3) to the storage server, and watched its status changing from live, to fail, and eventually to dead.
To do that, I used manhole:
> > > irs.getHostLeaseStatus({'2b95e786-1d9f-4567-8133-3fc350d8cea4':3})

{'status': {'message': 'OK', 'code': 0}, 'domains': {'2b95e786-1d9f-4567-8133-3fc350d8cea4': 'live'}}
> > > irs.getHostLeaseStatus({'2b95e786-1d9f-4567-8133-3fc350d8cea4':3})

{'status': {'message': 'OK', 'code': 0}, 'domains': {'2b95e786-1d9f-4567-8133-3fc350d8cea4': 'fail'}}
> > > irs.getHostLeaseStatus({'2b95e786-1d9f-4567-8133-3fc350d8cea4':3})

{'status': {'message': 'OK', 'code': 0}, 'domains': {'2b95e786-1d9f-4567-8133-3fc350d8cea4': 'dead'}}

To verify that only this host's (id=3) status has changed, I ran the command:
> > > sanlock.get_hosts('c8d24dc0-dfa6-4140-a11f-8b943f2dae0d')

And watched its status changing (where the others stayed the same):
[{'generation': 12, 'host_id': 1, 'flags': 3, 'io_timeout': 10, 'timestamp': 452680}, {'generation': 7, 'host_id': 2, 'flags': 3, 'io_timeout': 10, 'timestamp': 11591}, {'generation': 10, 'host_id': 3, 'flags': 3, 'io_timeout': 10, 'timestamp': 430448}]
[{'generation': 12, 'host_id': 1, 'flags': 3, 'io_timeout': 10, 'timestamp': 450199}, {'generation': 7, 'host_id': 2, 'flags': 3, 'io_timeout': 10, 'timestamp': 9101}, {'generation': 9, 'host_id': 3, 'flags': 4, 'io_timeout': 10, 'timestamp': 427850}]

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 4: Code-Review+2

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 4:

Thanks Idan!

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

clusterlock: Remove unneeded workaround

Sanlock 2.8 had a off-by-one bug when calling get_hosts with a host id,
returning info for the next host. This bug is fixed in version 2.8-2.
Now we can use the hostId parameter, making the call more efficient and
simplifying clusterlock code.

Change-Id: Ide75e749fbc2916540c2b526b78fedc247b5c6f9
Bug-Url: https://bugzilla.redhat.com/1131192
Signed-off-by: Nir Soffer 
Reviewed-on: https://gerrit.ovirt.org/31162
Tested-by: Idan Shaby 
Reviewed-by: Allon Mureinik 
Continuous-Integration: Dan Kenigsberg 
---
M vdsm.spec.in
M vdsm/storage/clusterlock.py
2 files changed, 5 insertions(+), 16 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, approved
  Dan Kenigsberg: Passed CI tests
  Allon Mureinik: Looks good to me, but someone else must approve
  Idan Shaby: Verified

Objections:
  Jenkins CI: Failed CI tests

Change in vdsm[master]: clusterlock: Remove unneeded workaround

gerrit-hooks has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 5:

* #1131192::Update tracker: OK
* Set MODIFIED::bug 1131192::::#1131192::::OK

Change in vdsm[master]: clusterlock: Remove unneeded workaround

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: clusterlock: Remove unneeded workaround
......................................................................

Patch Set 5:

Nir/Idan - please backport to the 3.6 branch too.
Thanks!