[PATCH 0 of 6] Add NetBSD support for libxl

[PATCH 0 of 6] Add NetBSD support for libxl

[Roger Pau Monne]

This serie of patches adds support for NetBSD in libxl and fixes a pair of bugs found on the process. All patches can be applied individually without breaking the build of xen.

[PATCH 1 of 6] xenbackendd: fix incorrect usage of pidfile

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1316692475 -7200
# Node ID 576c0ce336acb501cbba245dc801f06edc391479
# Parent  a422e2a4451e16dc791b293f41966b842fa4781d
xenbackendd: fix incorrect usage of pidfile

Fix xenbackendd ignoring the pidfile passed through the command line.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r a422e2a4451e -r 576c0ce336ac tools/xenbackendd/xenbackendd.c
--- a/tools/xenbackendd/xenbackendd.c	Sun Sep 18 00:26:52 2011 +0100
+++ b/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:35 2011 +0200
@@ -169,7 +169,7 @@ main(int argc, char * const argv[])
 			log_file = optarg;
 			break;
 		case 'p':
-			pidfile = pidfile;
+			pidfile = optarg;
 		case 's':
 			vbd_script = optarg;
 			break;

Re: [PATCH 1 of 6] xenbackendd: fix incorrect usage of pidfile

[Ian Campbell]

On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1316692475 -7200
> # Node ID 576c0ce336acb501cbba245dc801f06edc391479
> # Parent  a422e2a4451e16dc791b293f41966b842fa4781d
> xenbackendd: fix incorrect usage of pidfile
> 
> Fix xenbackendd ignoring the pidfile passed through the command line.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r a422e2a4451e -r 576c0ce336ac tools/xenbackendd/xenbackendd.c
> --- a/tools/xenbackendd/xenbackendd.c	Sun Sep 18 00:26:52 2011 +0100
> +++ b/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:35 2011 +0200
> @@ -169,7 +169,7 @@ main(int argc, char * const argv[])
>  			log_file = optarg;
>  			break;
>  		case 'p':
> -			pidfile = pidfile;
> +			pidfile = optarg;
>  		case 's':
>  			vbd_script = optarg;
>  			break;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 1 of 6] xenbackendd: fix incorrect usage of pidfile

[Christoph Egger]

On 09/22/11 15:23, Ian Campbell wrote:
> On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
>> # HG changeset patch
>> # User Roger Pau Monne<roger.pau@entel.upc.edu>
>> # Date 1316692475 -7200
>> # Node ID 576c0ce336acb501cbba245dc801f06edc391479
>> # Parent  a422e2a4451e16dc791b293f41966b842fa4781d
>> xenbackendd: fix incorrect usage of pidfile
>>
>> Fix xenbackendd ignoring the pidfile passed through the command line.
>>
>> Signed-off-by: Roger Pau Monne<roger.pau@entel.upc.edu>
>
> Acked-by: Ian Campbell<ian.campbell@citrix.com>

Acked-by: Christoph Egger <Christoph.Egger@amd.com>

>
>>
>> diff -r a422e2a4451e -r 576c0ce336ac tools/xenbackendd/xenbackendd.c
>> --- a/tools/xenbackendd/xenbackendd.c	Sun Sep 18 00:26:52 2011 +0100
>> +++ b/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:35 2011 +0200
>> @@ -169,7 +169,7 @@ main(int argc, char * const argv[])
>>   			log_file = optarg;
>>   			break;
>>   		case 'p':
>> -			pidfile = pidfile;
>> +			pidfile = optarg;
>>   		case 's':
>>   			vbd_script = optarg;
>>   			break;
>>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[PATCH 2 of 6] hotplug: set hotplug-status to disconnected at removal

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1316692482 -7200
# Node ID 1d3830f8a7306088b49d5732b5e88a73c2bc0315
# Parent  576c0ce336acb501cbba245dc801f06edc391479
hotplug: set hotplug-status to disconnected at removal

Set the hotplug-status attribute of xenstore to disconnected when the hotplug block script has finished disconnecting the device

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 576c0ce336ac -r 1d3830f8a730 tools/hotplug/Linux/block
--- a/tools/hotplug/Linux/block	Thu Sep 22 13:54:35 2011 +0200
+++ b/tools/hotplug/Linux/block	Thu Sep 22 13:54:42 2011 +0200
@@ -321,6 +321,7 @@ mount it read-write in a guest domain."
   remove)
     case $t in 
       phy)
+		xenstore_write "$XENBUS_PATH/hotplug-status" "disconnected"
 	exit 0
 	;;
 
@@ -329,6 +330,7 @@ mount it read-write in a guest domain."
         node=$(xenstore_read "$XENBUS_PATH/node")
 	losetup -d "$node"
         release_lock "block"
+		xenstore_write "$XENBUS_PATH/hotplug-status" "disconnected"
 	exit 0
 	;;
 
