|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] hotplug/Linux: add iscsi block hotplug script
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@xxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
> +#
> +# 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |