Skip to contentSkip to conversation

Add NetBSD support for libxl

PATCH New Roger Pau Monne 3 participants 16 messages Sep 22, 2011 – Sep 23, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add NetBSD support for libxl

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.

#1 lore raw

PATCH 1/1 xenbackendd: fix incorrect usage of pidfile

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1316692475 -7200
# Node ID 576c0ce336acb501cbba245dc801f06edc391479 # Parent a422e2a4451e16dc791b293f41966b842fa4781d
xenbackendd: fix incorrect usage of pidfile

Fix xenbackendd ignoring the pidfile passed through the command line.

tools/xenbackendd/xenbackendd.c +1 -1
diff -r a422e2a4451e -r 576c0ce336ac tools/xenbackendd/xenbackendd.c
@@ -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;
#4 lore raw

PATCH 1/1 hotplug: set hotplug-status to disconnected at removal

In reply to Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # 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

tools/hotplug/Linux/block +2 -0
diff -r 576c0ce336ac -r 1d3830f8a730 tools/hotplug/Linux/block
@@ -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
 	;;
 
tools/hotplug/NetBSD/block +1 -0
diff -r 576c0ce336ac -r 1d3830f8a730 tools/hotplug/NetBSD/block
@@ -38,6 +38,7 @@ 6)
 		echo "unknown type $xtype" >&2
 		;;
 	esac
+	xenstore-write $xpath/hotplug-status disconnected
 	xenstore-rm $xpath
 	exit 0
 	;;
#6 lore raw

PATCH 1/1 xenbackendd: pass type of block device to hotplug script

In reply to Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # 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.

tools/hotplug/NetBSD/block +1 -1
diff -r 1d3830f8a730 -r 2d77cbdc816b tools/hotplug/NetBSD/block
@@ -19,7 +19,7 @@ error() {
 
 xpath=$1
 xstatus=$2
-xtype=$(xenstore-read "$xpath/type")
+xtype=$3
 xparams=$(xenstore-read "$xpath/params")
 
 case $xstatus in
tools/xenbackendd/xenbackendd.c +35 -5
diff -r 1d3830f8a730 -r 2d77cbdc816b tools/xenbackendd/xenbackendd.c
@@ -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:
#9 lore raw

PATCH 1/1 libxl: fix for libxl not waiting for devices to disconnect

In reply to Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # 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.

tools/libxl/libxl_device.c +4 -1
diff -r 2d77cbdc816b -r d5cca9e3b5d3 tools/libxl/libxl_device.c
@@ -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--;
             }
#11 lore raw

PATCH 1/1 libxl: add support for NetBSD to use disk images as a PHY backend

In reply to Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]>
# 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.

tools/libxl/libxl_device.c +14 -9
diff -r d5cca9e3b5d3 -r a5c9a6083bef tools/libxl/libxl_device.c
@@ -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)) {
tools/libxl/libxl_osdeps.h +1 -0
diff -r d5cca9e3b5d3 -r a5c9a6083bef tools/libxl/libxl_osdeps.h
@@ -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__)
#13 lore raw

PATCH 1/1 libxl: use hotplug-status to synchronize the destruction of block devices

In reply to Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]>
# 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.

tools/libxl/libxl_device.c +43 -11
diff -r a5c9a6083bef -r 9e9c68768676 tools/libxl/libxl_device.c
@@ -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);
         }
Acked-by on v1 1/1
#2 lore raw

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

In reply to Roger Pau Monne
Quoted 10 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1316692475 -7200
# Node ID 576c0ce336acb501cbba245dc801f06edc391479 # Parent a422e2a4451e16dc791b293f41966b842fa4781d
xenbackendd: fix incorrect usage of pidfile

Fix xenbackendd ignoring the pidfile passed through the command line.

Quoted 18 lines

diff -r a422e2a4451e -r 576c0ce336ac tools/xenbackendd/xenbackendd.c

tools/xenbackendd/xenbackendd.c +1 -1
@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Acked-by on v1 1/1
#5 lore raw

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

In reply to Roger Pau Monne
Quoted 10 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # 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

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

So:

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

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

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

tools/hotplug/Linux/block +14 -14
diff -r e077264006c5 tools/hotplug/Linux/block
@@ -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
     ;;
Acked-by on v1 1/1
#10 lore raw

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

In reply to Roger Pau Monne
Quoted 10 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # 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.

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

Quoted 28 lines

diff -r 2d77cbdc816b -r d5cca9e3b5d3 tools/libxl/libxl_device.c

tools/libxl/libxl_device.c +4 -1
@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Acked-by on v1 1/1
#7 lore raw

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

In reply to Roger Pau Monne
Quoted 10 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]> # 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.

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:

Quoted 96 lines

diff -r 1d3830f8a730 -r 2d77cbdc816b tools/hotplug/NetBSD/block

tools/hotplug/NetBSD/block +1 -1
@@ -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

tools/xenbackendd/xenbackendd.c +35 -5
@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Acked-by on v1 1/1
#12 lore raw

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

In reply to Roger Pau Monne
Quoted 10 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]>
# 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.

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.

Quoted 50 lines

diff -r d5cca9e3b5d3 -r a5c9a6083bef tools/libxl/libxl_device.c

tools/libxl/libxl_device.c +14 -9
@@ -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

tools/libxl/libxl_osdeps.h +1 -0
@@ -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
[email protected]
http://lists.xensource.com/xen-devel

#14 lore raw

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

In reply to Roger Pau Monne
Quoted 10 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne <[email protected]>
# 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.

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...

Quoted 96 lines

diff -r a5c9a6083bef -r 9e9c68768676 tools/libxl/libxl_device.c

tools/libxl/libxl_device.c +43 -11
@@ -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
[email protected]
http://lists.xensource.com/xen-devel

#3 lore raw

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

In reply to Ian Campbell
Quoted 8 lines · Ian Campbell
Quoted 7 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne<[email protected]> # Date 1316692475 -7200
# Node ID 576c0ce336acb501cbba245dc801f06edc391479 # Parent a422e2a4451e16dc791b293f41966b842fa4781d
xenbackendd: fix incorrect usage of pidfile

Quoted 5 lines
Quoted 3 lines

Fix xenbackendd ignoring the pidfile passed through the command line.

Quoted 15 lines
Quoted 15 lines

diff -r a422e2a4451e -r 576c0ce336ac tools/xenbackendd/xenbackendd.c

tools/xenbackendd/xenbackendd.c +1 -1
@@ -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;
Signature · 6 lines
---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
#8 lore raw

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

In reply to Ian Campbell
Quoted 8 lines · Ian Campbell
Quoted 7 lines · Roger Pau Monne

# HG changeset patch
# User Roger Pau Monne<[email protected]> # Date 1316692489 -7200
# Node ID 2d77cbdc816bc943e9e69ecca34ae7157079045a # Parent 1d3830f8a7306088b49d5732b5e88a73c2bc0315
xenbackendd: pass type of block device to hotplug script

Quoted 8 lines
Quoted 3 lines

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.

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:

Quoted 92 lines
Quoted 92 lines

diff -r 1d3830f8a730 -r 2d77cbdc816b tools/hotplug/NetBSD/block

tools/hotplug/NetBSD/block +1 -1
@@ -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

tools/xenbackendd/xenbackendd.c +35 -5
@@ -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:
Signature · 6 lines
---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
#15 lore raw

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

In reply to Ian Campbell
Quoted 6 lines

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.

Quoted 5 lines

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.