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

2016-05-15 Thread Steven Haigh 

On 2016-05-12 21:02, Wei Liu wrote:

Hi Steven

On Mon, May 09, 2016 at 02:22:48PM +1000, 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']


You seem to suggest an extension (locktarget) to disk spec as well but
you patch doesn't contain modification to
docs/txt/misc/xl-disk-configuration.txt.


Correct. There is no documentation for the existing block-iscsi script 
within xl-disk-configuration.txt.


In fact, there is no mention at all regarding block-iscsi in any of the 
documentation that I can see.


--
Steven Haigh

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

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

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

2016-05-12 Thread Wei Liu 
Hi Steven

On Mon, May 09, 2016 at 02:22:48PM +1000, 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']

You seem to suggest an extension (locktarget) to disk spec as well but
you patch doesn't contain modification to
docs/txt/misc/xl-disk-configuration.txt.

Wei.

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

[Xen-devel] [PATCH] v3 - Add exclusive locking option to block-iscsi

2016-05-08 Thread Steven Haigh 

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 

(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)


--
Steven Haigh

Email: net...@crc.id.au
Web: https://www.crc.id.au
Phone: (03) 9001 6090 - 0412 935 897--- block-iscsi.orig2016-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 2016-05-09 15:16:35.447480532 +1000
+++ block-iscsi-lock2016-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