Re: Resend: [PATCH] v3 - Add exclusive locking option to block-iscsi

[Wei Liu <wei.liu2@citrix.com>]

On Thu, May 19, 2016 at 11:29:32AM +1000, Steven Haigh wrote:
> On 2016-05-09 14:22, Steven Haigh wrote:
> >On 2016-05-05 15:52, Steven Haigh wrote:
> >>On 2016-05-05 12:32, Steven Haigh wrote:
> >>>Overview
> >>>
> >>>If you're using iSCSI, you can mount a target by multiple Dom0
> >>>machines on the same target. For non-cluster aware filesystems, this
> >>>can lead to disk corruption and general bad times by all. The iSCSI
> >>>protocol allows the use of persistent reservations as per the SCSI
> >>>disk spec. Low level SCSI commands for locking are handled by the
> >>>sg_persist program (bundled with sg3_utils package in EL).
> >>>
> >>>The aim of this patch is to create a 'locktarget=y' option specified
> >>>within the disk 'target' command for iSCSI to lock the target in
> >>>exclusive mode on VM start with a key generated from the local systems
> >>>IP, and release this lock on the shutdown of the DomU.
> >>>
> >>>Example Config:
> >>>disk            =
> >>>['script=block-iscsi,vdev=xvda,target=iqn=iqn.1986-03.com.sun:02:mytarget,portal=iscsi.example.com,locktarget=y']
> >>>
> >>>In writing this, I have also re-factored parts of the script to put
> >>>some things in what I believe to be a better place to make expansion
> >>>easier. This is mainly in removing functions that purely call other
> >>>functions with no actual code execution.
> >>>
> >>>Signed-off-by: Steven Haigh <netwiz@crc.id.au>
> >>>
> >>>(on a side note, first time I've submitted a patch to the list and I'm
> >>>currently stuck on a webmail client, so apologies in advance if this
> >>>all goes wrong ;)
> >>
> >>Changes in v2:
> >>Bugfix: Call find_device to locate the /dev/sdX component of the iSCSI
> >>target before trying to run unlock_device().
> >>
> >>Apologies for this oversight.
> >>
> >
> >Changes in v3:
> >* Split the block-iscsi cleanup into a seperate patch
> >(block-iscsi-locking-v3_01_simplify_block-iscsi.patch).
> >* Add locking in second patch file
> >(block-iscsi-locking-v3_02_add_locking.patch)
> 
> Resend of patches.
> 
> There was a mention of having to add further documentation to
> xl-disk-configuration.txt - however there are no mentions of block-iscsi
> script within the documentation to add. As such, it probably would be out of
> place to add things here.
> 
> The locktarget option is presented directly to the block-iscsi script and
> not evaluated anywhere outside this script.
> 

Sorry I dropped the ball. I think it would be helpful if you or Roger
can send a patch to document all the knobs for block-iscsi. It doesn't
have to be in that file, we can figure out where to add.

FYI we are in the process of making 4.7 release, so the response here
might take a bit longer. FAOD this patch is not targeting 4.7, right?

I'm not too familiar with block script so I think I will
defer this to Roger.  I've also CC'ed Ian for you.

To make a proper patch, please could you read:

http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches

Wei.


> -- 
> Steven Haigh
> 
> Email: netwiz@crc.id.au
> Web: https://www.crc.id.au
> Phone: (03) 9001 6090 - 0412 935 897

> --- block-iscsi.orig    2016-05-09 15:12:02.489495212 +1000
> +++ block-iscsi 2016-05-09 15:16:35.447480532 +1000
> @@ -31,16 +31,6 @@
>      echo $1 | sed "s/^\("$2"\)//"
>  }
>  
> -check_tools()
> -{
> -    if ! command -v iscsiadm > /dev/null 2>&1; then
> -        fatal "Unable to find iscsiadm tool"
> -    fi
> -    if [ "$multipath" = "y" ] && ! command -v multipath > /dev/null 2>&1; then
> -        fatal "Unable to find multipath"
> -    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()
> @@ -52,12 +42,18 @@
>          case $param in
>          iqn=*)
>              iqn=$(remove_label $param "iqn=")
> +            if ! command -v iscsiadm > /dev/null 2>&1; then
> +                fatal "Could not find iscsiadm tool."
> +            fi
>              ;;
>          portal=*)
>              portal=$(remove_label $param "portal=")
>              ;;
>          multipath=*)
>              multipath=$(remove_label $param "multipath=")
> +            if ! command -v multipath > /dev/null 2>&1; then
> +                fatal "Multipath selected, but no multipath tools found"
> +            fi
>              ;;
>          esac
>      done
> @@ -96,40 +92,6 @@
>      fi
>  }
> 
> -# Attaches the target $iqn in $portal and sets $dev to point to the
> -# multipath device
> -attach()
> -{
> -    do_or_die iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null
> -    find_device
> -}
> -
> -# 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
> -    iscsiadm -m session 2>&1 | grep -q "$iqn" && fatal "Device already opened"
> -    # Discover portal targets
> -    iscsiadm -m discovery -t st -p $portal 2>&1 | grep -q "$iqn" || \
> -        fatal "No matching target iqn found"
> -}
> -
> -# Attaches the device and writes xenstore backend entries to connect
> -# the device
> -add()
> -{
> -    attach
> -    write_dev $dev
> -}
> -
> -# Disconnects the device
> -remove()
> -{
> -    find_device
> -    do_or_die iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null
> -}
> -
>  command=$1
>  target=$(xenstore-read $XENBUS_PATH/params || true)
>  if [ -z "$target" ]; then
> @@ -138,15 +100,21 @@
> 
>  parse_target "$target"
> 
> -check_tools || exit 1
> -
>  case $command in
>  add)
> -    prepare
> -    add
> +    # Check if target is already opened
> +    iscsiadm -m session 2>&1 | grep -q "$iqn" && fatal "Device already opened"
> +    # Discover portal targets
> +    iscsiadm -m discovery -t st -p $portal 2>&1 | grep -q "$iqn" || \
> +        fatal "No matching target iqn found"
> +
> +    ## Login to the iSCSI target.
> +    do_or_die iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null
> +
> +    write_dev $dev
>      ;;
>  remove)
> -    remove
> +    do_or_die iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null
>      ;;
>  *)
>      exit 1 

> --- block-iscsi.orig 2016-05-09 15:16:35.447480532 +1000
> +++ block-iscsi    2016-05-05 15:43:58.222159351 +1000
> @@ -37,8 +37,7 @@
>  {
>      # set multipath default value
>      multipath="n"
> -    for param in $(echo "$1" | tr "," "\n")
> -    do
> +    for param in $(echo "$1" | tr "," "\n"); do
>          case $param in
>          iqn=*)
>              iqn=$(remove_label $param "iqn=")
> @@ -55,6 +54,15 @@
>                  fatal "Multipath selected, but no multipath tools found"
>              fi
>              ;;
> +        locktarget=*)
> +            locktarget=$(remove_label $param "locktarget=")
> +            if ! command -v sg_persist > /dev/null 2>&1; then
> +                fatal "Locking requested but no sg_persist found"
> +            fi
> +            if ! command -v gethostip > /dev/null 2>&1; then
> +                fatal "Locking requested but no gethostip found for key generation"
> +            fi
> +            ;;
>          esac
>      done
>      if [ -z "$iqn" ] || [ -z "$portal" ]; then
> @@ -92,6 +100,31 @@
>      fi
>  }
> 
> +
> +lock_device()
> +{
> +    ## Lock the iSCSI target as Exclusive Access.
> +    key=$(gethostip -x $(uname -n))
> +    if ! sg_persist -d ${dev} -o -G -S ${key} > /dev/null; then
> +        unlock_device
> +        iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null
> +        fatal "iSCSI LOCK: Failed to register with target"
> +    fi
> +    if ! sg_persist -d ${dev} -o -R -K ${key} -T 6 > /dev/null; then
> +        unlock_device
> +        iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null
> +        fatal "iSCSI LOCK: Failed to set persistent reservation"
> +    fi
> +}
> +
> +unlock_device()
> +{
> +    ## Unlock the iSCSI target.
> +    key=$(gethostip -x $(uname -n))
> +    sg_persist -d ${dev} -o -L -K ${key} -T 6 > /dev/null || true
> +    sg_persist -d ${dev} -o -G -K ${key} -S 0 > /dev/null || true
> +}
> +
>  command=$1
>  target=$(xenstore-read $XENBUS_PATH/params || true)
>  if [ -z "$target" ]; then
> @@ -110,10 +143,17 @@
> 
>      ## Login to the iSCSI target.
>      do_or_die iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null
> -
> +    find_device
> +    if [ "$locktarget" = "y" ]; then
> +        lock_device
> +    fi
>      write_dev $dev
>      ;;
>  remove)
> +    find_device
> +    if [ "$locktarget" = "y" ]; then
> +        unlock_device
> +    fi
>      do_or_die iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null
>      ;;
>  *) 

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel