Change in vdsm[master]: mutipath: Remove unneeded and dangerous -r parameter

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................

mutipath: Remove unneeded and dangerous -r parameter

Since commit dbf2089488 (Jul 9 2013) multipath call was change to use
the -r flag, forcing a reload of the device map. This was tested to fix
a case where new lun is created on the storage server, while a host was
connected, and the new device is not available when issuing the
getDeviceList command. According to a comment on gerrit, the change was
tested for ISCSI and FC storage types, but there is no documentation of
the testing procedure. The related bug was verified, but has no
information about how it was verified.

We have two related bugs:

- Bug 1078879 tell us that invoking multipath with the -r flag sometimes
  triggers a segfault in the multipathd daemon. In the bug, multipath
  developer suggests that as long as multipathd daemon is running,
  there is no need to invoke multipath to detect new devices, and
  "multipath -r really isn't useful for much of anything".

- Bug 1071654 tell us that devices rescanning is broken on FC storage
  domains (although the -r flag is used). I reproduced this bug using
  storage QE FC server.

This patch removes the -r flag. To be on the safe side, I left the
multipath call as it was since the first multipath commit in 2009. We
will work with kernel and multipath developers further on removing this
call if it is indeed unneeded.

Bug-Url: https://bugzilla.redhat.com/1078879
Relates-to: https://bugzilla.redhat.com/1071654
Relates-to: http://gerrit.ovirt.org/17263
Change-Id: I880ab5343df3e0030638901e188320b20570747d
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M vdsm/storage/multipath.py
1 file changed, 1 insertion(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/27242/1

diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py
index 69a758e..ba98866 100644
--- a/vdsm/storage/multipath.py
+++ b/vdsm/storage/multipath.py
@@ -107,8 +107,7 @@
     supervdsm.getProxy().hbaRescan()
 
     # Now let multipath daemon pick up new devices
-    cmd = [constants.EXT_MULTIPATH, "-r"]
-    misc.execCmd(cmd, sudo=True)
+    misc.execCmd([constants.EXT_MULTIPATH], sudo=True)
 
 
 def isEnabled():


-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................


Patch Set 1: Code-Review+2

It seems that indeed running multipath is enough.

-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................


Patch Set 1:

Build Successful 

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

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

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8548/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................


Patch Set 1: Code-Review+1

Nir, can you verify?

-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................


Patch Set 1:

ping?

-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................


Patch Set 1: Verified+1

Verified by Elad while testing http://gerrit.ovirt.org/27122

-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................


Patch Set 2:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9669/ : SUCCESS

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

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

http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs... :
SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................


mutipath: Remove unneeded and dangerous -r parameter

Since commit dbf2089488 (Jul 9 2013) multipath call was change to use
the -r flag, forcing a reload of the device map. This was tested to fix
a case where new lun is created on the storage server, while a host was
connected, and the new device is not available when issuing the
getDeviceList command. According to a comment on gerrit, the change was
tested for ISCSI and FC storage types, but there is no documentation of
the testing procedure. The related bug was verified, but has no
information about how it was verified.

We have two related bugs:

- Bug 1078879 tell us that invoking multipath with the -r flag sometimes
  triggers a segfault in the multipathd daemon. In the bug, multipath
  developer suggests that as long as multipathd daemon is running,
  there is no need to invoke multipath to detect new devices, and
  "multipath -r really isn't useful for much of anything".

- Bug 1071654 tell us that devices rescanning is broken on FC storage
  domains (although the -r flag is used). I reproduced this bug using
  storage QE FC server.

This patch removes the -r flag. To be on the safe side, I left the
multipath call as it was since the first multipath commit in 2009. We
will work with kernel and multipath developers further on removing this
call if it is indeed unneeded.

Bug-Url: https://bugzilla.redhat.com/1078879
Relates-to: https://bugzilla.redhat.com/1071654
Relates-to: http://gerrit.ovirt.org/17263
Change-Id: I880ab5343df3e0030638901e188320b20570747d
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/27242
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Reviewed-by: Allon Mureinik <amureini(a)redhat.com&gt;
---
M vdsm/storage/multipath.py
1 file changed, 1 insertion(+), 2 deletions(-)

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



-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: mutipath: Remove unneeded and dangerous -r parameter
......................................................................


Patch Set 3:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1386/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/27242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I880ab5343df3e0030638901e188320b20570747d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No