diff -r 576c0ce336ac -r 1d3830f8a730 tools/hotplug/NetBSD/block
--- a/tools/hotplug/NetBSD/block	Thu Sep 22 13:54:35 2011 +0200
+++ b/tools/hotplug/NetBSD/block	Thu Sep 22 13:54:42 2011 +0200
@@ -38,6 +38,7 @@ 6)
 		echo "unknown type $xtype" >&2
 		;;
 	esac
+	xenstore-write $xpath/hotplug-status disconnected
 	xenstore-rm $xpath
 	exit 0
 	;;

Re: [PATCH 2 of 6] hotplug: set hotplug-status to disconnected at removal

[Ian Campbell]

On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1316692482 -7200
> # Node ID 1d3830f8a7306088b49d5732b5e88a73c2bc0315
> # Parent  576c0ce336acb501cbba245dc801f06edc391479
> hotplug: set hotplug-status to disconnected at removal
> 
> Set the hotplug-status attribute of xenstore to disconnected when the hotplug block script has finished disconnecting the device
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

Whitespace looks a bit screwed up. (Fair enough it already was but lets
not make things worse.

So:
Acked-by: Ian Campbell <ian.campbell@citrix.com>

but with the proviso that the following is subsequently applied too:

8<------------------------------------------------------------------

Subject: hotplug: s/^I/        / in block hotplug script

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r e077264006c5 tools/hotplug/Linux/block
--- a/tools/hotplug/Linux/block	Thu Sep 22 14:25:59 2011 +0100
+++ b/tools/hotplug/Linux/block	Thu Sep 22 14:26:41 2011 +0100
@@ -239,10 +239,10 @@ case "$command" in
 
         claim_lock "block"
         check_device_sharing "$dev" "$mode"
-	write_dev "$dev"
+        write_dev "$dev"
         release_lock "block"
-	exit 0
-	;;
+        exit 0
+        ;;
 
       file)
         # Canonicalise the file, for sharing check comparison, and the mode
@@ -254,7 +254,7 @@ case "$command" in
         claim_lock "block"
 
         # Avoid a race with the remove if the path has been deleted, or
-	# otherwise changed from "InitWait" state e.g. due to a timeout
+        # otherwise changed from "InitWait" state e.g. due to a timeout
         xenbus_state=$(xenstore_read_default "$XENBUS_PATH/state" 'unknown')
         if [ "$xenbus_state" != '2' ]
         then
@@ -308,35 +308,35 @@ mount it read-write in a guest domain."
         write_dev "$loopdev"
         release_lock "block"
         exit 0
-	;;
+        ;;
 
       "")
         claim_lock "block"
         success
         release_lock "block"
-	;;
+        ;;
     esac
     ;;
 
   remove)
     case $t in 
       phy)
-		xenstore_write "$XENBUS_PATH/hotplug-status" "disconnected"
-	exit 0
-	;;
+        xenstore_write "$XENBUS_PATH/hotplug-status" "disconnected"
+        exit 0
+        ;;
 
       file)
         claim_lock "block"
         node=$(xenstore_read "$XENBUS_PATH/node")
-	losetup -d "$node"
+        losetup -d "$node"
         release_lock "block"
-		xenstore_write "$XENBUS_PATH/hotplug-status" "disconnected"
-	exit 0
-	;;
+        xenstore_write "$XENBUS_PATH/hotplug-status" "disconnected"
+        exit 0
+        ;;
 
       "")
         exit 0
-	;;
+        ;;
     esac
     ;;

[PATCH 3 of 6] xenbackendd: pass type of block device to hotplug script

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1316692489 -7200
# Node ID 2d77cbdc816bc943e9e69ecca34ae7157079045a
# Parent  1d3830f8a7306088b49d5732b5e88a73c2bc0315
xenbackendd: pass type of block device to hotplug script

Pass the type of block device to attach to the block script instead of reading it from xenstore, since new Xen versions don't make a difference between a block device or an image.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 1d3830f8a730 -r 2d77cbdc816b tools/hotplug/NetBSD/block
--- a/tools/hotplug/NetBSD/block	Thu Sep 22 13:54:42 2011 +0200
+++ b/tools/hotplug/NetBSD/block	Thu Sep 22 13:54:49 2011 +0200
@@ -19,7 +19,7 @@ error() {
 
 xpath=$1
 xstatus=$2
-xtype=$(xenstore-read "$xpath/type")
+xtype=$3
 xparams=$(xenstore-read "$xpath/params")
 
 case $xstatus in
diff -r 1d3830f8a730 -r 2d77cbdc816b tools/xenbackendd/xenbackendd.c
--- a/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:42 2011 +0200
+++ b/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:49 2011 +0200
@@ -89,15 +89,15 @@ dodebug(const char *fmt, ...)
 }
 
 static void
-doexec(const char *cmd, const char *arg1, const char *arg2)
+doexec(const char *cmd, const char *arg1, const char *arg2, const char *arg3)
 {
-	dodebug("exec %s %s %s", cmd, arg1, arg2);
+	dodebug("exec %s %s %s %s", cmd, arg1, arg2, arg3);
 	switch(vfork()) {
 	case -1:
 		dolog(LOG_ERR, "can't vfork: %s", strerror(errno));
 		break;
 	case 0:
-		execl(cmd, cmd, arg1, arg2, NULL);
+		execl(cmd, cmd, arg1, arg2, arg3, NULL);
 		dolog(LOG_ERR, "can't exec %s: %s", cmd, strerror(errno));
 		exit(EXIT_FAILURE);
 		/* NOTREACHED */
@@ -145,11 +145,14 @@ xen_setup(void)
 int
 main(int argc, char * const argv[])
 {
+	struct stat stab;
 	char **vec;
 	unsigned int num;
 	char *s;
 	int state;
 	char *sstate;
+	char *stype;
+	char *params;
 	char *p;
 	char buf[80];
 	int type;
@@ -297,11 +300,38 @@ main(int argc, char * const argv[])
 				    strerror(errno));
 				goto next2;
 			}
-			doexec(s, vec[XS_WATCH_PATH], sstate);
+			doexec(s, vec[XS_WATCH_PATH], sstate, NULL);
 			break;
 
 		case DEVTYPE_VBD:
-			doexec(vbd_script, vec[XS_WATCH_PATH], sstate);
+			/* check if given file is a block device or a raw image */
+			snprintf(buf, sizeof(buf), "%s/params", vec[XS_WATCH_PATH]);
+			params = xs_read(xs, XBT_NULL, buf, 0);
+			if(params == NULL) {
+				dolog(LOG_ERR,
+					"Failed to read %s (%s)", buf, strerror(errno));
+				goto next2;
+			}
+			if (stat(params, &stab) < 0) {
+				dolog(LOG_ERR,
+					"Failed to get info about %s (%s)", params,
+					strerror(errno));
+				goto next3;
+			}
+			stype = NULL;
+			if (S_ISBLK(stab.st_mode))
+				stype = "phy";
+			if (S_ISREG(stab.st_mode))
+				stype = "file";
+			if (stype == NULL) {
+				dolog(LOG_ERR,
+					"Failed to attach %s (not a block device or raw image)",
+					params, strerror(errno));
+				goto next3;
+			}
+			doexec(vbd_script, vec[XS_WATCH_PATH], sstate, stype);
+next3:
+			free(params);
 			break;
 
 		default:

Re: [PATCH 3 of 6] xenbackendd: pass type of block device to hotplug script

[Ian Campbell]

On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1316692489 -7200
> # Node ID 2d77cbdc816bc943e9e69ecca34ae7157079045a
> # Parent  1d3830f8a7306088b49d5732b5e88a73c2bc0315
> xenbackendd: pass type of block device to hotplug script
> 
> Pass the type of block device to attach to the block script instead of reading it from xenstore, since new Xen versions don't make a difference between a block device or an image.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

I've not got any problem with this one, but I don't think my ACK is
worth much since it's NetBSD and I'm not really qualified, neverless:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r 1d3830f8a730 -r 2d77cbdc816b tools/hotplug/NetBSD/block
> --- a/tools/hotplug/NetBSD/block	Thu Sep 22 13:54:42 2011 +0200
> +++ b/tools/hotplug/NetBSD/block	Thu Sep 22 13:54:49 2011 +0200
> @@ -19,7 +19,7 @@ error() {
>  
>  xpath=$1
>  xstatus=$2
> -xtype=$(xenstore-read "$xpath/type")
> +xtype=$3
>  xparams=$(xenstore-read "$xpath/params")
>  
>  case $xstatus in
> diff -r 1d3830f8a730 -r 2d77cbdc816b tools/xenbackendd/xenbackendd.c
> --- a/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:42 2011 +0200
> +++ b/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:49 2011 +0200
> @@ -89,15 +89,15 @@ dodebug(const char *fmt, ...)
>  }
>  
>  static void
> -doexec(const char *cmd, const char *arg1, const char *arg2)
> +doexec(const char *cmd, const char *arg1, const char *arg2, const char *arg3)
>  {
> -	dodebug("exec %s %s %s", cmd, arg1, arg2);
> +	dodebug("exec %s %s %s %s", cmd, arg1, arg2, arg3);
>  	switch(vfork()) {
>  	case -1:
>  		dolog(LOG_ERR, "can't vfork: %s", strerror(errno));
>  		break;
>  	case 0:
> -		execl(cmd, cmd, arg1, arg2, NULL);
> +		execl(cmd, cmd, arg1, arg2, arg3, NULL);
>  		dolog(LOG_ERR, "can't exec %s: %s", cmd, strerror(errno));
>  		exit(EXIT_FAILURE);
>  		/* NOTREACHED */
> @@ -145,11 +145,14 @@ xen_setup(void)
>  int
>  main(int argc, char * const argv[])
>  {
> +	struct stat stab;
>  	char **vec;
>  	unsigned int num;
>  	char *s;
>  	int state;
>  	char *sstate;
> +	char *stype;
> +	char *params;
>  	char *p;
>  	char buf[80];
>  	int type;
> @@ -297,11 +300,38 @@ main(int argc, char * const argv[])
>  				    strerror(errno));
>  				goto next2;
>  			}
> -			doexec(s, vec[XS_WATCH_PATH], sstate);
> +			doexec(s, vec[XS_WATCH_PATH], sstate, NULL);
>  			break;
>  
>  		case DEVTYPE_VBD:
> -			doexec(vbd_script, vec[XS_WATCH_PATH], sstate);
> +			/* check if given file is a block device or a raw image */
> +			snprintf(buf, sizeof(buf), "%s/params", vec[XS_WATCH_PATH]);
> +			params = xs_read(xs, XBT_NULL, buf, 0);
> +			if(params == NULL) {
> +				dolog(LOG_ERR,
> +					"Failed to read %s (%s)", buf, strerror(errno));
> +				goto next2;
> +			}
> +			if (stat(params, &stab) < 0) {
> +				dolog(LOG_ERR,
> +					"Failed to get info about %s (%s)", params,
> +					strerror(errno));
> +				goto next3;
> +			}
> +			stype = NULL;
> +			if (S_ISBLK(stab.st_mode))
> +				stype = "phy";
> +			if (S_ISREG(stab.st_mode))
> +				stype = "file";
> +			if (stype == NULL) {
> +				dolog(LOG_ERR,
> +					"Failed to attach %s (not a block device or raw image)",
> +					params, strerror(errno));
> +				goto next3;
> +			}
> +			doexec(vbd_script, vec[XS_WATCH_PATH], sstate, stype);
> +next3:
> +			free(params);
>  			break;
>  
>  		default:
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 3 of 6] xenbackendd: pass type of block device to hotplug script

[Christoph Egger]

On 09/22/11 15:35, Ian Campbell wrote:
> On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
>> # HG changeset patch
>> # User Roger Pau Monne<roger.pau@entel.upc.edu>
>> # Date 1316692489 -7200
>> # Node ID 2d77cbdc816bc943e9e69ecca34ae7157079045a
>> # Parent  1d3830f8a7306088b49d5732b5e88a73c2bc0315
>> xenbackendd: pass type of block device to hotplug script
>>
>> Pass the type of block device to attach to the block script instead of reading it from xenstore, since new Xen versions don't make a difference between a block device or an image.
>>
>> Signed-off-by: Roger Pau Monne<roger.pau@entel.upc.edu>
>
> I've not got any problem with this one, but I don't think my ACK is
> worth much since it's NetBSD and I'm not really qualified, neverless:
>
> Acked-by: Ian Campbell<ian.campbell@citrix.com>

Acked-by: Christoph Egger <Christoph.Egger@amd.com>

>>
>> diff -r 1d3830f8a730 -r 2d77cbdc816b tools/hotplug/NetBSD/block
>> --- a/tools/hotplug/NetBSD/block	Thu Sep 22 13:54:42 2011 +0200
>> +++ b/tools/hotplug/NetBSD/block	Thu Sep 22 13:54:49 2011 +0200
>> @@ -19,7 +19,7 @@ error() {
>>
>>   xpath=$1
>>   xstatus=$2
>> -xtype=$(xenstore-read "$xpath/type")
>> +xtype=$3
>>   xparams=$(xenstore-read "$xpath/params")
>>
>>   case $xstatus in
>> diff -r 1d3830f8a730 -r 2d77cbdc816b tools/xenbackendd/xenbackendd.c
>> --- a/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:42 2011 +0200
>> +++ b/tools/xenbackendd/xenbackendd.c	Thu Sep 22 13:54:49 2011 +0200
>> @@ -89,15 +89,15 @@ dodebug(const char *fmt, ...)
>>   }
>>
>>   static void
>> -doexec(const char *cmd, const char *arg1, const char *arg2)
>> +doexec(const char *cmd, const char *arg1, const char *arg2, const char *arg3)
>>   {
>> -	dodebug("exec %s %s %s", cmd, arg1, arg2);
>> +	dodebug("exec %s %s %s %s", cmd, arg1, arg2, arg3);
>>   	switch(vfork()) {
>>   	case -1:
>>   		dolog(LOG_ERR, "can't vfork: %s", strerror(errno));
>>   		break;
>>   	case 0:
>> -		execl(cmd, cmd, arg1, arg2, NULL);
>> +		execl(cmd, cmd, arg1, arg2, arg3, NULL);
>>   		dolog(LOG_ERR, "can't exec %s: %s", cmd, strerror(errno));
>>   		exit(EXIT_FAILURE);
>>   		/* NOTREACHED */
>> @@ -145,11 +145,14 @@ xen_setup(void)
>>   int
>>   main(int argc, char * const argv[])
>>   {
>> +	struct stat stab;
>>   	char **vec;
>>   	unsigned int num;
>>   	char *s;
>>   	int state;
>>   	char *sstate;
>> +	char *stype;
>> +	char *params;
>>   	char *p;
>>   	char buf[80];
>>   	int type;
>> @@ -297,11 +300,38 @@ main(int argc, char * const argv[])
>>   				    strerror(errno));
>>   				goto next2;
>>   			}
>> -			doexec(s, vec[XS_WATCH_PATH], sstate);
>> +			doexec(s, vec[XS_WATCH_PATH], sstate, NULL);
>>   			break;
>>
>>   		case DEVTYPE_VBD:
>> -			doexec(vbd_script, vec[XS_WATCH_PATH], sstate);
>> +			/* check if given file is a block device or a raw image */
>> +			snprintf(buf, sizeof(buf), "%s/params", vec[XS_WATCH_PATH]);
>> +			params = xs_read(xs, XBT_NULL, buf, 0);
>> +			if(params == NULL) {
>> +				dolog(LOG_ERR,
>> +					"Failed to read %s (%s)", buf, strerror(errno));
>> +				goto next2;
>> +			}
>> +			if (stat(params,&stab)<  0) {
>> +				dolog(LOG_ERR,
>> +					"Failed to get info about %s (%s)", params,
>> +					strerror(errno));
>> +				goto next3;
>> +			}
>> +			stype = NULL;
>> +			if (S_ISBLK(stab.st_mode))
>> +				stype = "phy";
>> +			if (S_ISREG(stab.st_mode))
>> +				stype = "file";
>> +			if (stype == NULL) {
>> +				dolog(LOG_ERR,
>> +					"Failed to attach %s (not a block device or raw image)",
>> +					params, strerror(errno));
>> +				goto next3;
>> +			}
>> +			doexec(vbd_script, vec[XS_WATCH_PATH], sstate, stype);
>> +next3:
>> +			free(params);
>>   			break;
>>
>>   		default:
>>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[PATCH 4 of 6] libxl: fix for libxl not waiting for devices to disconnect

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1316692495 -7200
# Node ID d5cca9e3b5d3576073a543d0305e8f0d9dc9beeb
# Parent  2d77cbdc816bc943e9e69ecca34ae7157079045a
libxl: fix for libxl not waiting for devices to disconnect

libxl was ignoring the timeout and the number of devices to wait before destroying them.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 2d77cbdc816b -r d5cca9e3b5d3 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Sep 22 13:54:49 2011 +0200
+++ b/tools/libxl/libxl_device.c	Thu Sep 22 13:54:55 2011 +0200
@@ -422,6 +422,9 @@ static int wait_for_dev_destroy(libxl__g
             }
             free(l1);
         }
+    } else {
+        /* timeout reached */
+        rc = 0;
     }
     return rc;
 }
