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-device.... 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"