* [PATCH] hotplug/Linux: add iscsi block hotplug script @ 2013-04-24 16:31 Roger Pau Monne 2013-04-24 16:48 ` Ian Campbell 0 siblings, 1 reply; 8+ messages in thread From: Roger Pau Monne @ 2013-04-24 16:31 UTC (permalink / raw) To: xen-devel; +Cc: Ian Jackson, Ian Campbell, Roger Pau Monne This hotplug script has been tested with IET and NetBSD iSCSI targets, without authentication. This hotplug script will only work with PV guests not using pygrub. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@citrix.com> --- Changes due to 4.3 release freeze: * We can no longer provide a user/password, because they will be stored in xenstore "params" backend node, which can be read by the guest. * Only works with PV domains because we don't have a hook in libxl to pass the block device created by the script to Qemu. * Doesn't work with pygrub because we don't call the script until pygrub has already been executed, and even if we called it earlier we still need a hook in order to provide the right block device to pygrub. Changes since v1: * Add -e to script shebang, and use 'set +e' if we know hotplug execution might fail. --- tools/hotplug/Linux/Makefile | 1 + tools/hotplug/Linux/block-iscsi | 198 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 tools/hotplug/Linux/block-iscsi diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile index 0605559..98c7738 100644 --- a/tools/hotplug/Linux/Makefile +++ b/tools/hotplug/Linux/Makefile @@ -21,6 +21,7 @@ XEN_SCRIPTS += blktap XEN_SCRIPTS += xen-hotplug-cleanup XEN_SCRIPTS += external-device-migrate XEN_SCRIPTS += vscsi +XEN_SCRIPTS += block-iscsi XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh XEN_SCRIPT_DATA += block-common.sh diff --git a/tools/hotplug/Linux/block-iscsi b/tools/hotplug/Linux/block-iscsi new file mode 100644 index 0000000..3cd6e34 --- /dev/null +++ b/tools/hotplug/Linux/block-iscsi @@ -0,0 +1,198 @@ +#!/bin/sh -e +# +# Open-iSCSI Xen block device hotplug script +# +# Author Roger Pau Monné <roger.pau@citrix.com> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published +# by the Free Software Foundation; version 2.1 only. with the special +# exception on linking described in file LICENSE. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. + +remove_label() +{ + echo $1 | sed "s/^\("$2"\)//" +} + +check_tools() +{ + if ! type iscsiadm > /dev/null 2>&1; then + echo "Unable to find iscsiadm tool" + return 1 + fi + if [ "$multipath" = "y" ] && ! type multipath > /dev/null 2>&1; then + echo "Unable to find multipath" + return 1 + fi +} + +# Sets the following global variables based on the params field passed in as +# a parameter: iqn, portal, auth_method, user, multipath, password +parse_target() +{ + # set multipath default value + multipath="n" + for param in $(echo "$1" | tr "," "\n") + do + case $param in + iqn=*) + iqn=$(remove_label $param "iqn=") + ;; + portal=*) + portal=$(remove_label $param "portal=") + ;; + multipath=*) + multipath=$(remove_label $param "multipath=") + ;; + esac + done + if [ -z "$iqn" ] || [ -z "$portal" ]; then + echo "iqn= and portal= are required parameters" + return 1 + fi + if [ "$multipath" != "y" ] && [ "$multipath" != "n" ]; then + echo "multipath valid values are y and n, $multipath not a valid value" + return 1 + fi + return 0 +} + +# Outputs the block device major:minor +device_major_minor() +{ + stat -L -c %t:%T "$1" +} + +# Sets $dev to point to the device associated with the value in iqn +find_device() +{ + while [ ! -e /dev/disk/by-path/*"$iqn"-lun-0 ]; do + sleep 0.1 + done + set +e + sddev=$(readlink -f /dev/disk/by-path/*"$iqn"-lun-0) + set -e + if [ ! -b "$sddev" ]; then + echo "Unable to find attached device path" + return 1 + fi + if [ "$multipath" = "y" ]; then + mdev=$(multipath -ll "$sddev" | head -1 | awk '{ print $1}') + if [ ! -b /dev/mapper/"$mdev" ]; then + echo "Unable to find attached device multipath" + return 1 + fi + dev="/dev/mapper/$mdev" + else + dev="$sddev" + fi + return 0 +} + +# Attaches the target $iqn in $portal and sets $dev to point to the +# multipath device +attach() +{ + set +e + iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null 2>&1 + rc=$? + set -e + if [ $rc -ne 0 ]; then + echo "Unable to connect to target" + return 1 + fi + find_device + if [ $? -ne 0 ]; then + echo "Unable to find iSCSI device" + return 1 + fi + return 0 +} + +# Discovers targets in $portal and checks that $iqn is one of those targets +# Also sets the auth parameters to attach the device +prepare() +{ + # Check if target is already opened + set +e + iscsiadm -m session 2>&1 | grep -q "$iqn" + rc=$? + set -e + if [ $rc -eq 0 ]; then + echo "Device already opened" + return 1 + fi + # Discover portal targets + set +e + iscsiadm -m discovery -t st -p $portal 2>&1 | grep -q "$iqn" + rc=$? + set -e + if [ $rc -ne 0 ]; then + echo "No matching target iqn found" + return 1 + fi + return 0 +} + +# Attaches the device and writes xenstore backend entries to connect +# the device +add() +{ + attach + if [ $? -ne 0 ]; then + echo "Failed to attach device" + return 1 + fi + mm=$(device_major_minor "$dev") + xenstore-write "$XENBUS_PATH/physical-device" "$mm" + return 0 +} + +# Disconnects the device +remove() +{ + find_device + if [ $? -ne 0 ]; then + echo "Unable to find device" + return 1 + fi + set +e + iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null 2>&1 + rc=$? + set -e + if [ $rc -ne 0 ]; then + echo "Unable to disconnect target" + return 1 + fi + return 0 +} + +command=$1 +set +e +target=$(xenstore-read $XENBUS_PATH/params) +set -e +if [ -z "$target" ]; then + echo "No information about the target" + exit 1 +fi + +parse_target "$target" + +check_tools || exit 1 + +case $command in +add) + prepare + add + exit $? + ;; +remove) + remove + exit $? + ;; +esac -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] hotplug/Linux: add iscsi block hotplug script 2013-04-24 16:31 [PATCH] hotplug/Linux: add iscsi block hotplug script Roger Pau Monne @ 2013-04-24 16:48 ` Ian Campbell 2013-04-25 12:32 ` Roger Pau Monné 0 siblings, 1 reply; 8+ messages in thread From: Ian Campbell @ 2013-04-24 16:48 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Ian Jackson, xen-devel@lists.xen.org On Wed, 2013-04-24 at 17:31 +0100, Roger Pau Monne wrote: > This hotplug script has been tested with IET and NetBSD iSCSI targets, > without authentication. > > This hotplug script will only work with PV guests not using pygrub. Thanks. Does this need any docs somewhere, e.g. how to format the target spec? Is this derived from any of the other block-iscsi scripts floating around on the net? > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Ian Jackson <ian.jackson@citrix.com> > --- > Changes due to 4.3 release freeze: > * We can no longer provide a user/password, because they will be > stored in xenstore "params" backend node, which can be read by the > guest. > * Only works with PV domains because we don't have a hook in libxl to > pass the block device created by the script to Qemu. > * Doesn't work with pygrub because we don't call the script until > pygrub has already been executed, and even if we called it earlier > we still need a hook in order to provide the right block device to > pygrub. > Changes since v1: > * Add -e to script shebang, and use 'set +e' if we know hotplug > execution might fail. > --- > tools/hotplug/Linux/Makefile | 1 + > tools/hotplug/Linux/block-iscsi | 198 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 199 insertions(+), 0 deletions(-) > create mode 100644 tools/hotplug/Linux/block-iscsi > > diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile > index 0605559..98c7738 100644 > --- a/tools/hotplug/Linux/Makefile > +++ b/tools/hotplug/Linux/Makefile > @@ -21,6 +21,7 @@ XEN_SCRIPTS += blktap > XEN_SCRIPTS += xen-hotplug-cleanup > XEN_SCRIPTS += external-device-migrate > XEN_SCRIPTS += vscsi > +XEN_SCRIPTS += block-iscsi > XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh > XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh > XEN_SCRIPT_DATA += block-common.sh > diff --git a/tools/hotplug/Linux/block-iscsi b/tools/hotplug/Linux/block-iscsi > new file mode 100644 > index 0000000..3cd6e34 > --- /dev/null > +++ b/tools/hotplug/Linux/block-iscsi > @@ -0,0 +1,198 @@ > +#!/bin/sh -e Does this rely on any bashisms? Most of the other scripts use bash here, and I'd be a bit worried about bashisms in the common code. > +# > +# Open-iSCSI Xen block device hotplug script > +# > +# Author Roger Pau Monné <roger.pau@citrix.com> > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU Lesser General Public License as published > +# by the Free Software Foundation; version 2.1 only. with the special > +# exception on linking described in file LICENSE. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU Lesser General Public License for more details. > + > +remove_label() > +{ > + echo $1 | sed "s/^\("$2"\)//" > +} > + > +check_tools() > +{ > + if ! type iscsiadm > /dev/null 2>&1; then apparently command -v is more portable? > + echo "Unable to find iscsiadm tool" > + return 1 Error paths should use fatal() from xen-hotplug-common.sh so the error is propagated to xenstore and libxl can print it. > + fi > + if [ "$multipath" = "y" ] && ! type multipath > /dev/null 2>&1; then > + echo "Unable to find multipath" > + return 1 > + fi > +} > + > +# Sets the following global variables based on the params field passed in as > +# a parameter: iqn, portal, auth_method, user, multipath, password > +parse_target() > +{ > + # set multipath default value > + multipath="n" > + for param in $(echo "$1" | tr "," "\n") > + do > + case $param in > + iqn=*) > + iqn=$(remove_label $param "iqn=") > + ;; > + portal=*) > + portal=$(remove_label $param "portal=") > + ;; > + multipath=*) > + multipath=$(remove_label $param "multipath=") > + ;; > + esac > + done > + if [ -z "$iqn" ] || [ -z "$portal" ]; then > + echo "iqn= and portal= are required parameters" > + return 1 > + fi > + if [ "$multipath" != "y" ] && [ "$multipath" != "n" ]; then > + echo "multipath valid values are y and n, $multipath not a valid value" > + return 1 > + fi > + return 0 > +} > + > +# Outputs the block device major:minor > +device_major_minor() > +{ > + stat -L -c %t:%T "$1" > +} Could use block-common.sh provided functionality here. > + > +# Sets $dev to point to the device associated with the value in iqn > +find_device() > +{ > + while [ ! -e /dev/disk/by-path/*"$iqn"-lun-0 ]; do > + sleep 0.1 > + done This loop is potentially infinite? (i.e. if something is broken) > + set +e > + sddev=$(readlink -f /dev/disk/by-path/*"$iqn"-lun-0) > + set -e Can't you just to || true instead of frobbing +/-e? Or use do_without_error? > + if [ ! -b "$sddev" ]; then > + echo "Unable to find attached device path" > + return 1 > + fi > + if [ "$multipath" = "y" ]; then > + mdev=$(multipath -ll "$sddev" | head -1 | awk '{ print $1}') > + if [ ! -b /dev/mapper/"$mdev" ]; then > + echo "Unable to find attached device multipath" > + return 1 > + fi > + dev="/dev/mapper/$mdev" > + else > + dev="$sddev" > + fi > + return 0 > +} > + > +# Attaches the target $iqn in $portal and sets $dev to point to the > +# multipath device > +attach() > +{ > + set +e > + iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null 2>&1 Might the error message be useful here? if you don't redirect to /dev/null then you can use exec >>/tmp/hotplug.log tricks to debug things... > + rc=$? > + set -e > + if [ $rc -ne 0 ]; then > + echo "Unable to connect to target" > + return 1 > + fi > + find_device > + if [ $? -ne 0 ]; then > + echo "Unable to find iSCSI device" > + return 1 > + fi > + return 0 > +} > + > +# Discovers targets in $portal and checks that $iqn is one of those targets > +# Also sets the auth parameters to attach the device > +prepare() > +{ > + # Check if target is already opened > + set +e > + iscsiadm -m session 2>&1 | grep -q "$iqn" > + rc=$? > + set -e > + if [ $rc -eq 0 ]; then > + echo "Device already opened" > + return 1 > + fi > + # Discover portal targets > + set +e > + iscsiadm -m discovery -t st -p $portal 2>&1 | grep -q "$iqn" > + rc=$? > + set -e > + if [ $rc -ne 0 ]; then > + echo "No matching target iqn found" > + return 1 > + fi > + return 0 > +} > + > +# Attaches the device and writes xenstore backend entries to connect > +# the device > +add() > +{ > + attach > + if [ $? -ne 0 ]; then > + echo "Failed to attach device" > + return 1 > + fi > + mm=$(device_major_minor "$dev") > + xenstore-write "$XENBUS_PATH/physical-device" "$mm" this == write_dev > + return 0 > +} > + > +# Disconnects the device > +remove() > +{ > + find_device > + if [ $? -ne 0 ]; then > + echo "Unable to find device" > + return 1 > + fi > + set +e > + iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null 2>&1 > + rc=$? > + set -e > + if [ $rc -ne 0 ]; then > + echo "Unable to disconnect target" > + return 1 > + fi > + return 0 > +} > + > +command=$1 > +set +e > +target=$(xenstore-read $XENBUS_PATH/params) > +set -e > +if [ -z "$target" ]; then > + echo "No information about the target" > + exit 1 > +fi > + > +parse_target "$target" > + > +check_tools || exit 1 > + > +case $command in > +add) > + prepare > + add > + exit $? > + ;; > +remove) > + remove > + exit $? > + ;; exit 1 for the default case? > +esac _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hotplug/Linux: add iscsi block hotplug script 2013-04-24 16:48 ` Ian Campbell @ 2013-04-25 12:32 ` Roger Pau Monné 2013-04-25 12:42 ` Roger Pau Monné 2013-04-25 13:02 ` Ian Campbell 0 siblings, 2 replies; 8+ messages in thread From: Roger Pau Monné @ 2013-04-25 12:32 UTC (permalink / raw) To: Ian Campbell; +Cc: Ian Jackson, xen-devel@lists.xen.org On 24/04/13 18:48, Ian Campbell wrote: > On Wed, 2013-04-24 at 17:31 +0100, Roger Pau Monne wrote: >> This hotplug script has been tested with IET and NetBSD iSCSI targets, >> without authentication. >> >> This hotplug script will only work with PV guests not using pygrub. > > Thanks. Does this need any docs somewhere, e.g. how to format the target > spec? > > Is this derived from any of the other block-iscsi scripts floating > around on the net? > >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> Cc: Ian Campbell <ian.campbell@citrix.com> >> Cc: Ian Jackson <ian.jackson@citrix.com> >> --- >> Changes due to 4.3 release freeze: >> * We can no longer provide a user/password, because they will be >> stored in xenstore "params" backend node, which can be read by the >> guest. >> * Only works with PV domains because we don't have a hook in libxl to >> pass the block device created by the script to Qemu. >> * Doesn't work with pygrub because we don't call the script until >> pygrub has already been executed, and even if we called it earlier >> we still need a hook in order to provide the right block device to >> pygrub. >> Changes since v1: >> * Add -e to script shebang, and use 'set +e' if we know hotplug >> execution might fail. >> --- >> tools/hotplug/Linux/Makefile | 1 + >> tools/hotplug/Linux/block-iscsi | 198 +++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 199 insertions(+), 0 deletions(-) >> create mode 100644 tools/hotplug/Linux/block-iscsi >> >> diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile >> index 0605559..98c7738 100644 >> --- a/tools/hotplug/Linux/Makefile >> +++ b/tools/hotplug/Linux/Makefile >> @@ -21,6 +21,7 @@ XEN_SCRIPTS += blktap >> XEN_SCRIPTS += xen-hotplug-cleanup >> XEN_SCRIPTS += external-device-migrate >> XEN_SCRIPTS += vscsi >> +XEN_SCRIPTS += block-iscsi >> XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh >> XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh >> XEN_SCRIPT_DATA += block-common.sh >> diff --git a/tools/hotplug/Linux/block-iscsi b/tools/hotplug/Linux/block-iscsi >> new file mode 100644 >> index 0000000..3cd6e34 >> --- /dev/null >> +++ b/tools/hotplug/Linux/block-iscsi >> @@ -0,0 +1,198 @@ >> +#!/bin/sh -e > > Does this rely on any bashisms? Most of the other scripts use bash here, > and I'd be a bit worried about bashisms in the common code. This script originally didn't use any bashisms, but if I include block-common.sh I have to use bash for sure. > >> +# >> +# Open-iSCSI Xen block device hotplug script >> +# >> +# Author Roger Pau Monné <roger.pau@citrix.com> >> +# >> +# This program is free software; you can redistribute it and/or modify >> +# it under the terms of the GNU Lesser General Public License as published >> +# by the Free Software Foundation; version 2.1 only. with the special >> +# exception on linking described in file LICENSE. >> +# >> +# This program is distributed in the hope that it will be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +# GNU Lesser General Public License for more details. >> + >> +remove_label() >> +{ >> + echo $1 | sed "s/^\("$2"\)//" >> +} >> + >> +check_tools() >> +{ >> + if ! type iscsiadm > /dev/null 2>&1; then > > apparently command -v is more portable? OK, I have no preference. > >> + echo "Unable to find iscsiadm tool" >> + return 1 > > Error paths should use fatal() from xen-hotplug-common.sh so the error > is propagated to xenstore and libxl can print it. Yes, if we call the script directly from libxl the output form the script will be logged by libxl, but since this script can also be called from udev I have to use fatal. >> + fi >> + if [ "$multipath" = "y" ] && ! type multipath > /dev/null 2>&1; then >> + echo "Unable to find multipath" >> + return 1 >> + fi >> +} >> + >> +# Sets the following global variables based on the params field passed in as >> +# a parameter: iqn, portal, auth_method, user, multipath, password >> +parse_target() >> +{ >> + # set multipath default value >> + multipath="n" >> + for param in $(echo "$1" | tr "," "\n") >> + do >> + case $param in >> + iqn=*) >> + iqn=$(remove_label $param "iqn=") >> + ;; >> + portal=*) >> + portal=$(remove_label $param "portal=") >> + ;; >> + multipath=*) >> + multipath=$(remove_label $param "multipath=") >> + ;; >> + esac >> + done >> + if [ -z "$iqn" ] || [ -z "$portal" ]; then >> + echo "iqn= and portal= are required parameters" >> + return 1 >> + fi >> + if [ "$multipath" != "y" ] && [ "$multipath" != "n" ]; then >> + echo "multipath valid values are y and n, $multipath not a valid value" >> + return 1 >> + fi >> + return 0 >> +} >> + >> +# Outputs the block device major:minor >> +device_major_minor() >> +{ >> + stat -L -c %t:%T "$1" >> +} > > Could use block-common.sh provided functionality here. I've removed this and used write_dev instead. > >> + >> +# Sets $dev to point to the device associated with the value in iqn >> +find_device() >> +{ >> + while [ ! -e /dev/disk/by-path/*"$iqn"-lun-0 ]; do >> + sleep 0.1 >> + done > > This loop is potentially infinite? (i.e. if something is broken) Yes, I was trusting the timeout in libxl to prevent that, but again since this is not using the new calling convention it should work properly with udev and I have no idea if udev has a timeout, but we shouldn't rely on that. > >> + set +e >> + sddev=$(readlink -f /dev/disk/by-path/*"$iqn"-lun-0) >> + set -e > > Can't you just to || true instead of frobbing +/-e? Or use > do_without_error? || true seems appropriate in this case. >> + if [ ! -b "$sddev" ]; then >> + echo "Unable to find attached device path" >> + return 1 >> + fi >> + if [ "$multipath" = "y" ]; then >> + mdev=$(multipath -ll "$sddev" | head -1 | awk '{ print $1}') >> + if [ ! -b /dev/mapper/"$mdev" ]; then >> + echo "Unable to find attached device multipath" >> + return 1 >> + fi >> + dev="/dev/mapper/$mdev" >> + else >> + dev="$sddev" >> + fi >> + return 0 >> +} >> + >> +# Attaches the target $iqn in $portal and sets $dev to point to the >> +# multipath device >> +attach() >> +{ >> + set +e >> + iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null 2>&1 > > Might the error message be useful here? if you don't redirect > to /dev/null then you can use exec >>/tmp/hotplug.log tricks to debug > things... iscsiadm always prints output, even when the target is successfully attached, and I haven't found any option to prevent it, so for now I guess we will have to redirect it to /dev/null > >> + rc=$? >> + set -e >> + if [ $rc -ne 0 ]; then >> + echo "Unable to connect to target" >> + return 1 >> + fi >> + find_device >> + if [ $? -ne 0 ]; then >> + echo "Unable to find iSCSI device" >> + return 1 >> + fi >> + return 0 >> +} >> + >> +# Discovers targets in $portal and checks that $iqn is one of those targets >> +# Also sets the auth parameters to attach the device >> +prepare() >> +{ >> + # Check if target is already opened >> + set +e >> + iscsiadm -m session 2>&1 | grep -q "$iqn" >> + rc=$? >> + set -e >> + if [ $rc -eq 0 ]; then >> + echo "Device already opened" >> + return 1 >> + fi >> + # Discover portal targets >> + set +e >> + iscsiadm -m discovery -t st -p $portal 2>&1 | grep -q "$iqn" >> + rc=$? >> + set -e >> + if [ $rc -ne 0 ]; then >> + echo "No matching target iqn found" >> + return 1 >> + fi >> + return 0 >> +} >> + >> +# Attaches the device and writes xenstore backend entries to connect >> +# the device >> +add() >> +{ >> + attach >> + if [ $? -ne 0 ]; then >> + echo "Failed to attach device" >> + return 1 >> + fi >> + mm=$(device_major_minor "$dev") >> + xenstore-write "$XENBUS_PATH/physical-device" "$mm" > > this == write_dev Done > >> + return 0 >> +} >> + >> +# Disconnects the device >> +remove() >> +{ >> + find_device >> + if [ $? -ne 0 ]; then >> + echo "Unable to find device" >> + return 1 >> + fi >> + set +e >> + iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null 2>&1 >> + rc=$? >> + set -e >> + if [ $rc -ne 0 ]; then >> + echo "Unable to disconnect target" >> + return 1 >> + fi >> + return 0 >> +} >> + >> +command=$1 >> +set +e >> +target=$(xenstore-read $XENBUS_PATH/params) >> +set -e >> +if [ -z "$target" ]; then >> + echo "No information about the target" >> + exit 1 >> +fi >> + >> +parse_target "$target" >> + >> +check_tools || exit 1 >> + >> +case $command in >> +add) >> + prepare >> + add >> + exit $? >> + ;; >> +remove) >> + remove >> + exit $? >> + ;; > > exit 1 for the default case? Yes >> +esac > > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hotplug/Linux: add iscsi block hotplug script 2013-04-25 12:32 ` Roger Pau Monné @ 2013-04-25 12:42 ` Roger Pau Monné 2013-04-25 13:03 ` Ian Campbell 2013-04-25 13:02 ` Ian Campbell 1 sibling, 1 reply; 8+ messages in thread From: Roger Pau Monné @ 2013-04-25 12:42 UTC (permalink / raw) To: Ian Campbell; +Cc: Ian Jackson, xen-devel@lists.xen.org On 25/04/13 14:32, Roger Pau Monné wrote: > On 24/04/13 18:48, Ian Campbell wrote: >> On Wed, 2013-04-24 at 17:31 +0100, Roger Pau Monne wrote: >>> This hotplug script has been tested with IET and NetBSD iSCSI targets, >>> without authentication. >>> >>> This hotplug script will only work with PV guests not using pygrub. >> >> Thanks. Does this need any docs somewhere, e.g. how to format the target >> spec? Maybe an example in docs/misc/xl-disk-configuration.txt? I'm not really sure where's the best place to put it, any recommendations? >> Is this derived from any of the other block-iscsi scripts floating >> around on the net? Not really, I've taken a look at other scripts, but decided to start it from scratch. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hotplug/Linux: add iscsi block hotplug script 2013-04-25 12:42 ` Roger Pau Monné @ 2013-04-25 13:03 ` Ian Campbell 0 siblings, 0 replies; 8+ messages in thread From: Ian Campbell @ 2013-04-25 13:03 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Ian Jackson, xen-devel@lists.xen.org On Thu, 2013-04-25 at 13:42 +0100, Roger Pau Monne wrote: > On 25/04/13 14:32, Roger Pau Monné wrote: > > On 24/04/13 18:48, Ian Campbell wrote: > >> On Wed, 2013-04-24 at 17:31 +0100, Roger Pau Monne wrote: > >>> This hotplug script has been tested with IET and NetBSD iSCSI targets, > >>> without authentication. > >>> > >>> This hotplug script will only work with PV guests not using pygrub. > >> > >> Thanks. Does this need any docs somewhere, e.g. how to format the target > >> spec? > > Maybe an example in docs/misc/xl-disk-configuration.txt? I think this one is a documentation of the syntax in general and the examples are there to serve as examples of that rather than specific configurations. > I'm not really sure where's the best place to put it, any recommendations? A new docs/misc/*iscsi*.markdown? Perhaps a more comprehensive wiki howto too? > >> Is this derived from any of the other block-iscsi scripts floating > >> around on the net? > > Not really, I've taken a look at other scripts, but decided to start it > from scratch. OK. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hotplug/Linux: add iscsi block hotplug script 2013-04-25 12:32 ` Roger Pau Monné 2013-04-25 12:42 ` Roger Pau Monné @ 2013-04-25 13:02 ` Ian Campbell 2013-04-25 13:24 ` Roger Pau Monné 1 sibling, 1 reply; 8+ messages in thread From: Ian Campbell @ 2013-04-25 13:02 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Ian Jackson, xen-devel@lists.xen.org On Thu, 2013-04-25 at 13:32 +0100, Roger Pau Monne wrote: > On 24/04/13 18:48, Ian Campbell wrote: > > On Wed, 2013-04-24 at 17:31 +0100, Roger Pau Monne wrote: > >> This hotplug script has been tested with IET and NetBSD iSCSI targets, > >> without authentication. > >> > >> This hotplug script will only work with PV guests not using pygrub. > > > > Thanks. Does this need any docs somewhere, e.g. how to format the target > > spec? > > > > Is this derived from any of the other block-iscsi scripts floating > > around on the net? > > > >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > >> Cc: Ian Campbell <ian.campbell@citrix.com> > >> Cc: Ian Jackson <ian.jackson@citrix.com> > >> --- > >> Changes due to 4.3 release freeze: > >> * We can no longer provide a user/password, because they will be > >> stored in xenstore "params" backend node, which can be read by the > >> guest. > >> * Only works with PV domains because we don't have a hook in libxl to > >> pass the block device created by the script to Qemu. > >> * Doesn't work with pygrub because we don't call the script until > >> pygrub has already been executed, and even if we called it earlier > >> we still need a hook in order to provide the right block device to > >> pygrub. > >> Changes since v1: > >> * Add -e to script shebang, and use 'set +e' if we know hotplug > >> execution might fail. > >> --- > >> tools/hotplug/Linux/Makefile | 1 + > >> tools/hotplug/Linux/block-iscsi | 198 +++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 199 insertions(+), 0 deletions(-) > >> create mode 100644 tools/hotplug/Linux/block-iscsi > >> > >> diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile > >> index 0605559..98c7738 100644 > >> --- a/tools/hotplug/Linux/Makefile > >> +++ b/tools/hotplug/Linux/Makefile > >> @@ -21,6 +21,7 @@ XEN_SCRIPTS += blktap > >> XEN_SCRIPTS += xen-hotplug-cleanup > >> XEN_SCRIPTS += external-device-migrate > >> XEN_SCRIPTS += vscsi > >> +XEN_SCRIPTS += block-iscsi > >> XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh > >> XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh > >> XEN_SCRIPT_DATA += block-common.sh > >> diff --git a/tools/hotplug/Linux/block-iscsi b/tools/hotplug/Linux/block-iscsi > >> new file mode 100644 > >> index 0000000..3cd6e34 > >> --- /dev/null > >> +++ b/tools/hotplug/Linux/block-iscsi > >> @@ -0,0 +1,198 @@ > >> +#!/bin/sh -e > > > > Does this rely on any bashisms? Most of the other scripts use bash here, > > and I'd be a bit worried about bashisms in the common code. > > This script originally didn't use any bashisms, but if I include > block-common.sh I have to use bash for sure. I think I would prefer using the library functions over avoiding bash. If someone hates bash that much they should port the library over too ;-) > > > >> + echo "Unable to find iscsiadm tool" > >> + return 1 > > > > Error paths should use fatal() from xen-hotplug-common.sh so the error > > is propagated to xenstore and libxl can print it. > > Yes, if we call the script directly from libxl the output form the > script will be logged by libxl, Actually logged or just sent to stdout which libxl hasn't interfered with? or does libxl suck in the stdout of the script? It looks to me like libxl does the former. > but since this script can also be called from udev I have to use fatal. I think even for libxl, since it writes the error node which libxl does then log. > > > >> + > >> +# Sets $dev to point to the device associated with the value in iqn > >> +find_device() > >> +{ > >> + while [ ! -e /dev/disk/by-path/*"$iqn"-lun-0 ]; do > >> + sleep 0.1 > >> + done > > > > This loop is potentially infinite? (i.e. if something is broken) > > Yes, I was trusting the timeout in libxl to prevent that, but again > since this is not using the new calling convention it should work > properly with udev and I have no idea if udev has a timeout, but we > shouldn't rely on that. Good. > >> + if [ ! -b "$sddev" ]; then > >> + echo "Unable to find attached device path" > >> + return 1 > >> + fi > >> + if [ "$multipath" = "y" ]; then > >> + mdev=$(multipath -ll "$sddev" | head -1 | awk '{ print $1}') > >> + if [ ! -b /dev/mapper/"$mdev" ]; then > >> + echo "Unable to find attached device multipath" > >> + return 1 > >> + fi > >> + dev="/dev/mapper/$mdev" > >> + else > >> + dev="$sddev" > >> + fi > >> + return 0 > >> +} > >> + > >> +# Attaches the target $iqn in $portal and sets $dev to point to the > >> +# multipath device > >> +attach() > >> +{ > >> + set +e > >> + iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null 2>&1 > > > > Might the error message be useful here? if you don't redirect > > to /dev/null then you can use exec >>/tmp/hotplug.log tricks to debug > > things... > > iscsiadm always prints output, even when the target is successfully > attached, and I haven't found any option to prevent it, so for now I > guess we will have to redirect it to /dev/null Does it produce output on stderr even on success too? i.e. can we just redirect stdout? In a sane world the useful error messages would be on stderr ;-) Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hotplug/Linux: add iscsi block hotplug script 2013-04-25 13:02 ` Ian Campbell @ 2013-04-25 13:24 ` Roger Pau Monné 2013-04-25 13:57 ` Ian Campbell 0 siblings, 1 reply; 8+ messages in thread From: Roger Pau Monné @ 2013-04-25 13:24 UTC (permalink / raw) To: Ian Campbell; +Cc: Ian Jackson, xen-devel@lists.xen.org On 25/04/13 15:02, Ian Campbell wrote: > On Thu, 2013-04-25 at 13:32 +0100, Roger Pau Monne wrote: >> On 24/04/13 18:48, Ian Campbell wrote: >>> On Wed, 2013-04-24 at 17:31 +0100, Roger Pau Monne wrote: >>>> This hotplug script has been tested with IET and NetBSD iSCSI targets, >>>> without authentication. >>>> >>>> This hotplug script will only work with PV guests not using pygrub. >>> >>> Thanks. Does this need any docs somewhere, e.g. how to format the target >>> spec? >>> >>> Is this derived from any of the other block-iscsi scripts floating >>> around on the net? >>> >>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>>> Cc: Ian Campbell <ian.campbell@citrix.com> >>>> Cc: Ian Jackson <ian.jackson@citrix.com> >>>> --- >>>> Changes due to 4.3 release freeze: >>>> * We can no longer provide a user/password, because they will be >>>> stored in xenstore "params" backend node, which can be read by the >>>> guest. >>>> * Only works with PV domains because we don't have a hook in libxl to >>>> pass the block device created by the script to Qemu. >>>> * Doesn't work with pygrub because we don't call the script until >>>> pygrub has already been executed, and even if we called it earlier >>>> we still need a hook in order to provide the right block device to >>>> pygrub. >>>> Changes since v1: >>>> * Add -e to script shebang, and use 'set +e' if we know hotplug >>>> execution might fail. >>>> --- >>>> tools/hotplug/Linux/Makefile | 1 + >>>> tools/hotplug/Linux/block-iscsi | 198 +++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 199 insertions(+), 0 deletions(-) >>>> create mode 100644 tools/hotplug/Linux/block-iscsi >>>> >>>> diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile >>>> index 0605559..98c7738 100644 >>>> --- a/tools/hotplug/Linux/Makefile >>>> +++ b/tools/hotplug/Linux/Makefile >>>> @@ -21,6 +21,7 @@ XEN_SCRIPTS += blktap >>>> XEN_SCRIPTS += xen-hotplug-cleanup >>>> XEN_SCRIPTS += external-device-migrate >>>> XEN_SCRIPTS += vscsi >>>> +XEN_SCRIPTS += block-iscsi >>>> XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh >>>> XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh >>>> XEN_SCRIPT_DATA += block-common.sh >>>> diff --git a/tools/hotplug/Linux/block-iscsi b/tools/hotplug/Linux/block-iscsi >>>> new file mode 100644 >>>> index 0000000..3cd6e34 >>>> --- /dev/null >>>> +++ b/tools/hotplug/Linux/block-iscsi >>>> @@ -0,0 +1,198 @@ >>>> +#!/bin/sh -e >>> >>> Does this rely on any bashisms? Most of the other scripts use bash here, >>> and I'd be a bit worried about bashisms in the common code. >> >> This script originally didn't use any bashisms, but if I include >> block-common.sh I have to use bash for sure. > > I think I would prefer using the library functions over avoiding bash. > > If someone hates bash that much they should port the library over > too ;-) > >>> >>>> + echo "Unable to find iscsiadm tool" >>>> + return 1 >>> >>> Error paths should use fatal() from xen-hotplug-common.sh so the error >>> is propagated to xenstore and libxl can print it. >> >> Yes, if we call the script directly from libxl the output form the >> script will be logged by libxl, > > Actually logged or just sent to stdout which libxl hasn't interfered > with? or does libxl suck in the stdout of the script? It looks to me > like libxl does the former. > >> but since this script can also be called from udev I have to use fatal. > > I think even for libxl, since it writes the error node which libxl does > then log. Yes, libxl checks the node, so it's better to use fatal/log and all this functions instead of printing to stdout/stderr. > >>> >>>> + >>>> +# Sets $dev to point to the device associated with the value in iqn >>>> +find_device() >>>> +{ >>>> + while [ ! -e /dev/disk/by-path/*"$iqn"-lun-0 ]; do >>>> + sleep 0.1 >>>> + done >>> >>> This loop is potentially infinite? (i.e. if something is broken) >> >> Yes, I was trusting the timeout in libxl to prevent that, but again >> since this is not using the new calling convention it should work >> properly with udev and I have no idea if udev has a timeout, but we >> shouldn't rely on that. > > Good. > >>>> + if [ ! -b "$sddev" ]; then >>>> + echo "Unable to find attached device path" >>>> + return 1 >>>> + fi >>>> + if [ "$multipath" = "y" ]; then >>>> + mdev=$(multipath -ll "$sddev" | head -1 | awk '{ print $1}') >>>> + if [ ! -b /dev/mapper/"$mdev" ]; then >>>> + echo "Unable to find attached device multipath" >>>> + return 1 >>>> + fi >>>> + dev="/dev/mapper/$mdev" >>>> + else >>>> + dev="$sddev" >>>> + fi >>>> + return 0 >>>> +} >>>> + >>>> +# Attaches the target $iqn in $portal and sets $dev to point to the >>>> +# multipath device >>>> +attach() >>>> +{ >>>> + set +e >>>> + iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null 2>&1 >>> >>> Might the error message be useful here? if you don't redirect >>> to /dev/null then you can use exec >>/tmp/hotplug.log tricks to debug >>> things... >> >> iscsiadm always prints output, even when the target is successfully >> attached, and I haven't found any option to prevent it, so for now I >> guess we will have to redirect it to /dev/null > > Does it produce output on stderr even on success too? i.e. can we just > redirect stdout? In a sane world the useful error messages would be on > stderr ;-) Mmmm, good point, yes, I can redirect stdout only. > > Ian. > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hotplug/Linux: add iscsi block hotplug script 2013-04-25 13:24 ` Roger Pau Monné @ 2013-04-25 13:57 ` Ian Campbell 0 siblings, 0 replies; 8+ messages in thread From: Ian Campbell @ 2013-04-25 13:57 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Ian Jackson, xen-devel@lists.xen.org > >>> > >>>> + echo "Unable to find iscsiadm tool" > >>>> + return 1 > >>> > >>> Error paths should use fatal() from xen-hotplug-common.sh so the error > >>> is propagated to xenstore and libxl can print it. > >> > >> Yes, if we call the script directly from libxl the output form the > >> script will be logged by libxl, > > > > Actually logged or just sent to stdout which libxl hasn't interfered > > with? or does libxl suck in the stdout of the script? It looks to me > > like libxl does the former. > > > >> but since this script can also be called from udev I have to use fatal. > > > > I think even for libxl, since it writes the error node which libxl does > > then log. > > Yes, libxl checks the node, so it's better to use fatal/log and all this > functions instead of printing to stdout/stderr. Agreed. Slurping hotplug script output into a debug log file might actually be a useful thing for the TODO list though. Ian. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-04-25 13:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-24 16:31 [PATCH] hotplug/Linux: add iscsi block hotplug script Roger Pau Monne 2013-04-24 16:48 ` Ian Campbell 2013-04-25 12:32 ` Roger Pau Monné 2013-04-25 12:42 ` Roger Pau Monné 2013-04-25 13:03 ` Ian Campbell 2013-04-25 13:02 ` Ian Campbell 2013-04-25 13:24 ` Roger Pau Monné 2013-04-25 13:57 ` Ian Campbell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.