@@ -482,7 +485,7 @@ int libxl__devices_destroy(libxl__gc *gc
         tv.tv_usec = 0;
         while (n_watches > 0) {
             if (wait_for_dev_destroy(gc, &tv)) {
-                break;
+                continue;
             } else {
                 n_watches--;
             }

Re: [PATCH 4 of 6] libxl: fix for libxl not waiting for devices to disconnect

[Ian Campbell]

On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1316692495 -7200
> # Node ID d5cca9e3b5d3576073a543d0305e8f0d9dc9beeb
> # Parent  2d77cbdc816bc943e9e69ecca34ae7157079045a
> libxl: fix for libxl not waiting for devices to disconnect
> 
> libxl was ignoring the timeout and the number of devices to wait before destroying them.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Although I think it would read more naturally to set rc = 0 initially
and set it to 1 on success, IYSWIM.

> 
> diff -r 2d77cbdc816b -r d5cca9e3b5d3 tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c	Thu Sep 22 13:54:49 2011 +0200
> +++ b/tools/libxl/libxl_device.c	Thu Sep 22 13:54:55 2011 +0200
> @@ -422,6 +422,9 @@ static int wait_for_dev_destroy(libxl__g
>              }
>              free(l1);
>          }
> +    } else {
> +        /* timeout reached */
> +        rc = 0;
>      }
>      return rc;
>  }
> @@ -482,7 +485,7 @@ int libxl__devices_destroy(libxl__gc *gc
>          tv.tv_usec = 0;
>          while (n_watches > 0) {
>              if (wait_for_dev_destroy(gc, &tv)) {
> -                break;
> +                continue;
>              } else {
>                  n_watches--;
>              }
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 5 of 6] libxl: add support for NetBSD to use disk images as a PHY backend

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1316692503 -7200
# Node ID a5c9a6083bef9d4789a0194389db234740cd6005
# Parent  d5cca9e3b5d3576073a543d0305e8f0d9dc9beeb
libxl: add support for NetBSD to use disk images as a PHY backend

Add a NetBSD special case to use images as PHY disk devices, which can be attached using the regular block hotplug script.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r d5cca9e3b5d3 -r a5c9a6083bef tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Sep 22 13:54:55 2011 +0200
+++ b/tools/libxl/libxl_device.c	Thu Sep 22 13:55:03 2011 +0200
@@ -136,15 +136,20 @@ static int disk_try_backend(disk_try_bac
               a->disk->format == LIBXL_DISK_FORMAT_EMPTY)) {
             goto bad_format;
         }
