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

Alon Bar-Lev alonbl at redhat.com
Mon Sep 8 04:38:42 UTC 2014


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-device.in
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 at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list