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@redhat.com --- 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():
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.
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
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?
Allon Mureinik has posted comments on this change.
Change subject: mutipath: Remove unneeded and dangerous -r parameter ......................................................................
Patch Set 1:
ping?
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
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_ge... : SUCCESS
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@redhat.com Reviewed-on: http://gerrit.ovirt.org/27242 Reviewed-by: Federico Simoncelli fsimonce@redhat.com Reviewed-by: Allon Mureinik amureini@redhat.com --- 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
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
vdsm-patches@lists.fedorahosted.org
-
Allon Mureinik -
Dan Kenigsberg -
Federico Simoncelli -
Jenkins CI RO -
Nir Soffer