-        if (a->disk->format != LIBXL_DISK_FORMAT_EMPTY &&
-            !S_ISBLK(a->stab.st_mode)) {
-            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
-                       " unsuitable as phys path not a block device",
-                       a->disk->vdev);
-            return 0;
-        }
-
-        return backend;
+        if (S_ISBLK(a->stab.st_mode))
+            return backend;
+#ifdef HAVE_PHY_BACKEND_FILE_SUPPORT
+        if (S_ISREG(a->stab.st_mode))
+            return backend;
+        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
+            " unsuitable as phys path not a block device or"
+            " raw image", a->disk->vdev);
+#else
+        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
+            " unsuitable as phys path not a block device",
+            a->disk->vdev);
+#endif
+        return 0;
 
     case LIBXL_DISK_BACKEND_TAP:
         if (!libxl__blktap_enabled(a->gc)) {
diff -r d5cca9e3b5d3 -r a5c9a6083bef tools/libxl/libxl_osdeps.h
--- a/tools/libxl/libxl_osdeps.h	Thu Sep 22 13:54:55 2011 +0200
+++ b/tools/libxl/libxl_osdeps.h	Thu Sep 22 13:55:03 2011 +0200
@@ -25,6 +25,7 @@
 
 #if defined(__NetBSD__) || defined(__OpenBSD__)
 #include <util.h>
+#define HAVE_PHY_BACKEND_FILE_SUPPORT
 #elif defined(__linux__)
 #include <pty.h>
 #elif defined(__sun__)

Re: [PATCH 5 of 6] libxl: add support for NetBSD to use disk images as a PHY backend

[Ian Campbell]

On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1316692503 -7200
> # Node ID a5c9a6083bef9d4789a0194389db234740cd6005
> # Parent  d5cca9e3b5d3576073a543d0305e8f0d9dc9beeb
> libxl: add support for NetBSD to use disk images as a PHY backend
> 
> Add a NetBSD special case to use images as PHY disk devices, which can be attached using the regular block hotplug script.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I think the "return 0"s in this function ought to be
LIBXL_DISK_BACKEND_UNKNOWN for clarity but that's a pre-existing
problem.

> 
> diff -r d5cca9e3b5d3 -r a5c9a6083bef tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c	Thu Sep 22 13:54:55 2011 +0200
> +++ b/tools/libxl/libxl_device.c	Thu Sep 22 13:55:03 2011 +0200
> @@ -136,15 +136,20 @@ static int disk_try_backend(disk_try_bac
>                a->disk->format == LIBXL_DISK_FORMAT_EMPTY)) {
>              goto bad_format;
>          }
> -        if (a->disk->format != LIBXL_DISK_FORMAT_EMPTY &&
> -            !S_ISBLK(a->stab.st_mode)) {
> -            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
> -                       " unsuitable as phys path not a block device",
> -                       a->disk->vdev);
> -            return 0;
> -        }
> -
> -        return backend;
> +        if (S_ISBLK(a->stab.st_mode))
> +            return backend;
> +#ifdef HAVE_PHY_BACKEND_FILE_SUPPORT
> +        if (S_ISREG(a->stab.st_mode))
> +            return backend;
> +        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
> +            " unsuitable as phys path not a block device or"
> +            " raw image", a->disk->vdev);
> +#else
> +        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
> +            " unsuitable as phys path not a block device",
> +            a->disk->vdev);
> +#endif
> +        return 0;
>  
>      case LIBXL_DISK_BACKEND_TAP:
>          if (!libxl__blktap_enabled(a->gc)) {
> diff -r d5cca9e3b5d3 -r a5c9a6083bef tools/libxl/libxl_osdeps.h
> --- a/tools/libxl/libxl_osdeps.h	Thu Sep 22 13:54:55 2011 +0200
> +++ b/tools/libxl/libxl_osdeps.h	Thu Sep 22 13:55:03 2011 +0200
> @@ -25,6 +25,7 @@
>  
>  #if defined(__NetBSD__) || defined(__OpenBSD__)
>  #include <util.h>
> +#define HAVE_PHY_BACKEND_FILE_SUPPORT
>  #elif defined(__linux__)
>  #include <pty.h>
>  #elif defined(__sun__)
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 6 of 6] libxl: use hotplug-status to synchronize the destruction of block devices

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1316692512 -7200
# Node ID 9e9c68768676b0e1c9c76599e00d0922c6184602
# Parent  a5c9a6083bef9d4789a0194389db234740cd6005
libxl: use hotplug-status to synchronize the destruction of block devices

