Change in vdsm[master]: Configure iscsi_session recovery_tmo for multipath devices

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: Configure iscsi_session recovery_tmo for multipath devices
......................................................................

Configure iscsi_session recovery_tmo for multipath devices

This configuration is done by device-mapper-multipath in version
0.4.9-76 and later. However, device looses configuration after recovery
from network errors.

This patch adds the recovery_tmo configuration to vdsm-multipath udev
rules. Since setting the timeout requires a script, the dmsetup command
was moved into a new multipath-configure-device script.

When we require a multipath version fixing this issue, we can remove the
timeout handling code.

Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M .gitignore
M configure.ac
M debian/vdsm.install
M vdsm.spec.in
M vdsm/storage/Makefile.am
A vdsm/storage/multipath-configure-device.in
A vdsm/storage/vdsm-multipath.rules
D vdsm/storage/vdsm-multipath.rules.in
8 files changed, 53 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/82/32582/1

diff --git a/.gitignore b/.gitignore
index fca4f2e..5890806 100644
--- a/.gitignore
+++ b/.gitignore
@@ -61,7 +61,6 @@
 vdsm/storage/protect/safelease
 vdsm/storage/lvm.env
 vdsm/storage/vdsm-lvm.rules
-vdsm/storage/vdsm-multipath.rules
 vdsm/sudoers.vdsm
 vdsm/svdsm.logger.conf
 vdsm/vdscli.py
diff --git a/configure.ac b/configure.ac
index f40d57a..c467c37 100644
--- a/configure.ac
+++ b/configure.ac
@@ -335,10 +335,10 @@
 	vdsm/rpc/Makefile
 	vdsm/sos/Makefile
 	vdsm/storage/Makefile
+	vdsm/storage/multipath-configure-device
 	vdsm/storage/imageRepository/Makefile
 	vdsm/storage/protect/Makefile
 	vdsm/storage/vdsm-lvm.rules
-	vdsm/storage/vdsm-multipath.rules
 	vdsm/virt/Makefile
 	vdsm_hooks/Makefile
 	vdsm_hooks/checkimages/Makefile
diff --git a/debian/vdsm.install b/debian/vdsm.install
index bd63c72..759383d 100644
--- a/debian/vdsm.install
+++ b/debian/vdsm.install
@@ -25,6 +25,7 @@
 ./usr/lib/python2.7/dist-packages/sos/plugins/vdsm.py
 ./usr/lib/python2.7/dist-packages/vdsmapi.py
 ./usr/libexec/vdsm/curl-img-wrap
+./usr/libexec/vdsm/multiapth-configure-device
 ./usr/libexec/vdsm/ovirt_functions.sh
 ./usr/libexec/vdsm/persist-vdsm-hooks
 ./usr/libexec/vdsm/safelease
diff --git a/vdsm.spec.in b/vdsm.spec.in
index a5b354c..c912150 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -993,6 +993,7 @@
 %{_libexecdir}/%{vdsm_name}/persist-vdsm-hooks
 %{_libexecdir}/%{vdsm_name}/unpersist-vdsm-hook
 %{_libexecdir}/%{vdsm_name}/ovirt_functions.sh
+%{_libexecdir}/%{vdsm_name}/multipath-configure-device
 %{_libexecdir}/%{vdsm_name}/vdsm-gencerts.sh
 %{_libexecdir}/%{vdsm_name}/vdsmd_init_common.sh
 %{_datadir}/%{vdsm_name}/network/__init__.py*
diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am
index 99b1460..91c2f9c 100644
--- a/vdsm/storage/Makefile.am
+++ b/vdsm/storage/Makefile.am
@@ -72,7 +72,8 @@
 	volume.py
 
 dist_vdsmexec_SCRIPTS = \
-	curl-img-wrap
+	curl-img-wrap \
+	multipath-configure-device
 
 nodist_vdsmstorage_DATA = \
 	lvm.env \
diff --git a/vdsm/storage/multipath-configure-device.in
b/vdsm/storage/multipath-configure-device.in
new file mode 100644
index 0000000..dcdfea5
--- /dev/null
+++ b/vdsm/storage/multipath-configure-device.in
@@ -0,0 +1,33 @@
+#!/bin/sh
+#
+# Copyright 2014 Red Hat, Inc. and/or its affiliates.
+#
+# Licensed to you under the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.  See the files README and
+# LICENSE_GPL_v2 which accompany this distribution.
+#
+
+set -e
+
+# Ensure that multipath devices use no_path_retry fail, instead of
+# no_path_retry queue, which is hardcoded in multipath configuration for many
+# devices.
+#
+# See
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/...
+
+@DMSETUP_PATH@ message $DM_NAME 0 fail_if_no_path
+
+# Set iscsi_session recovery_tmo. This parameter is configured by multipath on
+# startup, but after a device fail and become active again, the configuration
+# is lost and revert back to iscsid.conf default (120).
+
+timeout=5
+
+for slave in /sys${DEVPATH}/slaves/*; do
+    path=$(@UDEVADM_PATH@ info --query=path --path="$slave")
+    session=$(echo "$path" | @SED_PATH@ -rn
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
+    if [ -n "$session" ]; then
+        echo $timeout > "/sys/class/iscsi_session/${session}/recovery_tmo"
+    fi
+done
diff --git a/vdsm/storage/vdsm-multipath.rules b/vdsm/storage/vdsm-multipath.rules
new file mode 100644
index 0000000..d4d0dd7
--- /dev/null
+++ b/vdsm/storage/vdsm-multipath.rules
@@ -0,0 +1,15 @@
+#
+# Copyright 2014 Red Hat, Inc. and/or its affiliates.
+#
+# Licensed to you under the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.  See the files README and
+# LICENSE_GPL_v2 which accompany this distribution.
+#
+
+# Vdsm rules for multipath devices.
+#
+# This rule runs vdsm-mutipath-configure tool to configure multiapth devices to
+# work with vdsm.
+
+ACTION=="add|change", ENV{DM_UUID}=="mpath-?*",
RUN+="/usr/libexec/vdsm/multipath-configure-device"
diff --git a/vdsm/storage/vdsm-multipath.rules.in b/vdsm/storage/vdsm-multipath.rules.in
deleted file mode 100644
index cc2a878..0000000
--- a/vdsm/storage/vdsm-multipath.rules.in
+++ /dev/null
@@ -1,19 +0,0 @@
-#
-# Copyright 2014 Red Hat, Inc. and/or its affiliates.
-#
-# Licensed to you under the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.  See the files README and
-# LICENSE_GPL_v2 which accompany this distribution.
-#
-
-# Vdsm rules for multipath devices.
-#
-# Ensure that multipath devices use no_path_retry fail, instead of
-# no_path_retry queue, which is hardcoded in multipath configuration for many
-# devices.
-#
-# See
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/...
-#
-
-ACTION=="add|change", ENV{DM_UUID}=="mpath-?*",
RUN+="@DMSETUP_PATH@ message $env{DM_NAME} 0 fail_if_no_path"


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

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Configure iscsi_session recovery_tmo for multipath devices
......................................................................


Patch Set 1:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11350/ : FAILURE

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

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

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/multipath-configure-devi...
File vdsm/storage/multipath-configure-device.in:

Line 30:              
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
Line 31:     if [ -n "$session" ]; then
Line 32:         echo $timeout >
"/sys/class/iscsi_session/${session}/recovery_tmo"
Line 33:     fi
Line 34: done
Note that this handle only iscsi devices. On the next patch I will add a rule for FC
devices (requires fc machine for testing).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Patch Set 2:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11351/ : FAILURE

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

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

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Patch Set 3:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11352/ : FAILURE

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

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

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/Makefile.am
File vdsm/storage/Makefile.am:

Line 72: 	volume.py
Line 73: 
Line 74: dist_vdsmexec_SCRIPTS = \
Line 75: 	curl-img-wrap \
Line 76: 	multipath-configure-device
$(NULL)?
Line 77: 
Line 78: nodist_vdsmstorage_DATA = \
Line 79: 	lvm.env \
Line 80: 	$(NULL)


http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/multipath-configure-devi...
File vdsm/storage/multipath-configure-device.in:

Line 7: # (at your option) any later version.  See the files README and
Line 8: # LICENSE_GPL_v2 which accompany this distribution.
Line 9: #
Line 10: 
Line 11: set -e
usually it is better to exit with custom message instead of having sysadmin figure out
what's wrong himself.
Line 12: 
Line 13: # Ensure that multipath devices use no_path_retry fail, instead of
Line 14: # no_path_retry queue, which is hardcoded in multipath configuration for many
Line 15: # devices.


Line 23: # is lost and revert back to iscsid.conf default (120).
Line 24: 
Line 25: timeout=5
Line 26: 
Line 27: for slave in /sys${DEVPATH}/slaves/*; do
please decide if you are within the ${XXX} or $XXX convention.
Line 28:     path=$(@UDEVADM_PATH@ info --query=path --path="$slave")
Line 29:     session=$(echo "$path" | @SED_PATH@ -rn \
Line 30:              
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
Line 31:     if [ -n "$session" ]; then


Line 24: 
Line 25: timeout=5
Line 26: 
Line 27: for slave in /sys${DEVPATH}/slaves/*; do
Line 28:     path=$(@UDEVADM_PATH@ info --query=path --path="$slave")
quotes all over please
Line 29:     session=$(echo "$path" | @SED_PATH@ -rn \
Line 30:              
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
Line 31:     if [ -n "$session" ]; then
Line 32:         echo $timeout >
"/sys/class/iscsi_session/${session}/recovery_tmo"


Line 26: 
Line 27: for slave in /sys${DEVPATH}/slaves/*; do
Line 28:     path=$(@UDEVADM_PATH@ info --query=path --path="$slave")
Line 29:     session=$(echo "$path" | @SED_PATH@ -rn \
Line 30:              
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
s' -> 's not that it is that important and working but it is clearer.

just to make sure... the only way you can get the session is from the path and not
attribute of the slave? looks ugly.
Line 31:     if [ -n "$session" ]; then
Line 32:         echo $timeout >
"/sys/class/iscsi_session/${session}/recovery_tmo"
Line 33:     fi


http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/vdsm-multipath.rules
File vdsm/storage/vdsm-multipath.rules:

Line 11: #
Line 12: # This rule runs vdsm-mutipath-configure tool to configure multiapth devices to
Line 13: # work with vdsm.
Line 14: 
Line 15: ACTION=="add|change", ENV{DM_UUID}=="mpath-?*",
RUN+="/usr/libexec/vdsm/multipath-configure-device"
this should be expanded by autoconf.

usually these scripts belongs to /lib/udev with vdsm-prefix


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/Makefile.am
File vdsm/storage/Makefile.am:

Line 72: 	volume.py
Line 73: 
Line 74: dist_vdsmexec_SCRIPTS = \
Line 75: 	curl-img-wrap \
Line 76: 	multipath-configure-device
>  $(NULL)? 
Why do we need this?
Line 77: 
Line 78: nodist_vdsmstorage_DATA = \
Line 79: 	lvm.env \
Line 80: 	$(NULL)


http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/multipath-configure-devi...
File vdsm/storage/multipath-configure-device.in:

Line 7: # (at your option) any later version.  See the files README and
Line 8: # LICENSE_GPL_v2 which accompany this distribution.
Line 9: #
Line 10: 
Line 11: set -e
>  usually it is better to exit with custom message instead of having
> sysadmin 
You suggest to replace this with check for each call and logging errors?
Line 12: 
Line 13: # Ensure that multipath devices use no_path_retry fail, instead of
Line 14: # no_path_retry queue, which is hardcoded in multipath configuration for many
Line 15: # devices.


Line 23: # is lost and revert back to iscsid.conf default (120).
Line 24: 
Line 25: timeout=5
Line 26: 
Line 27: for slave in /sys${DEVPATH}/slaves/*; do
>  please decide if you are within the ${XXX} or $XXX convention.
ok
Line 28:     path=$(@UDEVADM_PATH@ info --query=path --path="$slave")
Line 29:     session=$(echo "$path" | @SED_PATH@ -rn \
Line 30:              
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
Line 31:     if [ -n "$session" ]; then


Line 24: 
Line 25: timeout=5
Line 26: 
Line 27: for slave in /sys${DEVPATH}/slaves/*; do
Line 28:     path=$(@UDEVADM_PATH@ info --query=path --path="$slave")
>  quotes all over please 
Do you mean path="$(x y)"?
Line 29:     session=$(echo "$path" | @SED_PATH@ -rn \
Line 30:              
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
Line 31:     if [ -n "$session" ]; then
Line 32:         echo $timeout >
"/sys/class/iscsi_session/${session}/recovery_tmo"


Line 26: 
Line 27: for slave in /sys${DEVPATH}/slaves/*; do
Line 28:     path=$(@UDEVADM_PATH@ info --query=path --path="$slave")
Line 29:     session=$(echo "$path" | @SED_PATH@ -rn \
Line 30:              
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
>  s' -> 's not that it is that important and working but it
> is clearer. 
s' -> 's - ok

I did not find a better way to get the session from the path. multipath use similar
solution, iterating up the path items, until they find an item named
"session%d".
Line 31:     if [ -n "$session" ]; then
Line 32:         echo $timeout >
"/sys/class/iscsi_session/${session}/recovery_tmo"
Line 33:     fi


http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/vdsm-multipath.rules
File vdsm/storage/vdsm-multipath.rules:

Line 11: #
Line 12: # This rule runs vdsm-mutipath-configure tool to configure multiapth devices to
Line 13: # work with vdsm.
Line 14: 
Line 15: ACTION=="add|change", ENV{DM_UUID}=="mpath-?*",
RUN+="/usr/libexec/vdsm/multipath-configure-device"
>  this should be expanded by autoconf. 
Would be better, since
this script assumes the environment created by udev.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/Makefile.am
File vdsm/storage/Makefile.am:

Line 72: 	volume.py
Line 73: 
Line 74: dist_vdsmexec_SCRIPTS = \
Line 75: 	curl-img-wrap \
Line 76: 	multipath-configure-device
>  Why do we need this? 
so that future diffs will change one line
(the relevant one)?
Line 77: 
Line 78: nodist_vdsmstorage_DATA = \
Line 79: 	lvm.env \
Line 80: 	$(NULL)


http://gerrit.ovirt.org/#/c/32582/3/vdsm/storage/multipath-configure-devi...
File vdsm/storage/multipath-configure-device.in:

Line 7: # (at your option) any later version.  See the files README and
Line 8: # LICENSE_GPL_v2 which accompany this distribution.
Line 9: #
Line 10: 
Line 11: set -e
>  You suggest to replace this with check for each call and logging
> errors? 
yes... proper helpful message.

just recommendation.
Line 12: 
Line 13: # Ensure that multipath devices use no_path_retry fail, instead of
Line 14: # no_path_retry queue, which is hardcoded in multipath configuration for many
Line 15: # devices.


Line 24: 
Line 25: timeout=5
Line 26: 
Line 27: for slave in /sys${DEVPATH}/slaves/*; do
Line 28:     path=$(@UDEVADM_PATH@ info --query=path --path="$slave")
>  Do you mean path="$(x y)"? 
yes
Line 29:     session=$(echo "$path" | @SED_PATH@ -rn \
Line 30:              
s'|^/devices/platform/host[0-9]+/(session[0-9]+)/.+$|\1|p')
Line 31:     if [ -n "$session" ]; then
Line 32:         echo $timeout >
"/sys/class/iscsi_session/${session}/recovery_tmo"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Patch Set 3: Verified-1

The root cause of this issue is the 120 seconds timeout in
/etc/iscsi/iscsid.conf.

multipath try to fix this by changing session recovery_tmo
dynamically for multipath devices. They implement this when a
device is added, but do not handle the case where a device
become failed and then active again.

The udev rule added here does handle devies changes and keep the
session recovery_tmo correct.

But there is one hole - when restarting iscsid, it overwrites 
recovery_tmo with the value from iscsi database (/var/lib/iscsi/nodes).
There seems to be no udev event when this happens.

So it seems that the only way to fix this reliably is to change
the default replacement_tmo in /etc/iscsi/iscsid.conf.

This requires another vdsm-tool configure module, with the normal
issues of validating configuration on startup and handling user
changes.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

[Nir Soffer]

Nir Soffer has abandoned this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Abandoned

This should be fixed in the kernel now, so we don't need this ugly udev rule.

-- 
To view, visit https://gerrit.ovirt.org/32582
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org

Change in vdsm[master]: multipath: Configure iscsi_session recovery_tmo

automation(a)ovirt.org has posted comments on this change.

Change subject: multipath: Configure iscsi_session recovery_tmo
......................................................................


Patch Set 3:

* update_tracker: OK

-- 
To view, visit https://gerrit.ovirt.org/32582
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaaa40fa5e6dc86beef501ef4aaefa17c4c1574c1
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: Alon Bar-Lev <alonbl(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No