Use hotplug-status to synchronize the destruction of vbd devices instead of state, which did not reflect when the hotplug script had been executed.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r a5c9a6083bef -r 9e9c68768676 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Sep 22 13:55:03 2011 +0200
+++ b/tools/libxl/libxl_device.c	Thu Sep 22 13:55:12 2011 +0200
@@ -371,14 +371,26 @@ int libxl__device_destroy(libxl__gc *gc,
     libxl_ctx *ctx = libxl__gc_owner(gc);
     xs_transaction_t t;
     char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+    char *hotplug_path = libxl__sprintf(gc, "%s/hotplug-status", be_path);
     char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+    char *hotplug = libxl__xs_read(gc, XBT_NULL, hotplug_path);
     int rc = 0;
 
     if (!state)
         goto out;
-    if (atoi(state) != 4) {
-        xs_rm(ctx->xsh, XBT_NULL, be_path);
-        goto out;
+
+    if (!strstr(be_path, string_of_kinds[DEVICE_VBD])) {
+        if (atoi(state) != 4) {
+            xs_rm(ctx->xsh, XBT_NULL, be_path);
+            goto out;
+        }
+    } else {
+        if (!hotplug)
+            goto out;
+        if (!strcmp(hotplug, "disconnected")) {
+            xs_rm(ctx->xsh, XBT_NULL, be_path);
+            goto out;
+        }
     }
 
 retry_transaction:
@@ -394,8 +406,13 @@ retry_transaction:
         }
     }
     if (!force) {
-        xs_watch(ctx->xsh, state_path, be_path);
-        rc = 1;
+        if (strstr(be_path, string_of_kinds[DEVICE_VBD])) {
+            xs_watch(ctx->xsh, hotplug_path, be_path);
+            rc = 1;
+        } else {
+            xs_watch(ctx->xsh, state_path, be_path);
+            rc = 1;
+        }
     } else {
         xs_rm(ctx->xsh, XBT_NULL, be_path);
     }
@@ -410,6 +427,7 @@ static int wait_for_dev_destroy(libxl__g
     unsigned int n;
     fd_set rfds;
     char **l1 = NULL;
+    char *state, *hotplug;
 
     rc = 1;
     nfds = xs_fileno(ctx->xsh) + 1;
@@ -418,12 +436,26 @@ static int wait_for_dev_destroy(libxl__g
     if (select(nfds, &rfds, NULL, NULL, tv) > 0) {
         l1 = xs_read_watch(ctx->xsh, &n);
         if (l1 != NULL) {
-            char *state = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
-            if (!state || atoi(state) == 6) {
-                xs_unwatch(ctx->xsh, l1[0], l1[1]);
-                xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
-                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Destroyed device backend at %s", l1[XS_WATCH_TOKEN]);
-                rc = 0;
+            if (strstr(l1[XS_WATCH_PATH], "hotplug-status")) {
+                hotplug = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
+                if (!hotplug || !strcmp(hotplug, "disconnected")) {
+                    xs_unwatch(ctx->xsh, l1[0], l1[1]);
+                    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+                    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+                        "Destroyed device backend at %s hotplug-status: %s",
+                        l1[XS_WATCH_TOKEN], hotplug);
+                    rc = 0;
+                }
+            } else {
+                state = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
+                if (!state || atoi(state) == 6) {
+                    xs_unwatch(ctx->xsh, l1[0], l1[1]);
+                    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+                    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+                        "Destroyed device backend at %s",
+                        l1[XS_WATCH_TOKEN]);
+                    rc = 0;
+                }
             }
             free(l1);
         }

Re: [PATCH 6 of 6] libxl: use hotplug-status to synchronize the destruction of block devices

[Ian Campbell]

On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1316692512 -7200
> # Node ID 9e9c68768676b0e1c9c76599e00d0922c6184602
> # Parent  a5c9a6083bef9d4789a0194389db234740cd6005
> libxl: use hotplug-status to synchronize the destruction of block devices
> 
> Use hotplug-status to synchronize the destruction of vbd devices instead of state, which did not reflect when the hotplug script had been executed.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

I wonder if we can remove the hard coding of the VBD special case
somehow? For example an ops struct in an array mapping DEVICE_* to
	int (*watch_for_disconnect)(relevant arguments)
	int (*has_disconnected)(relevant arguments)
which would have two implementations, one checking state and the other
hotplug-status.

That leaves the change in wait_for_dev_destroy which unfortunately
doesn't have the context to do much else than what you've done. Perhaps
that's ok as is until I can finish of the patch series I have which
fixes that aspect. I suppose a function pointer could be encoded in the
XS_WATCH_TOKEN and sscanf'd back out again...

> 
> diff -r a5c9a6083bef -r 9e9c68768676 tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c	Thu Sep 22 13:55:03 2011 +0200
> +++ b/tools/libxl/libxl_device.c	Thu Sep 22 13:55:12 2011 +0200
> @@ -371,14 +371,26 @@ int libxl__device_destroy(libxl__gc *gc,
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      xs_transaction_t t;
>      char *state_path = libxl__sprintf(gc, "%s/state", be_path);
> +    char *hotplug_path = libxl__sprintf(gc, "%s/hotplug-status", be_path);
>      char *state = libxl__xs_read(gc, XBT_NULL, state_path);
> +    char *hotplug = libxl__xs_read(gc, XBT_NULL, hotplug_path);
>      int rc = 0;
>  
>      if (!state)
>          goto out;
> -    if (atoi(state) != 4) {
> -        xs_rm(ctx->xsh, XBT_NULL, be_path);
> -        goto out;
> +
> +    if (!strstr(be_path, string_of_kinds[DEVICE_VBD])) {
> +        if (atoi(state) != 4) {
> +            xs_rm(ctx->xsh, XBT_NULL, be_path);
> +            goto out;
> +        }
> +    } else {
> +        if (!hotplug)
> +            goto out;
> +        if (!strcmp(hotplug, "disconnected")) {
> +            xs_rm(ctx->xsh, XBT_NULL, be_path);
> +            goto out;
> +        }
>      }
>  
>  retry_transaction:
> @@ -394,8 +406,13 @@ retry_transaction:
>          }
>      }
>      if (!force) {
> -        xs_watch(ctx->xsh, state_path, be_path);
> -        rc = 1;
> +        if (strstr(be_path, string_of_kinds[DEVICE_VBD])) {
> +            xs_watch(ctx->xsh, hotplug_path, be_path);
> +            rc = 1;
> +        } else {
> +            xs_watch(ctx->xsh, state_path, be_path);
> +            rc = 1;
> +        }
>      } else {
>          xs_rm(ctx->xsh, XBT_NULL, be_path);
>      }
> @@ -410,6 +427,7 @@ static int wait_for_dev_destroy(libxl__g
>      unsigned int n;
>      fd_set rfds;
>      char **l1 = NULL;
> +    char *state, *hotplug;
>  
>      rc = 1;
>      nfds = xs_fileno(ctx->xsh) + 1;
> @@ -418,12 +436,26 @@ static int wait_for_dev_destroy(libxl__g
>      if (select(nfds, &rfds, NULL, NULL, tv) > 0) {
>          l1 = xs_read_watch(ctx->xsh, &n);
>          if (l1 != NULL) {
> -            char *state = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
> -            if (!state || atoi(state) == 6) {
> -                xs_unwatch(ctx->xsh, l1[0], l1[1]);
> -                xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
> -                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Destroyed device backend at %s", l1[XS_WATCH_TOKEN]);
> -                rc = 0;
> +            if (strstr(l1[XS_WATCH_PATH], "hotplug-status")) {
> +                hotplug = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
> +                if (!hotplug || !strcmp(hotplug, "disconnected")) {
> +                    xs_unwatch(ctx->xsh, l1[0], l1[1]);
> +                    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
> +                    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> +                        "Destroyed device backend at %s hotplug-status: %s",
> +                        l1[XS_WATCH_TOKEN], hotplug);
> +                    rc = 0;
> +                }
> +            } else {
> +                state = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
> +                if (!state || atoi(state) == 6) {
> +                    xs_unwatch(ctx->xsh, l1[0], l1[1]);
> +                    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
> +                    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> +                        "Destroyed device backend at %s",
> +                        l1[XS_WATCH_TOKEN]);
> +                    rc = 0;
> +                }
>              }
>              free(l1);
>          }
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 6 of 6] libxl: use hotplug-status to synchronize the destruction of block devices

[Roger Pau Monné]

> I wonder if we can remove the hard coding of the VBD special case
> somehow? For example an ops struct in an array mapping DEVICE_* to
>        int (*watch_for_disconnect)(relevant arguments)
>        int (*has_disconnected)(relevant arguments)
> which would have two implementations, one checking state and the other
> hotplug-status.

This would certainly make the code much cleaner and easy to understand.

> That leaves the change in wait_for_dev_destroy which unfortunately
> doesn't have the context to do much else than what you've done. Perhaps
> that's ok as is until I can finish of the patch series I have which
> fixes that aspect. I suppose a function pointer could be encoded in the
> XS_WATCH_TOKEN and sscanf'd back out again...

This patch it's the best way I came up with to fix the issue without
modifying the structure of the code much. If you have any other ideas,
I'm glad to try to implement them. Anyway, I will try to implement
something similar to what you exposed above and post a patch.

Regards, Roger.