[PATCH 00 of 29 RFC] libxl: move device plug/unplug to a separate daemon

[PATCH 00 of 29 RFC] libxl: move device plug/unplug to a separate daemon

[Roger Pau Monne]

This is the first attempt to delegate hotplug device plug/unplug to a 
separate daemon. This will also allow to plug/unplug devices from a 
domain different than Dom0, usually called a "driver domain".

This series contain a mix of bug fixes, mainly for device 
unplug/destroy synchronization, and a set of new functions that 
describe a new protocol to be used when delegating the plug/unplug of 
devices to a daemon, called xldeviced (xl-device-daemon).

As a very important note, I would like to add that this has only been 
tested with PV DomUs, and that qdisk support is missing (xldeviced is 
not prepared to launch a qemu-dm to support qdisk devices). I will 
work on that, but I think there's already a lot of stuff on this that 
needs proper review.

The series has the following order:

 * patches 1-15: set everything necessary to execute hotplug scripts 
   from libxl directly.

 * patches 16-25: add necessary libxl functions to delegate hotplug 
   addition to a different daemon

 * patches 26-29: introduce xldeviced and start using it by default.

Patch 20 contains a good description of the interaction between xl and 
xldeviced, and the xenstore entries used to accomplish this task.

[PATCH 01 of 29 RFC] libxl: Atomicaly check backend state and set it to 5 at device_remove

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1326548456 -3600
# Node ID e38c059da393c3fa2ee222d2e90275ca88b85d4f
# Parent  e2722b24dc0962de37215320b05d1bb7c4c42864
libxl: Atomicaly check backend state and set it to 5 at device_remove

libxl__device_remove was setting backend state to 5, which could
create a race condition, since the previous check for state != 4 and
setting state to 5 was not done inside the same transaction, so the
kernel could change the state to 6 in the space between the check for
state != 4 and setting it to 5.

The state != 4 check and setting it to 5 should happen in the same
transaction, to assure that nobody is modifying it behind our back.

Changes since v1:

 * Do the check and set in the same transaction, instead of removing
   the set state to 5.

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

diff -r e2722b24dc09 -r e38c059da393 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Jan 26 17:43:31 2012 +0000
+++ b/tools/libxl/libxl_device.c	Sat Jan 14 14:40:56 2012 +0100
@@ -433,19 +433,23 @@ int libxl__device_remove(libxl__gc *gc, 
     xs_transaction_t t;
     char *be_path = libxl__device_backend_path(gc, dev);
     char *state_path = libxl__sprintf(gc, "%s/state", be_path);
-    char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+    char *state;
     int rc = 0;
 
-    if (!state)
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+    state = libxl__xs_read(gc, t, state_path);
+    if (!state) {
+        xs_transaction_end(ctx->xsh, t, 0);
         goto out;
+    }
     if (atoi(state) != 4) {
+        xs_transaction_end(ctx->xsh, t, 0);
         libxl__device_destroy_tapdisk(gc, be_path);
         xs_rm(ctx->xsh, XBT_NULL, be_path);
         goto out;
     }
 
-retry_transaction:
-    t = xs_transaction_start(ctx->xsh);
     xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", strlen("0"));
     xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
     if (!xs_transaction_end(ctx->xsh, t, 0)) {

[PATCH 02 of 29 RFC] hotplug/block: get the type of block device from file path (NetBSD)

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1317386335 -7200
# Node ID 9a8ccac7b5986622f76fb8c501d2ef500f2d77a1
# Parent  e38c059da393c3fa2ee222d2e90275ca88b85d4f
hotplug/block: get the type of block device from file path (NetBSD)

Guess the type of block device to attach based on the file name
present in 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 e38c059da393 -r 9a8ccac7b598 tools/hotplug/NetBSD/block
--- a/tools/hotplug/NetBSD/block	Sat Jan 14 14:40:56 2012 +0100
+++ b/tools/hotplug/NetBSD/block	Fri Sep 30 14:38:55 2011 +0200
@@ -19,9 +19,14 @@ error() {
 
 xpath=$1
 xstatus=$2
-xtype=$(xenstore-read "$xpath/type")
 xparams=$(xenstore-read "$xpath/params")
 
+if [ -f $xparams ]; then
+	xtype="file"
+else
+	xtype="phy"
+fi
+
 case $xstatus in
 6)
 	# device removed

[PATCH 03 of 29 RFC] libxl: allow libxl__exec to take a parameter containing the env variables

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328175734 -3600
# Node ID b380ea895013853efd335c3fba4fe22f27ce1de1
# Parent  9a8ccac7b5986622f76fb8c501d2ef500f2d77a1
libxl: allow libxl__exec to take a parameter containing the env variables

Add another parameter to libxl__exec call that contains the
environment variables to use when performing the execvp call.

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

diff -r 9a8ccac7b598 -r b380ea895013 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl.c	Thu Feb 02 10:42:14 2012 +0100
@@ -940,7 +940,7 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
         args[2] = "-autopass";
     }
 
-    libxl__exec(autopass_fd, -1, -1, args[0], args);
+    libxl__exec(autopass_fd, -1, -1, args[0], args, NULL);
     abort();
 
  x_fail:
diff -r 9a8ccac7b598 -r b380ea895013 tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_bootloader.c	Thu Feb 02 10:42:14 2012 +0100
@@ -111,12 +111,12 @@ static int open_xenconsoled_pty(int *mas
 static pid_t fork_exec_bootloader(int *master, const char *arg0, char **args)
 {
     struct termios termattr;
+    char *env[] = {"TERM", "vt100", NULL};
     pid_t pid = forkpty(master, NULL, NULL, NULL);
     if (pid == -1)
         return -1;
     else if (pid == 0) {
-        setenv("TERM", "vt100", 1);
-        libxl__exec(-1, -1, -1, arg0, args);
+        libxl__exec(-1, -1, -1, arg0, args, env);
         return -1;
     }
 
diff -r 9a8ccac7b598 -r b380ea895013 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_dm.c	Thu Feb 02 10:42:14 2012 +0100
@@ -874,7 +874,7 @@ retry_transaction:
         goto out_close;
     if (!rc) { /* inner child */
         setsid();
-        libxl__exec(null, logfile_w, logfile_w, dm, args);
+        libxl__exec(null, logfile_w, logfile_w, dm, args, NULL);
     }
 
     rc = 0;
diff -r 9a8ccac7b598 -r b380ea895013 tools/libxl/libxl_exec.c
--- a/tools/libxl/libxl_exec.c	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_exec.c	Thu Feb 02 10:42:14 2012 +0100
@@ -72,7 +72,7 @@ static void check_open_fds(const char *w
 }
 
 void libxl__exec(int stdinfd, int stdoutfd, int stderrfd, const char *arg0,
-                char **args)
+                char **args, char **env)
      /* call this in the child */
 {
     if (stdinfd != -1)
@@ -95,6 +95,11 @@ void libxl__exec(int stdinfd, int stdout
     /* in case our caller set it to IGN.  subprocesses are entitled
      * to assume they got DFL. */
 
+    if (env != NULL) {
+        for (int i = 0; env[i] != NULL && env[i+1] != NULL; i += 2) {
+            setenv(env[i], env[i+1], 1);
+        }
+    }
     execvp(arg0, args);
 
     fprintf(stderr, "libxl: cannot execute %s: %s\n", arg0, strerror(errno));
diff -r 9a8ccac7b598 -r b380ea895013 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 10:42:14 2012 +0100
@@ -479,7 +479,7 @@ typedef struct {
  /* low-level stuff, for synchronous subprocesses etc. */
 
 _hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
-               const char *arg0, char **args); // logs errors, never returns
+               const char *arg0, char **args, char **env); // logs errors, never returns
 
 /* from xl_create */
 _hidden int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info, uint32_t *domid);

[PATCH 04 of 29 RFC] libxl: add libxl__forkexec function to libxl_exec

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1326564288 -3600
# Node ID 70e65c85e4c0af5be0d309ce7e5eec2a170cfcf7
# Parent  b380ea895013853efd335c3fba4fe22f27ce1de1
libxl: add libxl__forkexec function to libxl_exec

Add a new function to libxl_exec that performs a fork and executes
the passed arguments using libxl__exec.

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

diff -r b380ea895013 -r 70e65c85e4c0 tools/libxl/libxl_exec.c
--- a/tools/libxl/libxl_exec.c	Thu Feb 02 10:42:14 2012 +0100
+++ b/tools/libxl/libxl_exec.c	Sat Jan 14 19:04:48 2012 +0100
@@ -443,6 +443,42 @@ int libxl__spawn_check(libxl__gc *gc, li
     return ERROR_FAIL;
 }
 
+int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
+                    int stderrfd, const char *arg0, char **args,
+                    char **env, const char *what)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int status;
+    int rc = 0;
+    pid_t pid = fork();
+
+    switch (pid) {
+    case -1:
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "fork failed\n");
+        rc = -1;
+        break;
+    case 0:
+        libxl__exec(stdinfd, stdoutfd, stderrfd, arg0, args, env);
+        /* libxl__exec never returns */
+    default:
+        while (waitpid(pid, &status, 0) < 0) {
+            if (errno != EINTR) {
+                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, 
+                                 "waitpid failed for %s\n",
+                                 what);
+                rc = -1;
+                break;
+            }
+        }
+        if (status)
+            libxl_report_child_exitstatus(ctx, LIBXL__LOG_ERROR,
+                                          what, pid, status);
+        rc = status;
+        break;
+    }
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r b380ea895013 -r 70e65c85e4c0 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 10:42:14 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Sat Jan 14 19:04:48 2012 +0100
@@ -481,6 +481,21 @@ typedef struct {
 _hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
                const char *arg0, char **args, char **env); // logs errors, never returns
 
+/*
+ * libxl__forkexec - Executes a file synchronously
+ * argv0: file name associated with the file being executed.
+ * args: list of arguments. See execvp(3) man page for more info.
+ * env: environment variables. See execvp(3) man page for more info.
+ *
+ * Returns -1 if the execution fails or the exit status, as reported
+ * by waitpid, on success.
+ *
+ * Logs errors.
+ */
+_hidden int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
+                            int stderrfd, const char *arg0, char **args,
+                            char **env, const char *what);
+
 /* from xl_create */
 _hidden int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info, uint32_t *domid);
 _hidden int libxl__domain_build(libxl__gc *gc,

[PATCH 05 of 29 RFC] libxl: wait for devices to initialize upon addition to the domain

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1326564288 -3600
# Node ID 98c4670598600f62df864840a0cffb96ad9553f2
# Parent  70e65c85e4c0af5be0d309ce7e5eec2a170cfcf7
libxl: wait for devices to initialize upon addition to the domain.

Block waiting for devices (with backend PHY or TAP) to initialize
(switch to state 2). This is necessary because we need to call the
hotplug scripts when state is 2, otherwise the execution fails. Make
use of the newly introduced function libxl__wait_for_device_state.

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

diff -r 70e65c85e4c0 -r 98c467059860 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Sat Jan 14 19:04:48 2012 +0100
+++ b/tools/libxl/libxl.c	Sat Jan 14 19:04:48 2012 +0100
@@ -1002,6 +1002,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_t *front;
     flexarray_t *back;
     char *dev;
+    char *be_path, *state_path, *state;
+    struct timeval tv;
     libxl__device device;
     int major, minor, rc;
 
@@ -1106,6 +1108,27 @@ int libxl_device_disk_add(libxl_ctx *ctx
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
+    if (disk->backend == LIBXL_DISK_BACKEND_PHY ||
+        disk->backend == LIBXL_DISK_BACKEND_TAP) {
+        be_path = libxl__device_backend_path(gc, &device);
+        state_path = libxl__sprintf(gc, "%s/state", be_path);
+        state = libxl__xs_read(gc, XBT_NULL, state_path);
+
+        if (atoi(state) != XenbusStateInitWait) {
+            xs_watch(ctx->xsh, state_path, be_path);
+            tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+            tv.tv_usec = 0;
+            rc = libxl__wait_for_device_state(gc, &tv, XenbusStateInitWait,
+                                              NULL);
+            xs_unwatch(ctx->xsh, state_path, be_path);
+            if (rc < 0) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                           "unable to initialize disk device: %s",
+                           disk->pdev_path);
+                goto out_free;
+            }
+        }
+    }
     rc = 0;
 
 out_free:
@@ -1481,6 +1504,8 @@ int libxl_device_nic_add(libxl_ctx *ctx,
     flexarray_t *back;
     libxl__device device;
     char *dompath, **l;
+    char *be_path, *state_path, *state;
+    struct timeval tv;
     unsigned int nb, rc;
 
     front = flexarray_make(16, 1);
@@ -1555,8 +1580,25 @@ int libxl_device_nic_add(libxl_ctx *ctx,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
-    /* FIXME: wait for plug */
+    be_path = libxl__device_backend_path(gc, &device);
+    state_path = libxl__sprintf(gc, "%s/state", be_path);
+    state = libxl__xs_read(gc, XBT_NULL, state_path);
+
+    if (atoi(state) != XenbusStateInitWait) {
+        xs_watch(ctx->xsh, state_path, be_path);
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        rc = libxl__wait_for_device_state(gc, &tv, XenbusStateInitWait, NULL);
+        xs_unwatch(ctx->xsh, state_path, be_path);
+        if (rc < 0) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "unable to initialize nic device: %s",
+                       nic->ifname);
+            goto out_free;
+        }
+    }
     rc = 0;
+
 out_free:
     flexarray_free(back);
     flexarray_free(front);

[PATCH 06 of 29 RFC] hotplug NetBSD: pass an action instead of a state to hotplug scripts

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1326564288 -3600
# Node ID 2c5bf4967eb769c69de665fc84a4d3c0d87d3440
# Parent  98c4670598600f62df864840a0cffb96ad9553f2
hotplug NetBSD: pass an action instead of a state to hotplug scripts

change second parameter of NetBSD hotplug scripts to take an action
(CONNECT/DISCONNECT) instead of a xenbus state. This patch also
changes the behaviour of xenbackend to pass an action instead of a
state.

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

diff -r 98c467059860 -r 2c5bf4967eb7 tools/hotplug/NetBSD/block
--- a/tools/hotplug/NetBSD/block	Sat Jan 14 19:04:48 2012 +0100
+++ b/tools/hotplug/NetBSD/block	Sat Jan 14 19:04:48 2012 +0100
@@ -18,7 +18,7 @@ error() {
 	
 
 xpath=$1
-xstatus=$2
+xaction=$2
 xparams=$(xenstore-read "$xpath/params")
 
 if [ -f $xparams ]; then
@@ -27,8 +27,8 @@ else
 	xtype="phy"
 fi
 
-case $xstatus in
-6)
+case $xaction in
+2)
 	# device removed
 	case $xtype in
 	file)
@@ -46,7 +46,7 @@ 6)
 	xenstore-rm $xpath
 	exit 0
 	;;
-2)
+1)
 	case $xtype in
 	file)
 		# Store the list of available vnd(4) devices in
diff -r 98c467059860 -r 2c5bf4967eb7 tools/hotplug/NetBSD/vif-bridge
--- a/tools/hotplug/NetBSD/vif-bridge	Sat Jan 14 19:04:48 2012 +0100
+++ b/tools/hotplug/NetBSD/vif-bridge	Sat Jan 14 19:04:48 2012 +0100
@@ -11,15 +11,15 @@ PATH=${BINDIR}:${SBINDIR}:${LIBEXEC}:${P
 export PATH
 
 xpath=$1
-xstatus=$2
+xaction=$2
 
-case $xstatus in
-6)
+case $xaction in
+2)
 	# device removed
 	xenstore-rm $xpath
 	exit 0
 	;;
-2)
+1)
 	xbridge=$(xenstore-read "$xpath/bridge")
 	xfid=$(xenstore-read "$xpath/frontend-id")
 	xhandle=$(xenstore-read "$xpath/handle")
diff -r 98c467059860 -r 2c5bf4967eb7 tools/hotplug/NetBSD/vif-ip
--- a/tools/hotplug/NetBSD/vif-ip	Sat Jan 14 19:04:48 2012 +0100
+++ b/tools/hotplug/NetBSD/vif-ip	Sat Jan 14 19:04:48 2012 +0100
@@ -11,15 +11,15 @@ PATH=${BINDIR}:${SBINDIR}:${LIBEXEC}:${P
 export PATH
 
 xpath=$1
-xstatus=$2
+xaction=$2
 
-case $xstatus in
-6)
+case $xaction in
+2)
 	# device removed
 	xenstore-rm $xpath
 	exit 0
 	;;
-2)
+1)
 	xip=$(xenstore-read "$xpath/ip")
 	xfid=$(xenstore-read "$xpath/frontend-id")
 	xhandle=$(xenstore-read "$xpath/handle")
diff -r 98c467059860 -r 2c5bf4967eb7 tools/xenbackendd/xenbackendd.c
--- a/tools/xenbackendd/xenbackendd.c	Sat Jan 14 19:04:48 2012 +0100
+++ b/tools/xenbackendd/xenbackendd.c	Sat Jan 14 19:04:48 2012 +0100
@@ -34,6 +34,9 @@
 #define DEVTYPE_VIF 1
 #define DEVTYPE_VBD 2
 
+#define CONNECT "1"
+#define DISCONNECT "2"
+
 #define DOMAIN_PATH "/local/domain/0"
 
 #ifndef XEN_SCRIPT_DIR
@@ -149,6 +152,7 @@ main(int argc, char * const argv[])
 	unsigned int num;
 	char *s;
 	int state;
+	char *action;
 	char *sstate;
 	char *p;
 	char buf[80];
@@ -297,11 +301,13 @@ main(int argc, char * const argv[])
 				    strerror(errno));
 				goto next2;
 			}
-			doexec(s, vec[XS_WATCH_PATH], sstate);
+			action = (state == 6 ? DISCONNECT : CONNECT);
+			doexec(s, vec[XS_WATCH_PATH], action);
 			break;
 
 		case DEVTYPE_VBD:
-			doexec(vbd_script, vec[XS_WATCH_PATH], sstate);
+			action = (state == 6 ? DISCONNECT : CONNECT);
+			doexec(vbd_script, vec[XS_WATCH_PATH], action);
 			break;
 
 		default:

[PATCH 07 of 29 RFC] libxl: perform xenstore device cleanup from libxl

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328176485 -3600
# Node ID 7b7a3fb7c90a2a08da7348075ba4589d4f2cc19b
# Parent  2c5bf4967eb769c69de665fc84a4d3c0d87d3440
libxl: perform xenstore device cleanup from libxl

Perform cleanup of xenstore device entries in libxl, instead of
relying on xen-hotplug-cleanup script.

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

diff -r 2c5bf4967eb7 -r 7b7a3fb7c90a tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Sat Jan 14 19:04:48 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 10:54:45 2012 +0100
@@ -406,6 +406,28 @@ start:
     return rc;
 }
 
+static int libxl__xs_path_cleanup(libxl__gc *gc, char *path)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    unsigned int nb = 0;
+    char *last;
+
+    if (!path)
+        return 0;
+
+    xs_rm(ctx->xsh, XBT_NULL, path);
+
+    for (last = strrchr(path, '/'); last != NULL; last = strrchr(path, '/')) {
+        *last = '\0';
+        if (!libxl__xs_directory(gc, XBT_NULL, path, &nb))
+            continue;
+        if (nb == 0) {
+            xs_rm(ctx->xsh, XBT_NULL, path);
+        }
+    }
+    return 0;
+}
+
 /*
  * Handler function for device destruction to be passed to
  * libxl__wait_for_device_state
@@ -413,14 +435,26 @@ start:
 static int destroy_device(libxl__gc *gc, char **l1, char *state)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
+    libxl__device dev;
+    int rc = -1;
 
     xs_unwatch(ctx->xsh, l1[0], l1[1]);
-    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+
+    rc = libxl__parse_backend_path(gc, l1[0], &dev);
+    if (rc < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "failed to generate device from backend path %s",
+                   l1[0]);
+        goto out;
+    }
+
+    libxl__xs_path_cleanup(gc, libxl__device_backend_path(gc, &dev));
     LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
                "Destroyed device backend at %s",
                l1[XS_WATCH_TOKEN]);
-
-    return 0;
+    rc = 0;
+out:
+    return rc;
 }
 
 /*
@@ -446,7 +480,7 @@ retry_transaction:
     if (atoi(state) != 4) {
         xs_transaction_end(ctx->xsh, t, 0);
         libxl__device_destroy_tapdisk(gc, be_path);
-        xs_rm(ctx->xsh, XBT_NULL, be_path);
+        libxl__xs_path_cleanup(gc, be_path);
         goto out;
     }
 
@@ -483,12 +517,11 @@ out:
 
 int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
     char *be_path = libxl__device_backend_path(gc, dev);
     char *fe_path = libxl__device_frontend_path(gc, dev);
 
-    xs_rm(ctx->xsh, XBT_NULL, be_path);
-    xs_rm(ctx->xsh, XBT_NULL, fe_path);
+    libxl__xs_path_cleanup(gc, be_path);
+    libxl__xs_path_cleanup(gc, fe_path);
 
     libxl__device_destroy_tapdisk(gc, be_path);

[PATCH 08 of 29 RFC] libxl: remove force parameter from libxl__device_remove

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328176666 -3600
# Node ID 1efb9ee82433896e5a68f9a510f1f5542f071bd3
# Parent  7b7a3fb7c90a2a08da7348075ba4589d4f2cc19b
libxl: remove force parameter from libxl__device_remove

All callers where using the wait parameter from
libxl__device_remove, so it's safe to simplify the logic of the
function and assume that the callers always want libxl__device_remove
to wait for the unplug of the device.

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

diff -r 7b7a3fb7c90a -r 1efb9ee82433 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 10:54:45 2012 +0100
+++ b/tools/libxl/libxl.c	Thu Feb 02 10:57:46 2012 +0100
@@ -1149,7 +1149,7 @@ int libxl_device_disk_remove(libxl_ctx *
     rc = libxl__device_from_disk(gc, domid, disk, &device);
     if (rc != 0) goto out;
 
-    rc = libxl__device_remove(gc, &device, 1);
+    rc = libxl__device_remove(gc, &device);
 out:
     GC_FREE;
     return rc;
@@ -1617,7 +1617,7 @@ int libxl_device_nic_remove(libxl_ctx *c
     rc = libxl__device_from_nic(gc, domid, nic, &device);
     if (rc != 0) goto out;
 
-    rc = libxl__device_remove(gc, &device, 1);
+    rc = libxl__device_remove(gc, &device);
 out:
     GC_FREE;
     return rc;
@@ -1957,7 +1957,7 @@ int libxl_device_vkb_remove(libxl_ctx *c
     rc = libxl__device_from_vkb(gc, domid, vkb, &device);
     if (rc != 0) goto out;
 
-    rc = libxl__device_remove(gc, &device, 1);
+    rc = libxl__device_remove(gc, &device);
 out:
     GC_FREE;
     return rc;
@@ -2074,7 +2074,7 @@ int libxl_device_vfb_remove(libxl_ctx *c
     rc = libxl__device_from_vfb(gc, domid, vfb, &device);
     if (rc != 0) goto out;
 
-    rc = libxl__device_remove(gc, &device, 1);
+    rc = libxl__device_remove(gc, &device);
 out:
     GC_FREE;
     return rc;
diff -r 7b7a3fb7c90a -r 1efb9ee82433 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 10:54:45 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 10:57:46 2012 +0100
@@ -461,13 +461,14 @@ out:
  * Returns 0 (device already destroyed) or 1 (caller must
  * wait_for_dev_destroy) on success, ERROR_* on fail.
  */
-int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait)
+int libxl__device_remove(libxl__gc *gc, libxl__device *dev)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     xs_transaction_t t;
     char *be_path = libxl__device_backend_path(gc, dev);
     char *state_path = libxl__sprintf(gc, "%s/state", be_path);
     char *state;
+    struct timeval tv;
     int rc = 0;
 
 retry_transaction:
@@ -498,18 +499,13 @@ retry_transaction:
     xs_watch(ctx->xsh, state_path, be_path);
     libxl__device_destroy_tapdisk(gc, be_path);
 
-    if (wait) {
-        struct timeval tv;
-        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
-        tv.tv_usec = 0;
-        rc = libxl__wait_for_device_state(gc, &tv, XenbusStateClosed,
-                                          destroy_device);
-        if (rc < 0) /* an error or timeout occurred, clear watches */
-            xs_unwatch(ctx->xsh, state_path, be_path);
-        xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
-    } else {
-        rc = 1; /* Caller must wait_for_dev_destroy */
-    }
+    tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+    tv.tv_usec = 0;
+    rc = libxl__wait_for_device_state(gc, &tv, XenbusStateClosed,
+                                      destroy_device);
+    if (rc < 0) /* an error or timeout occurred, clear watches */
+        xs_unwatch(ctx->xsh, state_path, be_path);
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
 
 out:
     return rc;
diff -r 7b7a3fb7c90a -r 1efb9ee82433 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 10:54:45 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 10:57:46 2012 +0100
@@ -303,7 +303,7 @@ typedef struct {
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
                                       libxl__device *dev);
-_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait);
+_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);

[PATCH 09 of 29 RFC] libxl: add better error checking on libxl__device_remove

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328176865 -3600
# Node ID 8a5a82c9819558015ac75afdef28ee3d88d4f334
# Parent  1efb9ee82433896e5a68f9a510f1f5542f071bd3
libxl: add better error checking on libxl__device_remove

Check return value of libxl__wait_for_device_state on
libxl__device_remove and print an error message accordingly.

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

diff -r 1efb9ee82433 -r 8a5a82c98195 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 10:57:46 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 11:01:05 2012 +0100
@@ -503,8 +503,17 @@ retry_transaction:
     tv.tv_usec = 0;
     rc = libxl__wait_for_device_state(gc, &tv, XenbusStateClosed,
                                       destroy_device);
-    if (rc < 0) /* an error or timeout occurred, clear watches */
+    if (rc == ERROR_TIMEDOUT) {
+        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+                   "Timeout while waiting for unplug of "
+                   "device with backend path %s", be_path);
         xs_unwatch(ctx->xsh, state_path, be_path);
+    } else if (rc == ERROR_FAIL) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "failed to destroy device with "
+                   "backend path %s", be_path);
+        xs_unwatch(ctx->xsh, state_path, be_path);
+    }
     xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
 
 out:

[PATCH 10 of 29 RFC] libxl: destroy devices before device model

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328177046 -3600
# Node ID 80835b6fed80331703a525a3b6ac98153d9868f6
# Parent  8a5a82c9819558015ac75afdef28ee3d88d4f334
libxl: destroy devices before device model

Destroying the device model before unplugging the devices prevented
from doing a clean unplug, since libxl was waiting for dm  devices
to shutdown when the userspace process was already killed.

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

diff -r 8a5a82c98195 -r 80835b6fed80 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 11:01:05 2012 +0100
+++ b/tools/libxl/libxl.c	Thu Feb 02 11:04:06 2012 +0100
@@ -791,15 +791,15 @@ int libxl_domain_destroy(libxl_ctx *ctx,
     if (rc < 0) {
         LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, "xc_domain_pause failed for %d", domid);
     }
+    if (libxl__devices_destroy(gc, domid) < 0)
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, 
+                   "libxl__devices_destroy failed for %d", domid);
     if (dm_present) {
         if (libxl__destroy_device_model(gc, domid) < 0)
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl__destroy_device_model failed for %d", domid);
 
         libxl__qmp_cleanup(gc, domid);
     }
-    if (libxl__devices_destroy(gc, domid) < 0)
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, 
-                   "libxl__devices_destroy failed for %d", domid);
 
     vm_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/vm", dom_path));
     if (vm_path)

[PATCH 11 of 29 RFC] libxl: execute hotplug scripts directly from libxl

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1317386335 -7200
# Node ID 5e488e0c02b8c769e94c2f734c0c209739d9cb01
# Parent  80835b6fed80331703a525a3b6ac98153d9868f6
libxl: execute hotplug scripts directly from libxl.

Added the necessary handlers to execute hotplug scripts when necessary
from libxl. Split NetBSD and Linux hotplug calls into two separate
files, because parameters for hotplug scripts are different. This
patch adds the necessary functions to call hotplug scripts, but the
implementation of those functions is left empty and will be filled in
future patches.

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

diff -r 80835b6fed80 -r 5e488e0c02b8 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 11:04:06 2012 +0100
+++ b/tools/libxl/libxl.c	Fri Sep 30 14:38:55 2011 +0200
@@ -1049,6 +1049,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
             flexarray_append(back, "params");
             flexarray_append(back, dev);
 
+            flexarray_append(back, "script");
+            flexarray_append(back, libxl__sprintf(gc, "%s/%s",
+                                                  libxl_xen_script_dir_path(),
+                                                  "block"));
+
             assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
             break;
         case LIBXL_DISK_BACKEND_TAP:
@@ -1061,6 +1066,10 @@ int libxl_device_disk_add(libxl_ctx *ctx
             flexarray_append(back, libxl__sprintf(gc, "%s:%s",
                 libxl__device_disk_string_of_format(disk->format),
                 disk->pdev_path));
+            flexarray_append(back, "script");
+            flexarray_append(back, libxl__sprintf(gc, "%s/%s",
+                             libxl_xen_script_dir_path(),
+                             "blktap"));
 
             /* now create a phy device to export the device to the guest */
             goto do_backend_phy;
@@ -1129,6 +1138,16 @@ int libxl_device_disk_add(libxl_ctx *ctx
             }
         }
     }
+
+    /* Call hotplug scripts to attach device */
+    if (libxl__device_hotplug(gc, &device, CONNECT) < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "unable to execute hotplug script for disk: %s",
+                   disk->pdev_path);
+        rc = -1;
+        goto out_free;
+    }
+
     rc = 0;
 
 out_free:
@@ -1597,6 +1616,16 @@ int libxl_device_nic_add(libxl_ctx *ctx,
             goto out_free;
         }
     }
+
+    /* Call hotplug scripts to attach device */
+    if (libxl__device_hotplug(gc, &device, CONNECT) < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "unable to execute hotplug script for nic: %s\n",
+                   nic->ifname);
+        rc = -1;
+        goto out_free;
+    }
+
     rc = 0;
 
 out_free:
diff -r 80835b6fed80 -r 5e488e0c02b8 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 11:04:06 2012 +0100
+++ b/tools/libxl/libxl_device.c	Fri Sep 30 14:38:55 2011 +0200
@@ -448,10 +448,15 @@ static int destroy_device(libxl__gc *gc,
         goto out;
     }
 
+    rc = libxl__device_hotplug(gc, &dev, DISCONNECT);
+    if (rc < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "failed to execute hotplug script for "
+                   "device with backend path %s", l1[0]);
+        goto out;
+    }
+
     libxl__xs_path_cleanup(gc, libxl__device_backend_path(gc, &dev));
-    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
-               "Destroyed device backend at %s",
-               l1[XS_WATCH_TOKEN]);
     rc = 0;
 out:
     return rc;
@@ -480,6 +485,12 @@ retry_transaction:
     }
     if (atoi(state) != 4) {
         xs_transaction_end(ctx->xsh, t, 0);
+        rc = libxl__device_hotplug(gc, dev, DISCONNECT);
+        if (rc < 0) {
+             LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                        "failed to execute hotplug script for "
+                        "device with backend path %s", be_path);
+        }
         libxl__device_destroy_tapdisk(gc, be_path);
         libxl__xs_path_cleanup(gc, be_path);
         goto out;
@@ -524,13 +535,16 @@ int libxl__device_destroy(libxl__gc *gc,
 {
     char *be_path = libxl__device_backend_path(gc, dev);
     char *fe_path = libxl__device_frontend_path(gc, dev);
+    int rc;
 
+    libxl__xs_path_cleanup(gc, fe_path);
+    /* Wait for backend status to disconnect */
+    rc = libxl__device_remove(gc, dev);
     libxl__xs_path_cleanup(gc, be_path);
-    libxl__xs_path_cleanup(gc, fe_path);
 
     libxl__device_destroy_tapdisk(gc, be_path);
 
-    return 0;
+    return rc;
 }
 
 int libxl__devices_destroy(libxl__gc *gc, uint32_t domid)
diff -r 80835b6fed80 -r 5e488e0c02b8 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 11:04:06 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
@@ -342,6 +342,25 @@ typedef int libxl__device_state_handler(
  */
 _hidden int libxl__try_phy_backend(mode_t st_mode);
 
+/* hotplug functions */
+
+/* Action to pass to hotplug caller functions */
+typedef enum {
+    CONNECT = 1,
+    DISCONNECT = 2
+} libxl__hotplug_action;
+
+/*
+ * libxl__device_hotplug - generic function to execute hotplug scripts
+ * gc: allocation pool
+ * dev: reference to the device that executes the hotplug scripts
+ * action: action to execute
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
+                                  libxl__hotplug_action action);
+
 /* from libxl_pci */
 
 _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
diff -r 80835b6fed80 -r 5e488e0c02b8 tools/libxl/libxl_linux.c
--- a/tools/libxl/libxl_linux.c	Thu Feb 02 11:04:06 2012 +0100
+++ b/tools/libxl/libxl_linux.c	Fri Sep 30 14:38:55 2011 +0200
@@ -25,3 +25,11 @@ int libxl__try_phy_backend(mode_t st_mod
 
     return 1;
 }
+
+/* Hotplug scripts caller functions */
+
+int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
+                          libxl__hotplug_action action)
+{
+    return 0;
+}
diff -r 80835b6fed80 -r 5e488e0c02b8 tools/libxl/libxl_netbsd.c
--- a/tools/libxl/libxl_netbsd.c	Thu Feb 02 11:04:06 2012 +0100
+++ b/tools/libxl/libxl_netbsd.c	Fri Sep 30 14:38:55 2011 +0200
@@ -24,3 +24,11 @@ int libxl__try_phy_backend(mode_t st_mod
 
     return 0;
 }
+
+/* Hotplug scripts call function */
+
+int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
+                          libxl__hotplug_action action)
+{
+    return 0;
+}

[PATCH 12 of 29 RFC] libxl: add hotplug script calling for NetBSD

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328177424 -3600
# Node ID 6c2690921fba580dd7dd836da3be484dee049be0
# Parent  5e488e0c02b8c769e94c2f734c0c209739d9cb01
libxl: add hotplug script calling for NetBSD

Hotplug scripts in NetBSD are in charge of adding the virtual DomU
network interface to the desired bridge and also mount the vnd device
to use virtual images as block devices. The following scripts are
currently implemented:

 * block: executed when a vbd is used, is in charge mounting the
   virtual image to the vnd device and setting the state of xenstore
   to 4 (connected). When using a physical partition, the script only
   sets the state to 4.

 * vif-bridge: adds the virtual DomU interface to the desired bridge.

 * vif-ip: configures the physical network interface assigned to the
   DomU.

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

diff -r 5e488e0c02b8 -r 6c2690921fba tools/libxl/libxl_netbsd.c
--- a/tools/libxl/libxl_netbsd.c	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_netbsd.c	Thu Feb 02 11:10:24 2012 +0100
@@ -30,5 +30,50 @@ int libxl__try_phy_backend(mode_t st_mod
 int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
                           libxl__hotplug_action action)
 {
-    return 0;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *script, *what;
+    char **args;
+    int status, nr = 0;
+    int rc = -1;
+    flexarray_t *f_args;
+
+    if (dev->backend_kind != LIBXL__DEVICE_KIND_VIF &&
+        dev->backend_kind != LIBXL__DEVICE_KIND_VBD)
+        return 0;
+
+    script = libxl__xs_read(gc, XBT_NULL,
+                            libxl__sprintf(gc, "%s/%s", be_path, "script"));
+    if (!script) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to read script from %s",
+                                          be_path);
+        return -1;
+    }
+
+    f_args = flexarray_make(4, 1);
+    if (!f_args)
+        return -1;
+
+    flexarray_set(f_args, nr++, script);
+    flexarray_set(f_args, nr++, be_path);
+    flexarray_set(f_args, nr++, libxl__sprintf(gc, "%d", action));
+    flexarray_set(f_args, nr++, NULL);
+
+    args = (char **) flexarray_contents(f_args);
+
+    what = libxl__sprintf(gc, "%s %s", args[0],
+                          action == CONNECT ? "connect" : "disconnect");
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+               "Calling hotplug script: %s %s %s",
+               args[0], args[1], args[2]);
+    status = libxl__forkexec(gc, -1, -1, -1, args[0], args, NULL, what);
+    if (status) {
+        rc = -1;
+        goto out;
+    }
+    rc = 0;
+
+out:
+    free(args);
+    return rc;
 }

[PATCH 13 of 29 RFC] libxl: add hotplug script calls for Linux

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328177591 -3600
# Node ID 98a4f01033786a2fc15b4130897c35f64a676e40
# Parent  6c2690921fba580dd7dd836da3be484dee049be0
libxl: add hotplug script calls for Linux

This patchs adds the necessary logic to call hotplug scripts directly
from libxl. Linux hotplug scritps read most parameters from the caller
environment (since udev set this parameters automatically). In this
implementation we fake udev parameters, so no changes are needed to
the scripts itself.

Currently, the following scripts are called:

 * block: used when disk backend is PHY.

 * blktap: used when disk backend is TAP.

 * vif-*: used when adding a network interface and can be manually set
   by the user.

udev rules descrive more device types, currently the following scripts
are NOT executed from libxl because I wasn't able to find any support
for this device types in libxl:

 * vtpm

 * vif2

 * vscsi

 * vif-setup with devices of type tap*

This patch just adds the necessary code, but hotplug scripts are NOT
called from libxl yet, following patches will disable udev rules and
use this calls instead.

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

diff -r 6c2690921fba -r 98a4f0103378 tools/libxl/libxl_linux.c
--- a/tools/libxl/libxl_linux.c	Thu Feb 02 11:10:24 2012 +0100
+++ b/tools/libxl/libxl_linux.c	Thu Feb 02 11:13:11 2012 +0100
@@ -26,10 +26,172 @@ int libxl__try_phy_backend(mode_t st_mod
     return 1;
 }
 
+/* Hotplug scripts helpers */
+#if 0
+static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *be_path = libxl__device_backend_path(gc, dev);
+    flexarray_t *f_env;
+    int nr = 0;
+
+    f_env = flexarray_make(11, 1);
+    if (!f_env) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "unable to create environment array");
+        return NULL;
+    }
+
+    flexarray_set(f_env, nr++, "script");
+    flexarray_set(f_env, nr++, libxl__xs_read(gc, XBT_NULL,
+                               libxl__sprintf(gc, "%s/%s",
+                                              be_path,
+                                              "script")));
+    flexarray_set(f_env, nr++, "XENBUS_TYPE");
+    flexarray_set(f_env, nr++, (char *)
+                  libxl__device_kind_to_string(dev->backend_kind));
+    flexarray_set(f_env, nr++, "XENBUS_PATH");
+    flexarray_set(f_env, nr++,
+                  libxl__sprintf(gc, "backend/%s/%u/%d",
+                  libxl__device_kind_to_string(dev->backend_kind),
+                  dev->domid, dev->devid));
+    flexarray_set(f_env, nr++, "XENBUS_BASE_PATH");
+    flexarray_set(f_env, nr++, "backend");
+    if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
+        flexarray_set(f_env, nr++, "vif");
+        flexarray_set(f_env, nr++,
+                      libxl__sprintf(gc, "%s%u.%d",
+                      libxl__device_kind_to_string(dev->backend_kind),
+                      dev->domid, dev->devid));
+    }
+    flexarray_set(f_env, nr++, NULL);
+
+    return (char **) flexarray_contents(f_env);
+}
+
 /* Hotplug scripts caller functions */
 
+static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev,
+                       libxl__hotplug_action action)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *script, *what;
+    char **args, **env;
+    int status, nr = 0;
+    int rc = -1;
+    flexarray_t *f_args;
+
+    script = libxl__xs_read(gc, XBT_NULL,
+                            libxl__sprintf(gc, "%s/%s", be_path, "script"));
+    if (!script) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from %s",
+                                          be_path);
+        return -1;
+    }
+
+    env = get_hotplug_env(gc, dev);
+    if (!env)
+        return -1;
+
+    f_args = flexarray_make(4, 1);
+    if (!f_args) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments array");
+        return -1;
+    }
+
+    flexarray_set(f_args, nr++, script);
+    flexarray_set(f_args, nr++, action == CONNECT ? "online" : "offline");
+    flexarray_set(f_args, nr++, "type_if=vif");
+    flexarray_set(f_args, nr++, NULL);
+
+    args = (char **) flexarray_contents(f_args);
+    what = libxl__sprintf(gc, "%s %s", args[0],
+                          action == CONNECT ? "connect" : "disconnect");
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+               "Calling hotplug script: %s %s %s",
+               args[0], args[1], args[2]);
+    status = libxl__forkexec(gc, -1, -1, -1, args[0], args, env, what);
+    if (status) {
+        rc = -1;
+        goto out;
+    }
+    rc = 0;
+
+out:
+    free(env);
+    free(args);
+    return rc;
+}
+
+static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
+                       libxl__hotplug_action action)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *script, *what;
+    char **args, **env;
+    int status, nr = 0;
+    int rc = -1;
+    flexarray_t *f_args;
+
+    script = libxl__xs_read(gc, XBT_NULL,
+                            libxl__sprintf(gc, "%s/%s", be_path, "script"));
+    if (!script) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from %s",
+                                          be_path);
+        return -1;
+    }
+
+    env = get_hotplug_env(gc, dev);
+    if (!env)
+        return -1;
+
+    f_args = flexarray_make(3, 1);
+    if (!f_args) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments array");
+        return -1;
+    }
+
+    flexarray_set(f_args, nr++, script);
+    flexarray_set(f_args, nr++, action == CONNECT ? "add" : "remove");
+    flexarray_set(f_args, nr++, NULL);
+
+    args = (char **) flexarray_contents(f_args);
+    what = libxl__sprintf(gc, "%s %s", args[0],
+                          action == CONNECT ? "connect" : "disconnect");
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+               "Calling hotplug script: %s %s",
+               args[0], args[1]);
+    status = libxl__forkexec(gc, -1, -1, -1, args[0], args, env, what);
+    if (status) {
+        rc = -1;
+        goto out;
+    }
+    rc = 0;
+
+out:
+    free(env);
+    free(args);
+    return rc;
+}
+#endif /* 0 */
 int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
                           libxl__hotplug_action action)
 {
-    return 0;
+    int rc = 0;
+#if 0
+    switch (dev->backend_kind) {
+    case LIBXL__DEVICE_KIND_VIF:
+        rc = libxl__hotplug_nic(gc, dev, action);
+        break;
+    case LIBXL__DEVICE_KIND_VBD:
+        rc = libxl__hotplug_disk(gc, dev, action);
+        break;
+    default:
+        rc = 0;
+        break;
+    }
+#endif /* 0 */
+    return rc;
 }

Re: [PATCH 13 of 29 RFC] libxl: add hotplug script calls for Linux

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 7930 bytes --]

On Thu, Feb 2, 2012 at 5:26 AM, Roger Pau Monne <roger.pau@entel.upc.edu>wrote:

> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1328177591 -3600
> # Node ID 98a4f01033786a2fc15b4130897c35f64a676e40
> # Parent  6c2690921fba580dd7dd836da3be484dee049be0
> libxl: add hotplug script calls for Linux
>
> This patchs adds the necessary logic to call hotplug scripts directly
> from libxl. Linux hotplug scritps read most parameters from the caller
> environment (since udev set this parameters automatically). In this
> implementation we fake udev parameters, so no changes are needed to
> the scripts itself.
>
> Currently, the following scripts are called:
>
>  * block: used when disk backend is PHY.
>
>  * blktap: used when disk backend is TAP.
>
>
Sorry if this is a bit dense, but you have been through the code
already ;)
Is there an option to call other scripts other than block/blktap ?
For e.g., if I were to add a backend type DRBD/FOOBAR,
and wish to call script "block-<drbd|foobar>, how do you suggest I
proceed ?
  Start my way from defining new disk backend types and just add
hotplug handlers for these block-* devices and finally expose them as
phy devices to libxl ?

shriram


>  * vif-*: used when adding a network interface and can be manually set
>   by the user.
>
> udev rules descrive more device types, currently the following scripts
> are NOT executed from libxl because I wasn't able to find any support
> for this device types in libxl:
>
>  * vtpm
>
>  * vif2
>
>  * vscsi
>
>  * vif-setup with devices of type tap*
>
> This patch just adds the necessary code, but hotplug scripts are NOT
> called from libxl yet, following patches will disable udev rules and
> use this calls instead.
>
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
>
> diff -r 6c2690921fba -r 98a4f0103378 tools/libxl/libxl_linux.c
> --- a/tools/libxl/libxl_linux.c Thu Feb 02 11:10:24 2012 +0100
> +++ b/tools/libxl/libxl_linux.c Thu Feb 02 11:13:11 2012 +0100
> @@ -26,10 +26,172 @@ int libxl__try_phy_backend(mode_t st_mod
>     return 1;
>  }
>
> +/* Hotplug scripts helpers */
> +#if 0
> +static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    char *be_path = libxl__device_backend_path(gc, dev);
> +    flexarray_t *f_env;
> +    int nr = 0;
> +
> +    f_env = flexarray_make(11, 1);
> +    if (!f_env) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                   "unable to create environment array");
> +        return NULL;
> +    }
> +
> +    flexarray_set(f_env, nr++, "script");
> +    flexarray_set(f_env, nr++, libxl__xs_read(gc, XBT_NULL,
> +                               libxl__sprintf(gc, "%s/%s",
> +                                              be_path,
> +                                              "script")));
> +    flexarray_set(f_env, nr++, "XENBUS_TYPE");
> +    flexarray_set(f_env, nr++, (char *)
> +                  libxl__device_kind_to_string(dev->backend_kind));
> +    flexarray_set(f_env, nr++, "XENBUS_PATH");
> +    flexarray_set(f_env, nr++,
> +                  libxl__sprintf(gc, "backend/%s/%u/%d",
> +                  libxl__device_kind_to_string(dev->backend_kind),
> +                  dev->domid, dev->devid));
> +    flexarray_set(f_env, nr++, "XENBUS_BASE_PATH");
> +    flexarray_set(f_env, nr++, "backend");
> +    if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
> +        flexarray_set(f_env, nr++, "vif");
> +        flexarray_set(f_env, nr++,
> +                      libxl__sprintf(gc, "%s%u.%d",
> +                      libxl__device_kind_to_string(dev->backend_kind),
> +                      dev->domid, dev->devid));
> +    }
> +    flexarray_set(f_env, nr++, NULL);
> +
> +    return (char **) flexarray_contents(f_env);
> +}
> +
>  /* Hotplug scripts caller functions */
>
> +static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev,
> +                       libxl__hotplug_action action)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    char *be_path = libxl__device_backend_path(gc, dev);
> +    char *script, *what;
> +    char **args, **env;
> +    int status, nr = 0;
> +    int rc = -1;
> +    flexarray_t *f_args;
> +
> +    script = libxl__xs_read(gc, XBT_NULL,
> +                            libxl__sprintf(gc, "%s/%s", be_path,
> "script"));
> +    if (!script) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from %s",
> +                                          be_path);
> +        return -1;
> +    }
> +
> +    env = get_hotplug_env(gc, dev);
> +    if (!env)
> +        return -1;
> +
> +    f_args = flexarray_make(4, 1);
> +    if (!f_args) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments
> array");
> +        return -1;
> +    }
> +
> +    flexarray_set(f_args, nr++, script);
> +    flexarray_set(f_args, nr++, action == CONNECT ? "online" : "offline");
> +    flexarray_set(f_args, nr++, "type_if=vif");
> +    flexarray_set(f_args, nr++, NULL);
> +
> +    args = (char **) flexarray_contents(f_args);
> +    what = libxl__sprintf(gc, "%s %s", args[0],
> +                          action == CONNECT ? "connect" : "disconnect");
> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> +               "Calling hotplug script: %s %s %s",
> +               args[0], args[1], args[2]);
> +    status = libxl__forkexec(gc, -1, -1, -1, args[0], args, env, what);
> +    if (status) {
> +        rc = -1;
> +        goto out;
> +    }
> +    rc = 0;
> +
> +out:
> +    free(env);
> +    free(args);
> +    return rc;
> +}
> +
> +static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
> +                       libxl__hotplug_action action)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    char *be_path = libxl__device_backend_path(gc, dev);
> +    char *script, *what;
> +    char **args, **env;
> +    int status, nr = 0;
> +    int rc = -1;
> +    flexarray_t *f_args;
> +
> +    script = libxl__xs_read(gc, XBT_NULL,
> +                            libxl__sprintf(gc, "%s/%s", be_path,
> "script"));
> +    if (!script) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from %s",
> +                                          be_path);
> +        return -1;
> +    }
> +
> +    env = get_hotplug_env(gc, dev);
> +    if (!env)
> +        return -1;
> +
> +    f_args = flexarray_make(3, 1);
> +    if (!f_args) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments
> array");
> +        return -1;
> +    }
> +
> +    flexarray_set(f_args, nr++, script);
> +    flexarray_set(f_args, nr++, action == CONNECT ? "add" : "remove");
> +    flexarray_set(f_args, nr++, NULL);
> +
> +    args = (char **) flexarray_contents(f_args);
> +    what = libxl__sprintf(gc, "%s %s", args[0],
> +                          action == CONNECT ? "connect" : "disconnect");
> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> +               "Calling hotplug script: %s %s",
> +               args[0], args[1]);
> +    status = libxl__forkexec(gc, -1, -1, -1, args[0], args, env, what);
> +    if (status) {
> +        rc = -1;
> +        goto out;
> +    }
> +    rc = 0;
> +
> +out:
> +    free(env);
> +    free(args);
> +    return rc;
> +}
> +#endif /* 0 */
>  int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
>                           libxl__hotplug_action action)
>  {
> -    return 0;
> +    int rc = 0;
> +#if 0
> +    switch (dev->backend_kind) {
> +    case LIBXL__DEVICE_KIND_VIF:
> +        rc = libxl__hotplug_nic(gc, dev, action);
> +        break;
> +    case LIBXL__DEVICE_KIND_VBD:
> +        rc = libxl__hotplug_disk(gc, dev, action);
> +        break;
> +    default:
> +        rc = 0;
> +        break;
> +    }
> +#endif /* 0 */
> +    return rc;
>  }
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

[-- Attachment #1.2: Type: text/html, Size: 9744 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH 13 of 29 RFC] libxl: add hotplug script calls for Linux

[Roger Pau Monné]

2012/2/7 Shriram Rajagopalan <rshriram@cs.ubc.ca>:
> On Thu, Feb 2, 2012 at 5:26 AM, Roger Pau Monne <roger.pau@entel.upc.edu>
> wrote:
>>
>> # HG changeset patch
>> # User Roger Pau Monne <roger.pau@entel.upc.edu>
>> # Date 1328177591 -3600
>> # Node ID 98a4f01033786a2fc15b4130897c35f64a676e40
>> # Parent  6c2690921fba580dd7dd836da3be484dee049be0
>> libxl: add hotplug script calls for Linux
>>
>> This patchs adds the necessary logic to call hotplug scripts directly
>> from libxl. Linux hotplug scritps read most parameters from the caller
>> environment (since udev set this parameters automatically). In this
>> implementation we fake udev parameters, so no changes are needed to
>> the scripts itself.
>>
>> Currently, the following scripts are called:
>>
>>  * block: used when disk backend is PHY.
>>
>>  * blktap: used when disk backend is TAP.
>>
>
> Sorry if this is a bit dense, but you have been through the code
> already ;)
> Is there an option to call other scripts other than block/blktap ?

libxl has some preliminary support for setting custom hotplug scripts
for hard drives in configuration files (just like you do for nic
interfaces), this is currently disabled but could be enabled in future
releases.

> For e.g., if I were to add a backend type DRBD/FOOBAR,
> and wish to call script "block-<drbd|foobar>, how do you suggest I
> proceed ?

Well, I see several approaches here, right now we call Linux hotplug
scripts adding the necessary env variables, since udev used to set
those variables automatically, and we will continue to do it that way
until xend is removed. I think it's most suitable to pass those
parameters as arguments to the script, but this as I said before is a
long term goal.

>   Start my way from defining new disk backend types and just add
> hotplug handlers for these block-* devices and finally expose them as
> phy devices to libxl ?

That's really up to you, but if you want to pass them as PHY devices,
you should modify the libxl__try_phy_backend function inside
libxl_linux.c and libxl_netbsd.c. If you want this changes to be added
to upstream it might be best to define your own backend type,
overloading the PHY type it's not clearer, and PHY should only be used
for physical partitions (again, since I don't know your specific
implementation, I cannot be sure of the most suitable way to tackle
this problem).

>
> shriram
>
>>
>>  * vif-*: used when adding a network interface and can be manually set
>>   by the user.
>>
>> udev rules descrive more device types, currently the following scripts
>> are NOT executed from libxl because I wasn't able to find any support
>> for this device types in libxl:
>>
>>  * vtpm
>>
>>  * vif2
>>
>>  * vscsi
>>
>>  * vif-setup with devices of type tap*
>>
>> This patch just adds the necessary code, but hotplug scripts are NOT
>> called from libxl yet, following patches will disable udev rules and
>> use this calls instead.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
>>
>> diff -r 6c2690921fba -r 98a4f0103378 tools/libxl/libxl_linux.c
>> --- a/tools/libxl/libxl_linux.c Thu Feb 02 11:10:24 2012 +0100
>> +++ b/tools/libxl/libxl_linux.c Thu Feb 02 11:13:11 2012 +0100
>> @@ -26,10 +26,172 @@ int libxl__try_phy_backend(mode_t st_mod
>>     return 1;
>>  }
>>
>> +/* Hotplug scripts helpers */
>> +#if 0
>> +static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
>> +{
>> +    libxl_ctx *ctx = libxl__gc_owner(gc);
>> +    char *be_path = libxl__device_backend_path(gc, dev);
>> +    flexarray_t *f_env;
>> +    int nr = 0;
>> +
>> +    f_env = flexarray_make(11, 1);
>> +    if (!f_env) {
>> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
>> +                   "unable to create environment array");
>> +        return NULL;
>> +    }
>> +
>> +    flexarray_set(f_env, nr++, "script");
>> +    flexarray_set(f_env, nr++, libxl__xs_read(gc, XBT_NULL,
>> +                               libxl__sprintf(gc, "%s/%s",
>> +                                              be_path,
>> +                                              "script")));
>> +    flexarray_set(f_env, nr++, "XENBUS_TYPE");
>> +    flexarray_set(f_env, nr++, (char *)
>> +                  libxl__device_kind_to_string(dev->backend_kind));
>> +    flexarray_set(f_env, nr++, "XENBUS_PATH");
>> +    flexarray_set(f_env, nr++,
>> +                  libxl__sprintf(gc, "backend/%s/%u/%d",
>> +                  libxl__device_kind_to_string(dev->backend_kind),
>> +                  dev->domid, dev->devid));
>> +    flexarray_set(f_env, nr++, "XENBUS_BASE_PATH");
>> +    flexarray_set(f_env, nr++, "backend");
>> +    if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
>> +        flexarray_set(f_env, nr++, "vif");
>> +        flexarray_set(f_env, nr++,
>> +                      libxl__sprintf(gc, "%s%u.%d",
>> +                      libxl__device_kind_to_string(dev->backend_kind),
>> +                      dev->domid, dev->devid));
>> +    }
>> +    flexarray_set(f_env, nr++, NULL);
>> +
>> +    return (char **) flexarray_contents(f_env);
>> +}
>> +
>>  /* Hotplug scripts caller functions */
>>
>> +static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev,
>> +                       libxl__hotplug_action action)
>> +{
>> +    libxl_ctx *ctx = libxl__gc_owner(gc);
>> +    char *be_path = libxl__device_backend_path(gc, dev);
>> +    char *script, *what;
>> +    char **args, **env;
>> +    int status, nr = 0;
>> +    int rc = -1;
>> +    flexarray_t *f_args;
>> +
>> +    script = libxl__xs_read(gc, XBT_NULL,
>> +                            libxl__sprintf(gc, "%s/%s", be_path,
>> "script"));
>> +    if (!script) {
>> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from
>> %s",
>> +                                          be_path);
>> +        return -1;
>> +    }
>> +
>> +    env = get_hotplug_env(gc, dev);
>> +    if (!env)
>> +        return -1;
>> +
>> +    f_args = flexarray_make(4, 1);
>> +    if (!f_args) {
>> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments
>> array");
>> +        return -1;
>> +    }
>> +
>> +    flexarray_set(f_args, nr++, script);
>> +    flexarray_set(f_args, nr++, action == CONNECT ? "online" :
>> "offline");
>> +    flexarray_set(f_args, nr++, "type_if=vif");
>> +    flexarray_set(f_args, nr++, NULL);
>> +
>> +    args = (char **) flexarray_contents(f_args);
>> +    what = libxl__sprintf(gc, "%s %s", args[0],
>> +                          action == CONNECT ? "connect" : "disconnect");
>> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
>> +               "Calling hotplug script: %s %s %s",
>> +               args[0], args[1], args[2]);
>> +    status = libxl__forkexec(gc, -1, -1, -1, args[0], args, env, what);
>> +    if (status) {
>> +        rc = -1;
>> +        goto out;
>> +    }
>> +    rc = 0;
>> +
>> +out:
>> +    free(env);
>> +    free(args);
>> +    return rc;
>> +}
>> +
>> +static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
>> +                       libxl__hotplug_action action)
>> +{
>> +    libxl_ctx *ctx = libxl__gc_owner(gc);
>> +    char *be_path = libxl__device_backend_path(gc, dev);
>> +    char *script, *what;
>> +    char **args, **env;
>> +    int status, nr = 0;
>> +    int rc = -1;
>> +    flexarray_t *f_args;
>> +
>> +    script = libxl__xs_read(gc, XBT_NULL,
>> +                            libxl__sprintf(gc, "%s/%s", be_path,
>> "script"));
>> +    if (!script) {
>> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from
>> %s",
>> +                                          be_path);
>> +        return -1;
>> +    }
>> +
>> +    env = get_hotplug_env(gc, dev);
>> +    if (!env)
>> +        return -1;
>> +
>> +    f_args = flexarray_make(3, 1);
>> +    if (!f_args) {
>> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments
>> array");
>> +        return -1;
>> +    }
>> +
>> +    flexarray_set(f_args, nr++, script);
>> +    flexarray_set(f_args, nr++, action == CONNECT ? "add" : "remove");
>> +    flexarray_set(f_args, nr++, NULL);
>> +
>> +    args = (char **) flexarray_contents(f_args);
>> +    what = libxl__sprintf(gc, "%s %s", args[0],
>> +                          action == CONNECT ? "connect" : "disconnect");
>> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
>> +               "Calling hotplug script: %s %s",
>> +               args[0], args[1]);
>> +    status = libxl__forkexec(gc, -1, -1, -1, args[0], args, env, what);
>> +    if (status) {
>> +        rc = -1;
>> +        goto out;
>> +    }
>> +    rc = 0;
>> +
>> +out:
>> +    free(env);
>> +    free(args);
>> +    return rc;
>> +}
>> +#endif /* 0 */
>>  int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
>>                           libxl__hotplug_action action)
>>  {
>> -    return 0;
>> +    int rc = 0;
>> +#if 0
>> +    switch (dev->backend_kind) {
>> +    case LIBXL__DEVICE_KIND_VIF:
>> +        rc = libxl__hotplug_nic(gc, dev, action);
>> +        break;
>> +    case LIBXL__DEVICE_KIND_VBD:
>> +        rc = libxl__hotplug_disk(gc, dev, action);
>> +        break;
>> +    default:
>> +        rc = 0;
>> +        break;
>> +    }
>> +#endif /* 0 */
>> +    return rc;
>>  }
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>

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

[PATCH 14 of 29 RFC] rc.d NetBSD: don't start xenbackendd by default, only when xend needs it

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1317386335 -7200
# Node ID 6a7a2cc36fe7cadd7d0a50f6f265b6833a7b4e1f
# Parent  98a4f01033786a2fc15b4130897c35f64a676e40
rc.d NetBSD: don't start xenbackendd by default, only when xend needs it.

With the move of hotplug scripts from xenbackendd to libxl
xenbackendd is no longer needed by libxl, only start it when xend is
started.

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

diff -r 98a4f0103378 -r 6a7a2cc36fe7 tools/hotplug/NetBSD/rc.d/xenbackendd
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/hotplug/NetBSD/rc.d/xenbackendd	Fri Sep 30 14:38:55 2011 +0200
@@ -0,0 +1,27 @@
+#!/bin/sh
+#
+# PROVIDE: xenbackendd
+# REQUIRE: xencommons
+
+. /etc/rc.subr
+
+DIR=$(dirname "$0")
+. "${DIR}/xen-hotplugpath.sh"
+
+LD_LIBRARY_PATH="${LIBDIR}"
+export LD_LIBRARY_PATH PYTHONPATH
+PATH="${PATH}:${SBINDIR}"
+export PATH
+
+name="xenbackendd"
+rcvar=$name
+command="${SBINDIR}/xenbackendd"
+if [ -n "${XENBACKENDD_DEBUG}" ]; then
+	command_args="${XENBACKENDD_ARGS} -d"
+else
+	command_args="${XENBACKENDD_ARGS}"
+fi
+
+load_rc_config $name
+run_rc_command "$1"
+
diff -r 98a4f0103378 -r 6a7a2cc36fe7 tools/hotplug/NetBSD/rc.d/xencommons
--- a/tools/hotplug/NetBSD/rc.d/xencommons	Thu Feb 02 11:13:11 2012 +0100
+++ b/tools/hotplug/NetBSD/rc.d/xencommons	Fri Sep 30 14:38:55 2011 +0200
@@ -22,8 +22,6 @@ required_files="/kern/xen/privcmd"
 
 XENSTORED_PIDFILE="/var/run/xenstored.pid"
 XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid"
-XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid"
-#XENBACKENDD_DEBUG=1
 #XENCONSOLED_TRACE="/var/log/xen/xenconsole-trace.log"
 #XENSTORED_TRACE="/var/log/xen/xenstore-trace.log"
 
@@ -46,7 +44,7 @@ xen_startcmd()
 			XENSTORED_ROOTDIR="/var/lib/xenstored"
 		fi
 		rm -f ${XENSTORED_ROOTDIR}/tdb* >/dev/null 2>&1
-		printf "Starting xenservices: xenstored, xenconsoled, xenbackendd."
+		printf "Starting xenservices: xenstored, xenconsoled."
 		XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}"
 		if [ -n "${XENSTORED_TRACE}" ]; then
 			XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log"
@@ -58,7 +56,7 @@ xen_startcmd()
 			sleep 1
 		done
 	else
-		printf "Starting xenservices: xenconsoled, xenbackendd."
+		printf "Starting xenservices: xenconsoled."
 	fi
 
 	XENCONSOLED_ARGS=""
@@ -68,13 +66,6 @@ xen_startcmd()
 
 	${SBINDIR}/xenconsoled ${XENCONSOLED_ARGS}
 
-	XENBACKENDD_ARGS=""
-	if [ -n "${XENBACKENDD_DEBUG}" ]; then
-		XENBACKENDD_ARGS="${XENBACKENDD_ARGS} -d"
-	fi
-
-	${SBINDIR}/xenbackendd ${XENBACKENDD_ARGS}
-
 	printf "\n"
 
 	printf "Setting domain 0 name.\n"
@@ -87,8 +78,6 @@ xen_stop()
 	printf "Stopping xencommons.\n"
 	printf "WARNING: Not stopping xenstored, as it cannot be restarted.\n"
 
-	rc_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
-	pids="$pids $rc_pid"
 	rc_pid=$(check_pidfile ${XENCONSOLED_PIDFILE} ${SBINDIR}/xenconsoled)
 	pids="$pids $rc_pid"
 
@@ -108,17 +97,12 @@ xen_status()
 		pids="$pids $xenconsoled_pid"
 	fi
 
-	xenbackend_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
-	if test -n ${xenbackend_pid}; then
-		pids="$pids $xenbackend_pid"
-	fi
-
-	if test -n "$xenbackend_pid" -a -n "$xenconsoled_pid" -a -n "$xenstored_pid";
+	if test -n "$xenconsoled_pid" -a -n "$xenstored_pid";
 	then
 		echo "xencommons are running as pids $pids."
 		return 0
 	fi
-	if test -z "$xenbackend_pid" -a -z "$xenconsoled_pid" -a -z "$xenstored_pid";
+	if test -z "$xenconsoled_pid" -a -z "$xenstored_pid";
 	then
 		echo "xencommons are not running."
 		return 0
@@ -134,11 +118,6 @@ xen_status()
 	else
 		echo "xenconsoled is not running."
 	fi
-	if test -n $xenbackend_pid; then
-		echo "xenbackendd is running as pid $xenbackend_pid."
-	else
-		echo "xenbackendd is not running."
-	fi
 }
 
 load_rc_config $name
diff -r 98a4f0103378 -r 6a7a2cc36fe7 tools/hotplug/NetBSD/rc.d/xend
--- a/tools/hotplug/NetBSD/rc.d/xend	Thu Feb 02 11:13:11 2012 +0100
+++ b/tools/hotplug/NetBSD/rc.d/xend	Fri Sep 30 14:38:55 2011 +0200
@@ -1,7 +1,7 @@
 #!/bin/sh
 #
 # PROVIDE: xend
-# REQUIRE: xencommons
+# REQUIRE: xencommons xenbackendd
 
 . /etc/rc.subr

[PATCH 15 of 29 RFC] NetBSD/xencommons: remove xend precmd folder creation

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1326564288 -3600
# Node ID a7ef1bfa694bf581310e0242616a480e1c5e61b7
# Parent  6a7a2cc36fe7cadd7d0a50f6f265b6833a7b4e1f
NetBSD/xencommons: remove xend precmd folder creation

Since xend is not started by xencommons move the creation of the
necessary xend folders to xend init script.

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

diff -r 6a7a2cc36fe7 -r a7ef1bfa694b tools/hotplug/NetBSD/rc.d/xencommons
--- a/tools/hotplug/NetBSD/rc.d/xencommons	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/hotplug/NetBSD/rc.d/xencommons	Sat Jan 14 19:04:48 2012 +0100
@@ -27,8 +27,6 @@ XENCONSOLED_PIDFILE="/var/run/xenconsole
 
 xen_precmd()
 {
-	mkdir -p /var/run/xend || exit 1
-	mkdir -p /var/run/xend/boot || exit 1
 	mkdir -p /var/run/xenstored || exit 1
 }
 
diff -r 6a7a2cc36fe7 -r a7ef1bfa694b tools/hotplug/NetBSD/rc.d/xend
--- a/tools/hotplug/NetBSD/rc.d/xend	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/hotplug/NetBSD/rc.d/xend	Sat Jan 14 19:04:48 2012 +0100
@@ -15,10 +15,17 @@ export PATH
 
 name="xend"
 rcvar=$name
+start_precmd="xen_precmd"
 command="${SBINDIR}/xend"
 command_args="start"
 command_interpreter=`head -n 1 ${command} | awk '{ print substr($0,3) }'`
 sig_stop="SIGKILL"
 
+xen_precmd()
+{
+	mkdir -p /var/run/xend || exit 1
+	mkdir -p /var/run/xend/boot || exit 1
+}
+
 load_rc_config $name
 run_rc_command "$1"

Re: [PATCH 15 of 29 RFC] NetBSD/xencommons: remove xend precmd folder creation

[Ian Jackson]

Roger Pau Monne writes ("[Xen-devel] [PATCH 15 of 29 RFC] NetBSD/xencommons: remove xend precmd folder creation"):
> NetBSD/xencommons: remove xend precmd folder creation
> 
> Since xend is not started by xencommons move the creation of the
> necessary xend folders to xend init script.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

This looks plausible but I would call them "directories".  "folders"
is strictly a UI term for us I think.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

[PATCH 16 of 29 RFC] libxl: introduce libxl__device_hotplug_path

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328178086 -3600
# Node ID 88e1905ef078040a1d943ae08186373e2f4d3857
# Parent  a7ef1bfa694bf581310e0242616a480e1c5e61b7
libxl: introduce libxl__device_hotplug_path

Get xenstore hotplug path from a given libxl__device. Used in the same
way as libxl__device_frontend_path or libxl__device_backend_path.

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

diff -r a7ef1bfa694b -r 88e1905ef078 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Sat Jan 14 19:04:48 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 11:21:26 2012 +0100
@@ -40,6 +40,14 @@ char *libxl__device_backend_path(libxl__
                           device->domid, device->devid);
 }
 
+char *libxl__device_hotplug_path(libxl__gc *gc, libxl__device *device)
+{
+    return libxl__sprintf(gc, "/hotplug/%u/%u/%s/%u", device->backend_domid,
+                          device->domid,
+                          libxl__device_kind_to_string(device->backend_kind),
+                          device->devid);
+}
+
 int libxl__parse_backend_path(libxl__gc *gc,
                               const char *path,
                               libxl__device *dev)
diff -r a7ef1bfa694b -r 88e1905ef078 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Sat Jan 14 19:04:48 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 11:21:26 2012 +0100
@@ -301,6 +301,7 @@ typedef struct {
                              char **bents, char **fents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
+_hidden char *libxl__device_hotplug_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
                                       libxl__device *dev);
 _hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);

[PATCH 17 of 29 RFC] libxl: add enum with possible hotplug state

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328178267 -3600
# Node ID 6a6e7a195412935609c18bc7d52767f41521875b
# Parent  88e1905ef078040a1d943ae08186373e2f4d3857
libxl: add enum with possible hotplug state

enum describing the possible state of the hotplug device entries in
xenstore.

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

diff -r 88e1905ef078 -r 6a6e7a195412 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 11:21:26 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 11:24:27 2012 +0100
@@ -351,6 +351,17 @@ typedef enum {
     DISCONNECT = 2
 } libxl__hotplug_action;
 
+/* Possible hotplug state in xenstore */
+typedef enum {
+    HOTPLUG_DEVICE_ERROR = -1,
+    HOTPLUG_DEVICE_INIT = 0,
+    HOTPLUG_DEVICE_CONNECT = 1,
+    HOTPLUG_DEVICE_CONNECTED = 2,
+    HOTPLUG_DEVICE_DISCONNECT = 3,
+    HOTPLUG_DEVICE_DISCONNECTED = 4,
+    HOTPLUG_DEVICE_FORCE_DISCONNECT = 5,
+} libxl__hotplug_status;
+
 /*
  * libxl__device_hotplug - generic function to execute hotplug scripts
  * gc: allocation pool

[PATCH 18 of 29 RFC] libxl: introduce libxl__device_generic_hotplug_add

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328178447 -3600
# Node ID 94733bc9cc2fbeaa08a660ec90dcd9c5ad924e1b
# Parent  6a6e7a195412935609c18bc7d52767f41521875b
libxl: introduce libxl__device_generic_hotplug_add

Add an array of xenstore entries for the given libxl__device to
xenstore.

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

diff -r 6a6e7a195412 -r 94733bc9cc2f tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 11:24:27 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 11:27:27 2012 +0100
@@ -118,6 +118,42 @@ retry_transaction:
     return 0;
 }
 
+int libxl__device_generic_hotplug_add(libxl__gc *gc, libxl__device *device,
+                                      char **hents)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *hotplug_path;
+    xs_transaction_t t;
+    struct xs_permissions hotplug_perms[2];
+
+    hotplug_path = libxl__device_hotplug_path(gc, device);
+
+    hotplug_perms[0].id = 0;
+    hotplug_perms[0].perms = XS_PERM_NONE;
+    hotplug_perms[1].id = device->backend_domid;
+    hotplug_perms[1].perms = XS_PERM_NONE;
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+
+    if (hents) {
+        xs_rm(ctx->xsh, t, hotplug_path);
+        xs_mkdir(ctx->xsh, t, hotplug_path);
+        xs_set_permissions(ctx->xsh, t, hotplug_path, hotplug_perms,
+                           ARRAY_SIZE(hotplug_perms));
+        libxl__xs_writev(gc, t, hotplug_path, hents);
+    }
+
+    if (!xs_transaction_end(ctx->xsh, t, 0)) {
+        if (errno == EAGAIN)
+            goto retry_transaction;
+        else
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs transaction failed");
+    }
+
+    return 0;
+}
+
 typedef struct {
     libxl__gc *gc;
     libxl_device_disk *disk;
diff -r 6a6e7a195412 -r 94733bc9cc2f tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 11:24:27 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 11:27:27 2012 +0100
@@ -299,6 +299,9 @@ typedef struct {
 
 _hidden int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
                              char **bents, char **fents);
+_hidden int libxl__device_generic_hotplug_add(libxl__gc *gc,
+                                              libxl__device *device,
+                                              char **hents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_hotplug_path(libxl__gc *gc, libxl__device *device);

[PATCH 19 of 29 RFC] libxl: add libxl__device_hotplug_disconnect

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328178627 -3600
# Node ID 937bbe68a1942e22c40aa18c8bb66490aec56945
# Parent  94733bc9cc2fbeaa08a660ec90dcd9c5ad924e1b
libxl: add libxl__device_hotplug_disconnect

Sets the necessary xenstore entries to trigger the unplug of a device,
and waits for the device to react (either by setting the device state
to disconnected or to error).

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

diff -r 94733bc9cc2f -r 937bbe68a194 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 11:27:27 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 11:30:27 2012 +0100
@@ -405,7 +405,7 @@ int libxl__device_disk_dev_number(const 
  * or timeout occurred.
  */
 int libxl__wait_for_device_state(libxl__gc *gc, struct timeval *tv,
-                                 XenbusState state,
+                                 int state,
                                  libxl__device_state_handler handler)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -413,6 +413,7 @@ int libxl__wait_for_device_state(libxl__
     unsigned int n;
     fd_set rfds;
     char **l1 = NULL;
+    char *entry;
 
 start:
     rc = 1;
@@ -431,6 +432,10 @@ start:
         default:
             l1 = xs_read_watch(ctx->xsh, &n);
             if (l1 != NULL) {
+                entry = strrchr(l1[0], '/');
+                if (!entry || strcmp(entry, "/state"))
+                    goto start;
+
                 char *sstate = libxl__xs_read(gc, XBT_NULL,
                                              l1[XS_WATCH_PATH]);
                 if (!sstate || atoi(sstate) == state) {
@@ -575,6 +580,71 @@ out:
     return rc;
 }
 
+int libxl__device_hotplug_disconnect(libxl__gc *gc, libxl__device *dev,
+                                     libxl__hotplug_status disconnect_param)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    xs_transaction_t t;
+    char *hotplug_path = libxl__device_hotplug_path(gc, dev);
+    char *state_path = libxl__sprintf(gc, "%s/state", hotplug_path);
+    char *state;
+    struct timeval tv;
+    int rc = 0;
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+    state = libxl__xs_read(gc, t, state_path);
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disconnecting device with path %s and "
+                                      "state %s", hotplug_path, state);
+    if (!state) {
+        xs_transaction_end(ctx->xsh, t, 0);
+        goto out;
+    }
+    if (atoi(state) != HOTPLUG_DEVICE_CONNECTED) {
+        xs_transaction_end(ctx->xsh, t, 0);
+        goto out;
+    }
+    libxl__xs_write(gc, t, state_path, "%d", disconnect_param);
+    if (!xs_transaction_end(ctx->xsh, t, 0)) {
+        if (errno == EAGAIN)
+            goto retry_transaction;
+        else {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+    }
+
+    xs_watch(ctx->xsh, state_path, hotplug_path);
+
+    tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+    tv.tv_usec = 0;
+    rc = libxl__wait_for_device_state(gc, &tv, HOTPLUG_DEVICE_DISCONNECTED,
+                                      NULL);
+    xs_unwatch(ctx->xsh, state_path, hotplug_path);
+    state = libxl__xs_read(gc, XBT_NULL, state_path);
+    if (!state) {
+        goto out;
+    }
+    if (atoi(state) == HOTPLUG_DEVICE_ERROR) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "Error while unplug of "
+                   "device with hotplug path %s", hotplug_path);
+        rc = ERROR_FAIL;
+    } else if (rc == ERROR_TIMEDOUT) {
+        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+                   "Timeout while waiting for unplug of "
+                   "device with hotplug path %s", hotplug_path);
+    } else if (rc == ERROR_FAIL) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "failed to destroy device with "
+                   "hotplug path %s", hotplug_path);
+    }
+
+out:
+    libxl__xs_path_cleanup(gc, hotplug_path);
+    return rc;
+}
+
 int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
 {
     char *be_path = libxl__device_backend_path(gc, dev);
diff -r 94733bc9cc2f -r 937bbe68a194 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 11:27:27 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 11:30:27 2012 +0100
@@ -334,7 +334,7 @@ typedef int libxl__device_state_handler(
  * Returns 0 on success, and < 0 on error.
  */
 _hidden int libxl__wait_for_device_state(libxl__gc *gc, struct timeval *tv,
-                                         XenbusState state,
+                                         int state,
                                          libxl__device_state_handler handler);
 
 /*
@@ -366,6 +366,16 @@ typedef enum {
 } libxl__hotplug_status;
 
 /*
+ * libxl__device_hotplug_disconnect - disconnect remove device
+ * disconnect_param: action to use when disconnecting the device, either
+ * HOTPLUG_DEVICE_DISCONNECT or HOTPLUG_DEVICE_FORCE_DISCONNECT
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__device_hotplug_disconnect(libxl__gc *gc, libxl__device *dev,
+                                       libxl__hotplug_status disconnect_param);
+
+/*
  * libxl__device_hotplug - generic function to execute hotplug scripts
  * gc: allocation pool
  * dev: reference to the device that executes the hotplug scripts

[PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328178807 -3600
# Node ID bc38aa2e11e21b34dbc29a81f069133a3276848a
# Parent  937bbe68a1942e22c40aa18c8bb66490aec56945
libxl: introduce libxl hotplug public API functions

These functions mimic the name used by the local device functions,
following the nomenclature:

libxl_device_<type>_hotplug_<action>

They also take the same parameters as they local counterparts (ctx,
domid and libxl_device_<type>). The list of added functions:

libxl_device_disk_hotplug_add
libxl_device_disk_hotplug_remove
libxl_device_disk_hotplug_destroy
libxl_device_nic_hotplug_add
libxl_device_nic_hotplug_remove
libxl_device_nic_hotplug_destroy

The xenstore structure used by vbd disk entries:

/hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>
/hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/pdev_path
/hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/vdev
/hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/script
/hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/removable
/hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/readwrite
/hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/is_cdrom
/hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/state

and vif devices use the following:

/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>
/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mtu
/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/model
/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mac
/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ip
/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/bridge
/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ifname
/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/script
/hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/nictype

This will allow us to pass the libxl_device_disk and libxl_device_nic
struct from Dom0 to driver domain, and execute the necessary backends
there.

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

diff -r 937bbe68a194 -r bc38aa2e11e2 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 11:30:27 2012 +0100
+++ b/tools/libxl/libxl.c	Thu Feb 02 11:33:27 2012 +0100
@@ -996,6 +996,85 @@ static int libxl__device_from_disk(libxl
     return 0;
 }
 
+int libxl_device_disk_hotplug_add(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_device_disk *disk)
+{
+    GC_INIT(ctx);
+    flexarray_t *hotplug;
+    libxl__device device;
+    char *h_path, *state_path, *state;
+    struct timeval tv;
+    int rc;
+
+    /* 
+     * Always set backend to PHY and let the driver domain decide the most
+     * suitable backend to use
+     */
+    disk->backend = LIBXL_DISK_BACKEND_PHY;
+
+    hotplug = flexarray_make(14, 1);
+    if (!hotplug) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+
+    rc = libxl__device_from_disk(gc, domid, disk, &device);
+    if (rc != 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
+               " virtual disk identifier %s", disk->vdev);
+        goto out_free;
+    }
+
+    flexarray_append(hotplug, "pdev_path");
+    flexarray_append(hotplug, libxl__strdup(gc, disk->pdev_path));
+    flexarray_append(hotplug, "vdev");
+    flexarray_append(hotplug, libxl__strdup(gc, disk->vdev));
+    if (disk->script) {
+        flexarray_append(hotplug, "script");
+        flexarray_append(hotplug, libxl__strdup(gc, disk->script));
+    }
+    flexarray_append(hotplug, "removable");
+    flexarray_append(hotplug, libxl__sprintf(gc, "%d", disk->removable));
+    flexarray_append(hotplug, "readwrite");
+    flexarray_append(hotplug, libxl__sprintf(gc, "%d", disk->readwrite));
+    flexarray_append(hotplug, "is_cdrom");
+    flexarray_append(hotplug, libxl__sprintf(gc, "%d", disk->is_cdrom));
+    flexarray_append(hotplug, "format");
+    flexarray_append(hotplug, libxl__sprintf(gc, "%d", disk->format));
+    flexarray_append(hotplug, "state");
+    flexarray_append(hotplug, "1");
+
+    libxl__device_generic_hotplug_add(gc, &device,
+        libxl__xs_kvs_of_flexarray(gc, hotplug, hotplug->count));
+
+    /* Wait for device init or error */
+    h_path = libxl__device_hotplug_path(gc, &device);
+    state_path = libxl__sprintf(gc, "%s/state", h_path);
+    state = libxl__xs_read(gc, XBT_NULL, state_path);
+
+    if (atoi(state) != HOTPLUG_DEVICE_CONNECTED) {
+        xs_watch(ctx->xsh, state_path, h_path);
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        rc = libxl__wait_for_device_state(gc, &tv, HOTPLUG_DEVICE_CONNECTED,
+                                          NULL);
+        xs_unwatch(ctx->xsh, state_path, h_path);
+        if (rc < 0) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "unable to initialize disk device: %s",
+                       disk->pdev_path);
+            goto out_free;
+        }
+    }
+
+    rc = 0;
+out_free:
+    flexarray_free(hotplug);
+out:
+    GC_FREE;
+    return rc;
+}
+
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
 {
     GC_INIT(ctx);
@@ -1174,6 +1253,23 @@ out:
     return rc;
 }
 
+int libxl_device_disk_hotplug_remove(libxl_ctx *ctx, uint32_t domid,
+                                     libxl_device_disk *disk)
+{
+    GC_INIT(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_disk(gc, domid, disk, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_hotplug_disconnect(gc, &device,
+                                          HOTPLUG_DEVICE_DISCONNECT);
+out:
+    GC_FREE;
+    return rc;
+}
+
 int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_disk *disk)
 {
@@ -1190,6 +1286,23 @@ out:
     return rc;
 }
 
+int libxl_device_disk_hotplug_destroy(libxl_ctx *ctx, uint32_t domid,
+                                      libxl_device_disk *disk)
+{
+    GC_INIT(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_disk(gc, domid, disk, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_hotplug_disconnect(gc, &device,
+                                          HOTPLUG_DEVICE_FORCE_DISCONNECT);
+out:
+    GC_FREE;
+    return rc;
+}
+
 static void libxl__device_disk_from_xs_be(libxl__gc *gc,
                                           const char *be_path,
                                           libxl_device_disk *disk)
@@ -1516,6 +1629,85 @@ static int libxl__device_from_nic(libxl_
     return 0;
 }
 
+int libxl_device_nic_hotplug_add(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_device_nic *nic)
+{
+    GC_INIT(ctx);
+    flexarray_t *hotplug;
+    libxl__device device;
+    char *h_path, *state_path, *state;
+    struct timeval tv;
+    int rc;
+
+    hotplug = flexarray_make(18, 1);
+    if (!hotplug) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+
+    rc = libxl__device_from_nic(gc, domid, nic, &device);
+    if (rc != 0)
+        goto out_free;
+
+    flexarray_append(hotplug, "mtu");
+    flexarray_append(hotplug, libxl__sprintf(gc, "%d", nic->mtu));
+    if (nic->model) {
+        flexarray_append(hotplug, "model");
+        flexarray_append(hotplug, libxl__strdup(gc, nic->model));
+    }
+    flexarray_append(hotplug, "mac");
+    flexarray_append(hotplug, libxl__sprintf(gc,
+                                    LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
+    if (nic->ip) {
+        flexarray_append(hotplug, "ip");
+        flexarray_append(hotplug, libxl__strdup(gc, nic->ip));
+    }
+    flexarray_append(hotplug, "bridge");
+    flexarray_append(hotplug, libxl__strdup(gc, nic->bridge));
+    if (nic->ifname) {
+        flexarray_append(hotplug, "ifname");
+        flexarray_append(hotplug, libxl__strdup(gc, nic->ifname));
+    }
+    if (nic->script) {
+        flexarray_append(hotplug, "script");
+        flexarray_append(hotplug, libxl__strdup(gc, nic->script));
+    }
+    flexarray_append(hotplug, "nictype");
+    flexarray_append(hotplug, libxl__sprintf(gc, "%d", nic->nictype));
+    flexarray_append(hotplug, "state");
+    flexarray_append(hotplug, "1");
+
+    libxl__device_generic_hotplug_add(gc, &device,
+        libxl__xs_kvs_of_flexarray(gc, hotplug, hotplug->count));
+
+    /* Wait for device init or error */
+    h_path = libxl__device_hotplug_path(gc, &device);
+    state_path = libxl__sprintf(gc, "%s/state", h_path);
+    state = libxl__xs_read(gc, XBT_NULL, state_path);
+
+    if (atoi(state) != HOTPLUG_DEVICE_CONNECTED) {
+        xs_watch(ctx->xsh, state_path, h_path);
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        rc = libxl__wait_for_device_state(gc, &tv, HOTPLUG_DEVICE_CONNECTED,
+                                          NULL);
+        xs_unwatch(ctx->xsh, state_path, h_path);
+        if (rc < 0) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "unable to initialize nic device: %s",
+                       nic->ifname);
+            goto out_free;
+        }
+    }
+
+    rc = 0;
+out_free:
+    flexarray_free(hotplug);
+out:
+    GC_FREE;
+    return rc;
+}
+
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
 {
     GC_INIT(ctx);
@@ -1652,6 +1844,22 @@ out:
     return rc;
 }
 
+int libxl_device_nic_hotplug_remove(libxl_ctx *ctx, uint32_t domid,
+                                    libxl_device_nic *nic)
+{
+    GC_INIT(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_nic(gc, domid, nic, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_hotplug_disconnect(gc, &device, HOTPLUG_DEVICE_DISCONNECT);
+out:
+    GC_FREE;
+    return rc;
+}
+
 int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid,
                                   libxl_device_nic *nic)
 {
@@ -1668,6 +1876,23 @@ out:
     return rc;
 }
 
+int libxl_device_nic_hotplug_destroy(libxl_ctx *ctx, uint32_t domid,
+                                     libxl_device_nic *nic)
+{
+    GC_INIT(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_nic(gc, domid, nic, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_hotplug_disconnect(gc, &device,
+                                          HOTPLUG_DEVICE_FORCE_DISCONNECT);
+out:
+    GC_FREE;
+    return rc;
+}
+
 static void libxl__device_nic_from_xs_be(libxl__gc *gc,
                                          const char *be_path,
                                          libxl_device_nic *nic)
diff -r 937bbe68a194 -r bc38aa2e11e2 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Thu Feb 02 11:30:27 2012 +0100
+++ b/tools/libxl/libxl.h	Thu Feb 02 11:33:27 2012 +0100
@@ -416,6 +416,11 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  *
  *    Initalises device to a default configuration.
  *
+ * libxl_device_<type>_hotplug_add(ctx, domid, device):
+ *
+ *   Creates the necessary xenstore entries to trigger the execution of
+ *   a device addition, possibly on a different domain.
+ *
  * libxl_device_<type>_add(ctx, domid, device):
  *
  *   Adds the given device to the specified domain. This can be called
@@ -425,6 +430,10 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  *   domain to connect to the device and therefore cannot block on the
  *   guest.
  *
+ * libxl_device_<type>_hotplug_remove(ctx, domid, device):
+ *
+ *   Request the removal of the given device and waits for the result.
+ *
  * libxl_device_<type>_remove(ctx, domid, device):
  *
  *   Removes the given device from the specified domain by performing
@@ -442,14 +451,25 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  *
  *   This function does not interact with the guest and therefore
  *   cannot block on the guest.
+ *
+ * libxl_device_<type>_hotplug_destroy(ctx, domid, device):
+ *
+ *   Requests de destruction of the given device and waits for the result.
+ *
  */
 
 /* Disks */
 int libxl_device_disk_init(libxl_ctx *ctx, libxl_device_disk *disk);
+int libxl_device_disk_hotplug_add(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_device_disk *disk);
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
 int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
+int libxl_device_disk_hotplug_remove(libxl_ctx *ctx, uint32_t domid,
+                                     libxl_device_disk *disk);
 int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_disk *disk);
+int libxl_device_disk_hotplug_destroy(libxl_ctx *ctx, uint32_t domid,
+                                      libxl_device_disk *disk);
 
 libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num);
 int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -470,9 +490,15 @@ int libxl_device_disk_local_detach(libxl
 
 /* Network Interfaces */
 int libxl_device_nic_init(libxl_ctx *ctx, libxl_device_nic *nic);
+int libxl_device_nic_hotplug_add(libxl_ctx *ctx, uint32_t domid,
+                                 libxl_device_nic *nic);
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
 int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
+int libxl_device_nic_hotplug_remove(libxl_ctx *ctx, uint32_t domid,
+                                    libxl_device_nic *nic);
 int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
+int libxl_device_nic_hotplug_destroy(libxl_ctx *ctx, uint32_t domid,
+                                     libxl_device_nic *nic);
 
 libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num);
 int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Jackson]

Roger Pau Monne writes ("[Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> libxl: introduce libxl hotplug public API functions
> 
> These functions mimic the name used by the local device functions,
> following the nomenclature:

I think these should be private functions.  Your new device daemon is,
I think, part of the implementation of libxl, not something that
another toolstack on top of libxl will reasonably want to replace.

This is a new departure for libxl, which doesn't currently have any
internal executables.

> The xenstore structure used by vbd disk entries:
> 
> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>
> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/pdev_path
> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/vdev
> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/script
> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/removable
> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/readwrite
> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/is_cdrom
> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/state
> 
> and vif devices use the following:
> 
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mtu
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/model
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mac
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ip
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/bridge
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ifname
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/script
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/nictype
> 
> This will allow us to pass the libxl_device_disk and libxl_device_nic
> struct from Dom0 to driver domain, and execute the necessary backends
> there.

Is this really the best encoding of this information in xenstore ?

If we're allowed to assume that the backendd is a libxl program too,
and of a reasonably similar version, perhaps we should have some kind
of idl-generated mapping from xenstore keys to struct members ?

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Stefano Stabellini]

On Wed, 8 Feb 2012, Ian Jackson wrote:
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mtu
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/model
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mac
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ip
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/bridge
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ifname
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/script
> > /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/nictype
> > 
> > This will allow us to pass the libxl_device_disk and libxl_device_nic
> > struct from Dom0 to driver domain, and execute the necessary backends
> > there.
> 
> Is this really the best encoding of this information in xenstore ?
> 
> If we're allowed to assume that the backendd is a libxl program too,
> and of a reasonably similar version, perhaps we should have some kind
> of idl-generated mapping from xenstore keys to struct members ?

You have a point, however using the same encoding has two advantages:

- it is well understood and lots of lines of code have been written in many
toolstacks to support it;

- we can reuse the "state" based mechanism to establish a connection:
again not a great protocol, but very well known and understood.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> - we can reuse the "state" based mechanism to establish a connection:
> again not a great protocol, but very well known and understood.

I don't think we have, in general, a good understanding of these
"state" based protocols ...

Also for an internal mechanism I think we should have something
autogenerated so avoid having lots of handwritten parsing/emitting code.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Stefano Stabellini]

On Thu, 9 Feb 2012, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > - we can reuse the "state" based mechanism to establish a connection:
> > again not a great protocol, but very well known and understood.
> 
> I don't think we have, in general, a good understanding of these
> "state" based protocols ...

What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!


> Also for an internal mechanism I think we should have something
> autogenerated so avoid having lots of handwritten parsing/emitting code.
 
If I am not mistaken in this series we are reusing existing code, not
adding new one.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> On Thu, 9 Feb 2012, Ian Jackson wrote:
> > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > - we can reuse the "state" based mechanism to establish a connection:
> > > again not a great protocol, but very well known and understood.
> > 
> > I don't think we have, in general, a good understanding of these
> > "state" based protocols ...
> 
> What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
> kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!

In many (most?) of these cases we don't know exactly what the state
numbers mean, let alone exactly the details of the rest of the
protocol.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Stefano Stabellini]

On Thu, 9 Feb 2012, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > On Thu, 9 Feb 2012, Ian Jackson wrote:
> > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > > - we can reuse the "state" based mechanism to establish a connection:
> > > > again not a great protocol, but very well known and understood.
> > > 
> > > I don't think we have, in general, a good understanding of these
> > > "state" based protocols ...
> > 
> > What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
> > kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!
> 
> In many (most?) of these cases we don't know exactly what the state
> numbers mean

see xen/include/public/io/xenbus.h


> let alone exactly the details of the rest of the
> protocol.
 
The details of the rest of the protocol are a well known contract
between frontend, backend and the toolstack. In fact they work with
different toolstacks (xend, libxl, xapi), different backend
implementations (Linux, QEMU), different frontends (Linux, Windows PV
drivers). And the examples I mentioned are just the ones that I happen
to work with.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> The details of the rest of the protocol are a well known contract
> between frontend, backend and the toolstack. In fact they work with
> different toolstacks (xend, libxl, xapi), different backend
> implementations (Linux, QEMU), different frontends (Linux, Windows PV
> drivers). And the examples I mentioned are just the ones that I happen
> to work with.

If they are so well known, please point me to the detailed protocol
specification.

Oh wait you can't because Justin Gibbs hasn't finished the disk one
yet and no-one has started on the others.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Campbell]

On Thu, 2012-02-09 at 15:32 +0000, Stefano Stabellini wrote:
> On Thu, 9 Feb 2012, Ian Jackson wrote:
> > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > - we can reuse the "state" based mechanism to establish a connection:
> > > again not a great protocol, but very well known and understood.
> > 
> > I don't think we have, in general, a good understanding of these
> > "state" based protocols ...
> 
> What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
> kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!

And no one I know is able to describe, accurately, exactly what the
state diagram for even one of those actually looks like or indeed should
look like. It became quite evident in these threads about hotplug script
handling etc that no one really knows for sure what (is supposed to)
happens when.

Justin just posted a good description for blkif.h which included a state
machine description. We need the same for pciif.h, netif.h etc etc.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Stefano Stabellini]

On Thu, 9 Feb 2012, Ian Campbell wrote:
> On Thu, 2012-02-09 at 15:32 +0000, Stefano Stabellini wrote:
> > On Thu, 9 Feb 2012, Ian Jackson wrote:
> > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > > - we can reuse the "state" based mechanism to establish a connection:
> > > > again not a great protocol, but very well known and understood.
> > > 
> > > I don't think we have, in general, a good understanding of these
> > > "state" based protocols ...
> > 
> > What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
> > kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!
> 
> And no one I know is able to describe, accurately, exactly what the
> state diagram for even one of those actually looks like or indeed should
> look like. It became quite evident in these threads about hotplug script
> handling etc that no one really knows for sure what (is supposed to)
> happens when.

I thought that most of the thread was about the interface with the block
scripts, that is an entirely different matter and completely obscure.
If I am mistaken, please point me at the right email.


> Justin just posted a good description for blkif.h which included a state
> machine description. We need the same for pciif.h, netif.h etc etc.
 
The state machine is the same for block and network.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Campbell]

On Thu, 2012-02-09 at 16:00 +0000, Stefano Stabellini wrote:
> On Thu, 9 Feb 2012, Ian Campbell wrote:
> > On Thu, 2012-02-09 at 15:32 +0000, Stefano Stabellini wrote:
> > > On Thu, 9 Feb 2012, Ian Jackson wrote:
> > > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > > > - we can reuse the "state" based mechanism to establish a connection:
> > > > > again not a great protocol, but very well known and understood.
> > > > 
> > > > I don't think we have, in general, a good understanding of these
> > > > "state" based protocols ...
> > > 
> > > What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
> > > kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!
> > 
> > And no one I know is able to describe, accurately, exactly what the
> > state diagram for even one of those actually looks like or indeed should
> > look like. It became quite evident in these threads about hotplug script
> > handling etc that no one really knows for sure what (is supposed to)
> > happens when.
> 
> I thought that most of the thread was about the interface with the block
> scripts, that is an entirely different matter and completely obscure.
> If I am mistaken, please point me at the right email.

We are talking about reusing the existing xenbus state machine schema
for a new purpose. Ian J pointed out that these are not generally well
understood, you replied that it was and cited some examples. I pointed
out why these were not examples of why this stuff was well understood at
all, in fact quite the opposite.

> > Justin just posted a good description for blkif.h which included a state
> > machine description. We need the same for pciif.h, netif.h etc etc.
>  
> The state machine is the same for block and network.

No, it's not. This is exactly what IanJ and I are talking about.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Stefano Stabellini]

On Thu, 9 Feb 2012, Ian Campbell wrote:
> On Thu, 2012-02-09 at 16:00 +0000, Stefano Stabellini wrote:
> > On Thu, 9 Feb 2012, Ian Campbell wrote:
> > > On Thu, 2012-02-09 at 15:32 +0000, Stefano Stabellini wrote:
> > > > On Thu, 9 Feb 2012, Ian Jackson wrote:
> > > > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > > > > - we can reuse the "state" based mechanism to establish a connection:
> > > > > > again not a great protocol, but very well known and understood.
> > > > > 
> > > > > I don't think we have, in general, a good understanding of these
> > > > > "state" based protocols ...
> > > > 
> > > > What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
> > > > kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!
> > > 
> > > And no one I know is able to describe, accurately, exactly what the
> > > state diagram for even one of those actually looks like or indeed should
> > > look like. It became quite evident in these threads about hotplug script
> > > handling etc that no one really knows for sure what (is supposed to)
> > > happens when.
> > 
> > I thought that most of the thread was about the interface with the block
> > scripts, that is an entirely different matter and completely obscure.
> > If I am mistaken, please point me at the right email.
> 
> We are talking about reusing the existing xenbus state machine schema
> for a new purpose. Ian J pointed out that these are not generally well
> understood, you replied that it was and cited some examples. I pointed
> out why these were not examples of why this stuff was well understood at
> all, in fact quite the opposite.

Sorry but I don't understand how these examples are supposed to be
"quite the opposite".
I quite like the idea of being able to read a single source file of less
than 400 LOC to understand how a protocol works
(drivers/input/misc/xen-kbdfront.c).
In fact I don't think that understanding the protocol has been an issue
for the GSoC student that had to write a new one.
I think we are under influence of a "reiventing the wheel" virus.


> > > Justin just posted a good description for blkif.h which included a state
> > > machine description. We need the same for pciif.h, netif.h etc etc.
> >  
> > The state machine is the same for block and network.
> 
> No, it's not. This is exactly what IanJ and I are talking about.

Could you please elaborate?

I am sure you know that the xenstore state machine is handled the same
way for all the backends in QEMU (see hw/xen_backend.c).
And the same thing is true for the frontends and the backends in Linux.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Campbell]

On Thu, 2012-02-09 at 16:18 +0000, Stefano Stabellini wrote:
> On Thu, 9 Feb 2012, Ian Campbell wrote:
> > On Thu, 2012-02-09 at 16:00 +0000, Stefano Stabellini wrote:
> > > On Thu, 9 Feb 2012, Ian Campbell wrote:
> > > > On Thu, 2012-02-09 at 15:32 +0000, Stefano Stabellini wrote:
> > > > > On Thu, 9 Feb 2012, Ian Jackson wrote:
> > > > > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > > > > > - we can reuse the "state" based mechanism to establish a connection:
> > > > > > > again not a great protocol, but very well known and understood.
> > > > > > 
> > > > > > I don't think we have, in general, a good understanding of these
> > > > > > "state" based protocols ...
> > > > > 
> > > > > What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
> > > > > kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!
> > > > 
> > > > And no one I know is able to describe, accurately, exactly what the
> > > > state diagram for even one of those actually looks like or indeed should
> > > > look like. It became quite evident in these threads about hotplug script
> > > > handling etc that no one really knows for sure what (is supposed to)
> > > > happens when.
> > > 
> > > I thought that most of the thread was about the interface with the block
> > > scripts, that is an entirely different matter and completely obscure.
> > > If I am mistaken, please point me at the right email.
> > 
> > We are talking about reusing the existing xenbus state machine schema
> > for a new purpose. Ian J pointed out that these are not generally well
> > understood, you replied that it was and cited some examples. I pointed
> > out why these were not examples of why this stuff was well understood at
> > all, in fact quite the opposite.
> 
> Sorry but I don't understand how these examples are supposed to be
> "quite the opposite".
> I quite like the idea of being able to read a single source file of less
> than 400 LOC to understand how a protocol works
> (drivers/input/misc/xen-kbdfront.c).

That is not a protocol specification, merely one implementation of it.
What does the BSD driver do? Is it exactly the same as Linux? Should BSD
driver authors be expected to reverse engineer the protocol from the
Linux code? What/who arbitrates when the two behave differently?

> In fact I don't think that understanding the protocol has been an issue
> for the GSoC student that had to write a new one.

Being able to reverse engineer something which works is not proof that
these things are "well understood" in the general case.

> I think we are under influence of a "reiventing the wheel" virus.

I think we are in danger of making the same mistakes again as have been
made with the device protocols and this is what I want to avoid.

Now, perhaps this style of state machine protocol is a reasonable design
choice in this case, but since we are starting afresh here this specific
new instance should be well documented _up_front_ not left in the "oh,
just read the Linux code" state we have now for many of our devices
which has lead to multiple slightly divergent implementations of the
same basic concept.

> > > > Justin just posted a good description for blkif.h which included a state
> > > > machine description. We need the same for pciif.h, netif.h etc etc.
> > >  
> > > The state machine is the same for block and network.
> > 
> > No, it's not. This is exactly what IanJ and I are talking about.
> 
> Could you please elaborate?
> 
> I am sure you know that the xenstore state machine is handled the same
> way for all the backends in QEMU (see hw/xen_backend.c).
> And the same thing is true for the frontends and the backends in Linux.

A substantial proportion of the threads about this hotplug script stuff
has been about the fact that no one is quite sure what really happens
when for all implementations nor what the common semantics are.

e.g. How do you ask a backend to shut down (do you set it to state 5?
state 6? do you nuke the xenstore dir?). Neither is anyone sure when the
correct point to call the hotplug scripts actually is, or even what
actually happens with them right now across the different backend
drivers or kernel types.

The actual state transitions which netback and blkback go through are
not the same: The netback protocol uses InitWait, the blkback one does
not or is it vice-versa? I can't remember and it isn't documented. Some
Linux frontends handled the kexec reconnect sequencing differently, by
disconnecting or reconnecting the actual underlying devices at subtly
different times and/or handling the transition from Closing back to Init
or InitWait differently.

And this is just for Linux talking to Linux.

I know for sure that the Windows frontends follow a different state
transition path to Linux (and that it has interacted badly with the
kexec differences in the Linux backends discussed above). I bet BSD has
some subtle differences in behaviour too.

The fact is that none of our device state machine protocols are not well
documented (although blkif.h is about to be). If this stuff were well
understood we would already have such documentation because it would be
trivial to write -- but it is not. If you disagree then please document
the netif state machine protocol in the form of a patch to netif.h.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Stefano Stabellini]

On Thu, 9 Feb 2012, Ian Campbell wrote:
> On Thu, 2012-02-09 at 16:18 +0000, Stefano Stabellini wrote:
> > On Thu, 9 Feb 2012, Ian Campbell wrote:
> > > On Thu, 2012-02-09 at 16:00 +0000, Stefano Stabellini wrote:
> > > > On Thu, 9 Feb 2012, Ian Campbell wrote:
> > > > > On Thu, 2012-02-09 at 15:32 +0000, Stefano Stabellini wrote:
> > > > > > On Thu, 9 Feb 2012, Ian Jackson wrote:
> > > > > > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > > > > > > - we can reuse the "state" based mechanism to establish a connection:
> > > > > > > > again not a great protocol, but very well known and understood.
> > > > > > > 
> > > > > > > I don't think we have, in general, a good understanding of these
> > > > > > > "state" based protocols ...
> > > > > > 
> > > > > > What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
> > > > > > kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!
> > > > > 
> > > > > And no one I know is able to describe, accurately, exactly what the
> > > > > state diagram for even one of those actually looks like or indeed should
> > > > > look like. It became quite evident in these threads about hotplug script
> > > > > handling etc that no one really knows for sure what (is supposed to)
> > > > > happens when.
> > > > 
> > > > I thought that most of the thread was about the interface with the block
> > > > scripts, that is an entirely different matter and completely obscure.
> > > > If I am mistaken, please point me at the right email.
> > > 
> > > We are talking about reusing the existing xenbus state machine schema
> > > for a new purpose. Ian J pointed out that these are not generally well
> > > understood, you replied that it was and cited some examples. I pointed
> > > out why these were not examples of why this stuff was well understood at
> > > all, in fact quite the opposite.
> > 
> > Sorry but I don't understand how these examples are supposed to be
> > "quite the opposite".
> > I quite like the idea of being able to read a single source file of less
> > than 400 LOC to understand how a protocol works
> > (drivers/input/misc/xen-kbdfront.c).
> 
> That is not a protocol specification, merely one implementation of it.
> What does the BSD driver do? Is it exactly the same as Linux? Should BSD
> driver authors be expected to reverse engineer the protocol from the
> Linux code? What/who arbitrates when the two behave differently?

The lack of documentation is an issue.


> > In fact I don't think that understanding the protocol has been an issue
> > for the GSoC student that had to write a new one.
> 
> Being able to reverse engineer something which works is not proof that
> these things are "well understood" in the general case.
> 
> > I think we are under influence of a "reiventing the wheel" virus.
> 
> I think we are in danger of making the same mistakes again as have been
> made with the device protocols and this is what I want to avoid.
> 
> Now, perhaps this style of state machine protocol is a reasonable design
> choice in this case, but since we are starting afresh here this specific
> new instance should be well documented _up_front_ not left in the "oh,
> just read the Linux code" state we have now for many of our devices
> which has lead to multiple slightly divergent implementations of the
> same basic concept.

I agree.


> > > > > Justin just posted a good description for blkif.h which included a state
> > > > > machine description. We need the same for pciif.h, netif.h etc etc.
> > > >  
> > > > The state machine is the same for block and network.
> > > 
> > > No, it's not. This is exactly what IanJ and I are talking about.
> > 
> > Could you please elaborate?
> > 
> > I am sure you know that the xenstore state machine is handled the same
> > way for all the backends in QEMU (see hw/xen_backend.c).
> > And the same thing is true for the frontends and the backends in Linux.
> 
> A substantial proportion of the threads about this hotplug script stuff
> has been about the fact that no one is quite sure what really happens
> when for all implementations nor what the common semantics are.
> 
> e.g. How do you ask a backend to shut down (do you set it to state 5?
> state 6? do you nuke the xenstore dir?). Neither is anyone sure when the
> correct point to call the hotplug scripts actually is, or even what
> actually happens with them right now across the different backend
> drivers or kernel types.

Yeah, this needs to be documented in advance.


> The actual state transitions which netback and blkback go through are
> not the same: The netback protocol uses InitWait, the blkback one does
> not or is it vice-versa? I can't remember and it isn't documented. Some
> Linux frontends handled the kexec reconnect sequencing differently, by
> disconnecting or reconnecting the actual underlying devices at subtly
> different times and/or handling the transition from Closing back to Init
> or InitWait differently.

Interesting... however these changes are just about when netfront or
blkfront decide to bring up the interface/device, they don't affect the
protocol itself, I think. What I mean is that from the toolstack POV we
don't need to worry about this.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Roger Pau Monné]

2012/2/9 Ian Campbell <Ian.Campbell@citrix.com>:
> On Thu, 2012-02-09 at 16:18 +0000, Stefano Stabellini wrote:
>> On Thu, 9 Feb 2012, Ian Campbell wrote:
>> > On Thu, 2012-02-09 at 16:00 +0000, Stefano Stabellini wrote:
>> > > On Thu, 9 Feb 2012, Ian Campbell wrote:
>> > > > On Thu, 2012-02-09 at 15:32 +0000, Stefano Stabellini wrote:
>> > > > > On Thu, 9 Feb 2012, Ian Jackson wrote:
>> > > > > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
>> > > > > > > - we can reuse the "state" based mechanism to establish a connection:
>> > > > > > > again not a great protocol, but very well known and understood.
>> > > > > >
>> > > > > > I don't think we have, in general, a good understanding of these
>> > > > > > "state" based protocols ...
>> > > > >
>> > > > > What?! We have netback, netfront, blkback, blkfront, pciback, pcifront,
>> > > > > kbdfront, fbfront, xenconsole, and these are only the ones in Linux!!
>> > > >
>> > > > And no one I know is able to describe, accurately, exactly what the
>> > > > state diagram for even one of those actually looks like or indeed should
>> > > > look like. It became quite evident in these threads about hotplug script
>> > > > handling etc that no one really knows for sure what (is supposed to)
>> > > > happens when.
>> > >
>> > > I thought that most of the thread was about the interface with the block
>> > > scripts, that is an entirely different matter and completely obscure.
>> > > If I am mistaken, please point me at the right email.
>> >
>> > We are talking about reusing the existing xenbus state machine schema
>> > for a new purpose. Ian J pointed out that these are not generally well
>> > understood, you replied that it was and cited some examples. I pointed
>> > out why these were not examples of why this stuff was well understood at
>> > all, in fact quite the opposite.
>>
>> Sorry but I don't understand how these examples are supposed to be
>> "quite the opposite".
>> I quite like the idea of being able to read a single source file of less
>> than 400 LOC to understand how a protocol works
>> (drivers/input/misc/xen-kbdfront.c).
>
> That is not a protocol specification, merely one implementation of it.
> What does the BSD driver do? Is it exactly the same as Linux? Should BSD
> driver authors be expected to reverse engineer the protocol from the
> Linux code? What/who arbitrates when the two behave differently?
>
>> In fact I don't think that understanding the protocol has been an issue
>> for the GSoC student that had to write a new one.
>
> Being able to reverse engineer something which works is not proof that
> these things are "well understood" in the general case.
>
>> I think we are under influence of a "reiventing the wheel" virus.
>
> I think we are in danger of making the same mistakes again as have been
> made with the device protocols and this is what I want to avoid.
>
> Now, perhaps this style of state machine protocol is a reasonable design
> choice in this case, but since we are starting afresh here this specific
> new instance should be well documented _up_front_ not left in the "oh,
> just read the Linux code" state we have now for many of our devices
> which has lead to multiple slightly divergent implementations of the
> same basic concept.

Yes, documentation about this protocol should go in together with the
protocol itself.

>
>> > > > Justin just posted a good description for blkif.h which included a state
>> > > > machine description. We need the same for pciif.h, netif.h etc etc.
>> > >
>> > > The state machine is the same for block and network.
>> >
>> > No, it's not. This is exactly what IanJ and I are talking about.
>>
>> Could you please elaborate?
>>
>> I am sure you know that the xenstore state machine is handled the same
>> way for all the backends in QEMU (see hw/xen_backend.c).
>> And the same thing is true for the frontends and the backends in Linux.
>
> A substantial proportion of the threads about this hotplug script stuff
> has been about the fact that no one is quite sure what really happens
> when for all implementations nor what the common semantics are.
>
> e.g. How do you ask a backend to shut down (do you set it to state 5?
> state 6? do you nuke the xenstore dir?). Neither is anyone sure when the
> correct point to call the hotplug scripts actually is, or even what
> actually happens with them right now across the different backend
> drivers or kernel types.


This is true, BSD, Linux and Qemu have slightly different
implementations of the backend protocol at least, BSD and qemu-xen
doesn't react when setting backend "state" to 5 or "online" to 0. This
gave me some headaches that could be solved if this was properly
documented/implemented.

> The actual state transitions which netback and blkback go through are
> not the same: The netback protocol uses InitWait, the blkback one does
> not or is it vice-versa? I can't remember and it isn't documented. Some
> Linux frontends handled the kexec reconnect sequencing differently, by
> disconnecting or reconnecting the actual underlying devices at subtly
> different times and/or handling the transition from Closing back to Init
> or InitWait differently.

On this implementation I wait for backend to switch to state 2
(XenbusStateInitWait) before executing hotplug scripts for both vifs
and vbds, and it seems like they work ok on both Linux and NetBSD, and
again, since it's not documented anywhere, I just guess it's the
correct way to do it, but I can't be sure.

> And this is just for Linux talking to Linux.
>
> I know for sure that the Windows frontends follow a different state
> transition path to Linux (and that it has interacted badly with the
> kexec differences in the Linux backends discussed above). I bet BSD has
> some subtle differences in behaviour too.
>
> The fact is that none of our device state machine protocols are not well
> documented (although blkif.h is about to be). If this stuff were well
> understood we would already have such documentation because it would be
> trivial to write -- but it is not. If you disagree then please document
> the netif state machine protocol in the form of a patch to netif.h.
>
> Ian.
>

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Roger Pau Monné]

2012/2/8 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
>> libxl: introduce libxl hotplug public API functions
>>
>> These functions mimic the name used by the local device functions,
>> following the nomenclature:
>
> I think these should be private functions.  Your new device daemon is,
> I think, part of the implementation of libxl, not something that
> another toolstack on top of libxl will reasonably want to replace.
>
> This is a new departure for libxl, which doesn't currently have any
> internal executables.

Not sure about the best approach here, I though that xldeviced will be
like xl, but that's something we can discuss about. As you can see,
the code in xldeviced is minimal, and I'm not sure if we shouldn't
allow people to make use of this new interface to develop a custom
xldeviced. It will provide much more flexibility than hotplug scripts.

>
>> The xenstore structure used by vbd disk entries:
>>
>> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>
>> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/pdev_path
>> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/vdev
>> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/script
>> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/removable
>> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/readwrite
>> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/is_cdrom
>> /hotplug/<backend_domid>/<frontend_domid>/vbd/<devid>/state
>>
>> and vif devices use the following:
>>
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mtu
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/model
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mac
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ip
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/bridge
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ifname
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/script
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/nictype
>>
>> This will allow us to pass the libxl_device_disk and libxl_device_nic
>> struct from Dom0 to driver domain, and execute the necessary backends
>> there.
>
> Is this really the best encoding of this information in xenstore ?

I've just cloned the current protocol used for the backend/frontend
devices, I have to say I don't dislike it (apart from the fact that it
is not documented), it's easy to view what we are doing, and to debug
it.

> If we're allowed to assume that the backendd is a libxl program too,
> and of a reasonably similar version, perhaps we should have some kind
> of idl-generated mapping from xenstore keys to struct members ?

Will look into it, currently neither libxl_device_disk or
libxl_device_nic don't have mappings to it's related xenstore
frontend/backend entries, it might be good to add those too first.

> Ian.

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

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Campbell]

On Tue, 2012-02-14 at 14:23 +0000, Roger Pau Monné wrote:
> 2012/2/8 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> > Roger Pau Monne writes ("[Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> >> libxl: introduce libxl hotplug public API functions
> >>
> >> These functions mimic the name used by the local device functions,
> >> following the nomenclature:
> >
> > I think these should be private functions.  Your new device daemon is,
> > I think, part of the implementation of libxl, not something that
> > another toolstack on top of libxl will reasonably want to replace.
> >
> > This is a new departure for libxl, which doesn't currently have any
> > internal executables.
> 
> Not sure about the best approach here, I though that xldeviced will be
> like xl, but that's something we can discuss about.

I was wondering if perhaps xl/libxl should handle these events by
default (using the same APIs as xldeviced uses) such that in the simple
case users can continue to just use xl without worrying about
configuring or enabling additional daemons. There would then be an
option to disable this if you were running xldeviced (either in dom0 or
in a driver dom). Perhaps a more DWIM extension would be to default to
running things within xl/libxl only if the backend is in dom0 and assu,e
xldeviced is running in backend doms != 0.

Do you integrate with the libxl event loop? If you did wouldn't this be
trivial to handle in xl and wouldn't the xldeviced become the trivial
setup + run the event loop program?

Ian.



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

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> I was wondering if perhaps xl/libxl should handle these events by
> default (using the same APIs as xldeviced uses) such that in the simple
> case users can continue to just use xl without worrying about
> configuring or enabling additional daemons. There would then be an
> option to disable this if you were running xldeviced (either in dom0 or
> in a driver dom). Perhaps a more DWIM extension would be to default to
> running things within xl/libxl only if the backend is in dom0 and assu,e
> xldeviced is running in backend doms != 0.

This is a good idea.  It can probably be implemented with some kind of
loopback function which either runs the script, or communicates with
the xldeviced via xenstore.

> Do you integrate with the libxl event loop? If you did wouldn't this be
> trivial to handle in xl and wouldn't the xldeviced become the trivial
> setup + run the event loop program?

That would be one way to look at it.

  void libxl_xldeviced_enter(libxl_ctx*); /* never returns */

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Jackson]

Ian Jackson writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > Do you integrate with the libxl event loop? If you did wouldn't this be
> > trivial to handle in xl and wouldn't the xldeviced become the trivial
> > setup + run the event loop program?
> 
> That would be one way to look at it.
> 
>   void libxl_xldeviced_enter(libxl_ctx*); /* never returns */

Actually, thinking about it, the interface is probably:

   int libxl_devicedaemon_start(libxl_ctx*);
     /* Causes this libxl ctx to behave like the xl device daemon
      * whenever it is processing libxl events. */

and then the caller can do this

   rc = libxl_devicedaemon_start(ctx);
   if (rc) ...

   for (;;) {
       ... libxl_event_wait ...   
   }

In principle it would be possible for this device daemon to be part of
some other daemon this way.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Campbell]

On Mon, 2012-02-20 at 18:55 +0000, Ian Jackson wrote:
> Ian Jackson writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > Ian Campbell writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > > Do you integrate with the libxl event loop? If you did wouldn't this be
> > > trivial to handle in xl and wouldn't the xldeviced become the trivial
> > > setup + run the event loop program?
> > 
> > That would be one way to look at it.
> > 
> >   void libxl_xldeviced_enter(libxl_ctx*); /* never returns */
> 
> Actually, thinking about it, the interface is probably:
> 
>    int libxl_devicedaemon_start(libxl_ctx*);
>      /* Causes this libxl ctx to behave like the xl device daemon
>       * whenever it is processing libxl events. */
> 
> and then the caller can do this
> 
>    rc = libxl_devicedaemon_start(ctx);
>    if (rc) ...
> 
>    for (;;) {
>        ... libxl_event_wait ...   
>    }

This is the sort of thing I was imagining too.

> In principle it would be possible for this device daemon to be part of
> some other daemon this way.

Yes.

Ian

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

On Tue, 14 Feb 2012, Ian Campbell wrote:
> On Tue, 2012-02-14 at 14:23 +0000, Roger Pau Monné wrote:
> > 2012/2/8 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> > > Roger Pau Monne writes ("[Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > >> libxl: introduce libxl hotplug public API functions
> > >>
> > >> These functions mimic the name used by the local device functions,
> > >> following the nomenclature:
> > >
> > > I think these should be private functions.  Your new device daemon is,
> > > I think, part of the implementation of libxl, not something that
> > > another toolstack on top of libxl will reasonably want to replace.
> > >
> > > This is a new departure for libxl, which doesn't currently have any
> > > internal executables.
> > 
> > Not sure about the best approach here, I though that xldeviced will be
> > like xl, but that's something we can discuss about.
> 
> I was wondering if perhaps xl/libxl should handle these events by
> default (using the same APIs as xldeviced uses) such that in the simple
> case users can continue to just use xl without worrying about
> configuring or enabling additional daemons. There would then be an
> option to disable this if you were running xldeviced (either in dom0 or
> in a driver dom). Perhaps a more DWIM extension would be to default to
> running things within xl/libxl only if the backend is in dom0 and assu,e
> xldeviced is running in backend doms != 0.

While I understand that technically is not difficult to achieve and
doesn't add much complexity in terms of lines of code; starting daemons
is cheap and is taken care by xencommons. I am not sure if it is worth
introducing yet another way of doing the same thing.

[-- Attachment #2: Type: text/plain, Size: 132 bytes --]

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

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> On Tue, 14 Feb 2012, Ian Campbell wrote:
> > I was wondering if perhaps xl/libxl should handle these events by
> > default (using the same APIs as xldeviced uses) such that in the simple
> > case users can continue to just use xl without worrying about
> > configuring or enabling additional daemons. There would then be an
> > option to disable this if you were running xldeviced (either in dom0 or
> > in a driver dom). Perhaps a more DWIM extension would be to default to
> > running things within xl/libxl only if the backend is in dom0 and assu,e
> > xldeviced is running in backend doms != 0.
> 
> While I understand that technically is not difficult to achieve and
> doesn't add much complexity in terms of lines of code; starting daemons
> is cheap and is taken care by xencommons. I am not sure if it is worth
> introducing yet another way of doing the same thing.

This is a good point.  Also, if anyone does object to this daemon, and
we take the event-driven approach already suggested, it probably won't
be too hard to add later the short-cut to run it in-process.

So I think for now we should just have the daemonic version.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Campbell]

On Tue, 2012-02-21 at 16:42 +0000, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> > On Tue, 14 Feb 2012, Ian Campbell wrote:
> > > I was wondering if perhaps xl/libxl should handle these events by
> > > default (using the same APIs as xldeviced uses) such that in the simple
> > > case users can continue to just use xl without worrying about
> > > configuring or enabling additional daemons. There would then be an
> > > option to disable this if you were running xldeviced (either in dom0 or
> > > in a driver dom). Perhaps a more DWIM extension would be to default to
> > > running things within xl/libxl only if the backend is in dom0 and assu,e
> > > xldeviced is running in backend doms != 0.
> > 
> > While I understand that technically is not difficult to achieve and
> > doesn't add much complexity in terms of lines of code; starting daemons
> > is cheap and is taken care by xencommons. I am not sure if it is worth
> > introducing yet another way of doing the same thing.
> 
> This is a good point.  Also, if anyone does object to this daemon, and
> we take the event-driven approach already suggested, it probably won't
> be too hard to add later the short-cut to run it in-process.
> 
> So I think for now we should just have the daemonic version.

If the smarts are all in libxl and the daemon is the noddy event loop
then that's ok with me too.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Ian Jackson]

Roger Pau Monne writes ("[Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mtu
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/model
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mac
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ip
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/bridge
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ifname
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/script
> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/nictype
> 
> This will allow us to pass the libxl_device_disk and libxl_device_nic
> struct from Dom0 to driver domain, and execute the necessary backends
> there.

Also I think as you're inventing a new protocol we should have a
protocol document which, unless I've missed it, doesn't appear in your
series.

Ian.

Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions

[Roger Pau Monné]

2012/2/8 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions"):
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mtu
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/model
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/mac
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ip
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/bridge
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/ifname
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/script
>> /hotplug/<backend_domid>/<frontend_domid>/vif/<devid>/nictype
>>
>> This will allow us to pass the libxl_device_disk and libxl_device_nic
>> struct from Dom0 to driver domain, and execute the necessary backends
>> there.
>
> Also I think as you're inventing a new protocol we should have a
> protocol document which, unless I've missed it, doesn't appear in your
> series.

No, you are right, it's not documented anywhere, were is the best
place to put this? libxl.h?

> Ian.

[PATCH 21 of 29 RFC] libxl: add libxl__parse_hotplug_path

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328178994 -3600
# Node ID 346d86654b1dd81b5aba821bf9732844bb35c7a3
# Parent  bc38aa2e11e21b34dbc29a81f069133a3276848a
libxl: add libxl__parse_hotplug_path

Generate libxl__device from hotplug path.

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

diff -r bc38aa2e11e2 -r 346d86654b1d tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 11:33:27 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 11:36:34 2012 +0100
@@ -66,6 +66,24 @@ int libxl__parse_backend_path(libxl__gc 
     return libxl__device_kind_from_string(strkind, &dev->backend_kind);
 }
 
+int libxl__parse_hotplug_path(libxl__gc *gc,
+                              const char *path,
+                              libxl__device *dev)
+{
+    /* /hotplug/<backend_domid>/<frontend_domid>/<kind> */
+    char strkind[16]; /* Longest is actually "console" */
+    int rc = sscanf(path, "/hotplug/%u/%u/%15[^/]/%d",
+                    &dev->backend_domid,
+                    &dev->domid,
+                    strkind,
+                    &dev->devid);
+
+    if (rc != 4)
+        return ERROR_FAIL;
+
+    return libxl__device_kind_from_string(strkind, &dev->backend_kind);
+}
+
 int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
                              char **bents, char **fents)
 {
diff -r bc38aa2e11e2 -r 346d86654b1d tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 11:33:27 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 11:36:34 2012 +0100
@@ -307,6 +307,9 @@ typedef struct {
 _hidden char *libxl__device_hotplug_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
                                       libxl__device *dev);
+_hidden int libxl__parse_hotplug_path(libxl__gc *gc,
+                                      const char *path,
+                                      libxl__device *dev);
 _hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid);

[PATCH 22 of 29 RFC] libxl: add libxl__parse_disk_hotplug_path

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328179164 -3600
# Node ID 478459fa6bfce280bf31e1d971c5f2b39eee6986
# Parent  346d86654b1dd81b5aba821bf9732844bb35c7a3
libxl: add libxl__parse_disk_hotplug_path

Create a libxl_device_disk from hotplug xenstore information.

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

diff -r 346d86654b1d -r 478459fa6bfc tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 11:36:34 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 11:39:24 2012 +0100
@@ -84,6 +84,45 @@ int libxl__parse_hotplug_path(libxl__gc 
     return libxl__device_kind_from_string(strkind, &dev->backend_kind);
 }
 
+int libxl__parse_disk_hotplug_path(libxl__gc *gc,
+                                   const char *path,
+                                   libxl_device_disk *disk)
+{
+    char *value;
+
+    disk->pdev_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s",
+                                                                  path, 
+                                                                  "pdev_path"));
+
+    disk->vdev = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                                 "vdev"));
+
+    disk->script = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s",
+                                                                   path,
+                                                                   "script"));
+    value = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                            "removable"));
+    if (value)
+        disk->removable = atoi(value);
+
+    value = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                            "readwrite"));
+    if (value)
+        disk->readwrite = atoi(value);
+
+    value = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                            "is_cdrom"));
+    if (value)
+        disk->is_cdrom = atoi(value);
+
+    value = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                            "format"));
+    if (value)
+        disk->format = atoi(value);
+
+    return 0;
+}
+
 int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
                              char **bents, char **fents)
 {
diff -r 346d86654b1d -r 478459fa6bfc tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 11:36:34 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 11:39:24 2012 +0100
@@ -310,6 +310,9 @@ typedef struct {
 _hidden int libxl__parse_hotplug_path(libxl__gc *gc,
                                       const char *path,
                                       libxl__device *dev);
+_hidden int libxl__parse_disk_hotplug_path(libxl__gc *gc,
+                                           const char *path,
+                                           libxl_device_disk *disk);
 _hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid);

[PATCH 23 of 29 RFC] libxl: add libxl__parse_nic_hotplug_path

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328179333 -3600
# Node ID 2708343292be6facb6a2d8aaf9a9d95afa61b7f6
# Parent  478459fa6bfce280bf31e1d971c5f2b39eee6986
libxl: add libxl__parse_nic_hotplug_path

Create a libxl_device_nic from hotplug xenstore information.

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

diff -r 478459fa6bfc -r 2708343292be tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 11:39:24 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 11:42:13 2012 +0100
@@ -123,6 +123,56 @@ int libxl__parse_disk_hotplug_path(libxl
     return 0;
 }
 
+int libxl__parse_nic_hotplug_path(libxl__gc *gc,
+                                  const char *path,
+                                  libxl_device_nic *nic)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *value;
+
+    if (sscanf(path, "/hotplug/%*u/%*u/vif/%d", &nic->devid) != 1) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to get nic device id from "
+                                          "%s", path);
+        return -1;
+    }
+
+    value = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                            "mtu"));
+    if (value)
+        nic->mtu = atoi(value);
+
+    nic->model = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                                 "model"));
+
+    value = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                            "mac"));
+    if (value) {
+        if (sscanf(value, LIBXL_MAC_FMT, LIBXL_MAC_BYTES(&nic->mac)) != 6) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to parse mac %s", value);
+            return -1;
+        }
+    }
+
+    nic->ip = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                              "ip"));
+
+    nic->bridge = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                                  "bridge"));
+    nic->ifname = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                                  "ifname"));
+
+    nic->script = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                                  "script"));
+
+    value = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", path,
+                                                            "nictype"));
+    if (value)
+        nic->nictype = atoi(value);
+
+    return 0;
+}
+
+
 int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
                              char **bents, char **fents)
 {
diff -r 478459fa6bfc -r 2708343292be tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 11:39:24 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 11:42:13 2012 +0100
@@ -313,6 +313,9 @@ typedef struct {
 _hidden int libxl__parse_disk_hotplug_path(libxl__gc *gc,
                                            const char *path,
                                            libxl_device_disk *disk);
+_hidden int libxl__parse_nic_hotplug_path(libxl__gc *gc,
+                                          const char *path,
+                                          libxl_device_nic *nic);
 _hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid);

Re: [PATCH 23 of 29 RFC] libxl: add libxl__parse_nic_hotplug_path

[Ian Jackson]

Roger Pau Monne writes ("[Xen-devel] [PATCH 23 of 29 RFC] libxl: add libxl__parse_nic_hotplug_path"):
> libxl: add libxl__parse_nic_hotplug_path

This series has a lot of "add <some function>" but without adding any
callers.

Can I suggest it be restructured to try to avoid introducing too many
functions without callers unless it's necessary to stop other patches
becoming too large.

Ian.

[PATCH 24 of 29 RFC] libxl: add libxl_setup_hotplug_listener

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328179503 -3600
# Node ID 1506b1f2ab0fe5f4cd5bcd86a566d5a7be5f838b
# Parent  2708343292be6facb6a2d8aaf9a9d95afa61b7f6
libxl: add libxl_setup_hotplug_listener

Setup a xenstore watch on /hotplug/<domid> to react to hotplug
changes.

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

diff -r 2708343292be -r 1506b1f2ab0f tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 11:42:13 2012 +0100
+++ b/tools/libxl/libxl.c	Thu Feb 02 11:45:03 2012 +0100
@@ -949,6 +949,40 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 }
 
 /******************************************************************************/
+/*
+ * Hotplug functions
+ *
+ */
+
+int libxl_setup_hotplug_listener(libxl_ctx *ctx)
+{
+    GC_INIT(ctx);
+    char *hotplug_path, *id;
+    int rc;
+
+    id = libxl__xs_read(gc, XBT_NULL, "domid");
+    if (!id) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to get domid");
+        rc = -1;
+        goto out;
+    }
+    hotplug_path = libxl__sprintf(gc, "/hotplug/%s", id);
+
+    xs_mkdir(ctx->xsh, XBT_NULL, hotplug_path);
+
+    if (!xs_watch(ctx->xsh, hotplug_path, hotplug_path)) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to watch /hotplug/%s", id);
+        rc = -1;
+        goto out;
+    }
+
+    rc = 0;
+out:
+    GC_FREE;
+    return rc;
+}
+
+/******************************************************************************/
 
 int libxl_device_disk_init(libxl_ctx *ctx, libxl_device_disk *disk)
 {
diff -r 2708343292be -r 1506b1f2ab0f tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Thu Feb 02 11:42:13 2012 +0100
+++ b/tools/libxl/libxl.h	Thu Feb 02 11:45:03 2012 +0100
@@ -456,6 +456,14 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  *
  *   Requests de destruction of the given device and waits for the result.
  *
+ * Hotplug
+ * -------
+ *
+ * libxl_setup_hotplug_listener(ctx):
+ *
+ *   Set the necessary watches to start monitoring /hotplug/ entries for
+ *   the caller domain.
+ *
  */
 
 /* Disks */
@@ -523,6 +531,9 @@ int libxl_device_pci_remove(libxl_ctx *c
 int libxl_device_pci_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
 libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid, int *num);
 
+/* Hotplug */
+int libxl_setup_hotplug_listener(libxl_ctx *ctx);
+
 /*
  * Parse a PCI BDF into a PCI device structure.
  */

[PATCH 25 of 29 RFC] libxl: add libxl_hotplug_dispatch

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328179686 -3600
# Node ID 94532199f647fc03816fc5ae50ab53c8c5b80cd8
# Parent  1506b1f2ab0fe5f4cd5bcd86a566d5a7be5f838b
libxl: add libxl_hotplug_dispatch

Added a new function to libxl API to handle device hotplug. Actions to
execute upon hotplug device state changes are handled using the
libxl_device_disk_hotplug_actions and libxl_device_nic_hotplug_actions
depending on the type of device. Currently only VIF and VBD devices
are handled by the hotplug mechanism.

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

diff -r 1506b1f2ab0f -r 94532199f647 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 11:45:03 2012 +0100
+++ b/tools/libxl/libxl.c	Thu Feb 02 11:48:06 2012 +0100
@@ -982,6 +982,188 @@ out:
     return rc;
 }
 
+static int libxl__hotplug_generic_dispatcher(libxl__gc *gc, char *path,
+                                uint32_t domid,
+                                libxl__hotplug_status state,
+                                libxl__device_generic_hotplug_actions *action,
+                                void *device)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *spath = libxl__sprintf(gc, "%s/state", path);
+    int rc = 0;
+
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "processing device %s with state %d",
+                                      path, state);
+    switch(state) {
+    case HOTPLUG_DEVICE_INIT:
+        if (action->init) {
+            rc = action->init(ctx, domid, device);
+            if (rc < 0) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to init"
+                                                  " device %s", path);
+                libxl__xs_write(gc, XBT_NULL, spath, "%d",
+                                HOTPLUG_DEVICE_ERROR);
+                break;
+            }
+        }
+        libxl__xs_write(gc, XBT_NULL, spath, "%d", HOTPLUG_DEVICE_CONNECT);
+        break;
+    case HOTPLUG_DEVICE_CONNECT:
+        if (action->connect) {
+            rc = action->connect(ctx, domid, device);
+            if (rc < 0) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to connect"
+                                                  " device %s", path);
+                libxl__xs_write(gc, XBT_NULL, spath, "%d",
+                                HOTPLUG_DEVICE_ERROR);
+                break;
+            }
+        }
+        libxl__xs_write(gc, XBT_NULL, spath, "%d", HOTPLUG_DEVICE_CONNECTED);
+        break;
+    case HOTPLUG_DEVICE_CONNECTED:
+        if (action->connected) {
+            rc = action->connected(ctx, domid, device);
+            if (rc < 0) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute "
+                                                  "connected action on"
+                                                  " device %s", path);
+                libxl__xs_write(gc, XBT_NULL, spath, "%d",
+                                HOTPLUG_DEVICE_ERROR);
+                break;
+            }
+        }
+        break;
+    case HOTPLUG_DEVICE_DISCONNECT:
+        if (action->disconnect) {
+            rc = action->disconnect(ctx, domid, device);
+            if (rc < 0) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to unplug"
+                                                  " device %s", path);
+                libxl__xs_write(gc, XBT_NULL, spath, "%d",
+                                HOTPLUG_DEVICE_ERROR);
+                break;
+            }
+        }
+        libxl__xs_write(gc, XBT_NULL, spath, "%d",
+                        HOTPLUG_DEVICE_DISCONNECTED);
+        break;
+    case HOTPLUG_DEVICE_DISCONNECTED:
+        if (action->disconnected) {
+            rc = action->disconnected(ctx, domid, device);
+            if (rc < 0) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute "
+                                                  "unpluged action on"
+                                                  " device %s", path);
+                libxl__xs_write(gc, XBT_NULL, spath, "%d",
+                                HOTPLUG_DEVICE_ERROR);
+                break;
+            }
+        }
+        break;
+    case HOTPLUG_DEVICE_FORCE_DISCONNECT:
+        if (action->force_disconnect) {
+            rc = action->force_disconnect(ctx, domid, device);
+            if (rc < 0) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to force "
+                                                  "disconnect device "
+                                                  "%s", path);
+                libxl__xs_write(gc, XBT_NULL, spath, "%d",
+                                HOTPLUG_DEVICE_ERROR);
+                break;
+            }
+        }
+        libxl__xs_write(gc, XBT_NULL, spath, "%d",
+                        HOTPLUG_DEVICE_DISCONNECTED);
+        break;
+    case HOTPLUG_DEVICE_ERROR:
+        if (action->error)
+            rc = action->error(ctx, domid, device);
+        break;
+    }
+    return rc;
+}
+
+int libxl_hotplug_dispatch(libxl_ctx *ctx,
+                           libxl_device_disk_hotplug_actions *disk_handler,
+                           libxl_device_nic_hotplug_actions *nic_handler)
+{
+    unsigned int n;
+    char **l1, *sstate, *entry, *spath;
+    libxl__device dev;
+    libxl_device_disk disk;
+    libxl_device_nic nic;
+    libxl__hotplug_status state;
+
+    for (;;) {
+        GC_INIT(ctx);
+        l1 = xs_read_watch(ctx->xsh, &n);
+        if (!l1) {
+            continue;
+        }
+
+        /* Only react to "state" changes */
+        entry = strrchr(l1[0], '/');
+        if (!entry || strcmp(entry, "/state"))
+            goto next;
+
+        spath = libxl__strdup(gc, l1[0]);
+        /* Fetch state info */
+        sstate = libxl__xs_read(gc, XBT_NULL, l1[0]);
+        if (!sstate) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to parse state"
+                                              "from %s", l1[0]);
+            goto next;
+        }
+        state = atoi(sstate);
+
+        /* remove "state" part from path */
+        *entry = '\0';
+        if (libxl__parse_hotplug_path(gc, l1[0], &dev) < 0)
+            goto next;
+
+        switch(dev.backend_kind) {
+        case LIBXL__DEVICE_KIND_VBD:
+            libxl_device_disk_init(ctx, &disk);
+            if (libxl__parse_disk_hotplug_path(gc, l1[0], &disk)) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to parse disk data"
+                                                  "from %s", l1[0]);
+                goto next;
+            }
+            /* Choose suitable backend */
+            disk.backend = LIBXL_DISK_BACKEND_UNKNOWN;
+            if (libxl__device_disk_set_backend(gc, &disk) < 0) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "no suitable backend for"
+                                                  "disk %s", disk.pdev_path);
+                goto next;
+            }
+            libxl__hotplug_generic_dispatcher(gc, l1[0], dev.domid, state,
+                    (libxl__device_generic_hotplug_actions *) disk_handler,
+                    (void *) &disk);
+            break;
+        case LIBXL__DEVICE_KIND_VIF:
+            libxl_device_nic_init(ctx, &nic);
+            if (libxl__parse_nic_hotplug_path(gc, l1[0], &nic)) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to parse nic data"
+                                                  "from %s", l1[0]);
+                goto next;
+            }
+            libxl__hotplug_generic_dispatcher(gc, l1[0], dev.domid, state,
+                    (libxl__device_generic_hotplug_actions *) nic_handler,
+                    (void *) &nic);
+            break;
+        default:
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "unable to handle device type %s from %s",
+                       libxl__device_kind_to_string(dev.backend_kind), l1[0]);
+            break;
+        }
+next:
+        free(l1);
+        GC_FREE;
+    }
+}
+
 /******************************************************************************/
 
 int libxl_device_disk_init(libxl_ctx *ctx, libxl_device_disk *disk)
diff -r 1506b1f2ab0f -r 94532199f647 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Thu Feb 02 11:45:03 2012 +0100
+++ b/tools/libxl/libxl.h	Thu Feb 02 11:48:06 2012 +0100
@@ -464,6 +464,12 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  *   Set the necessary watches to start monitoring /hotplug/ entries for
  *   the caller domain.
  *
+ * libxl_hotplug_dispatch(ctx, disk_handler, nic_handler):
+ *
+ *   React to /hotplug/<domid> changes and execute the necessary handlers
+ *   based on the passed disk_handler and nic_handler structs that contain
+ *   the pointers to the functions to be executed.
+ *
  */
 
 /* Disks */
@@ -532,7 +538,37 @@ int libxl_device_pci_destroy(libxl_ctx *
 libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid, int *num);
 
 /* Hotplug */
+typedef int libxl_device_disk_hotplug_handler(libxl_ctx *ctx,
+                                              uint32_t domid,
+                                              libxl_device_disk *disk);
+typedef int libxl_device_nic_hotplug_handler(libxl_ctx *ctx,
+                                             uint32_t domid,
+                                             libxl_device_nic *nic);
+
+typedef struct libxl_device_disk_hotplug_actions {
+    libxl_device_disk_hotplug_handler *init;
+    libxl_device_disk_hotplug_handler *connect;
+    libxl_device_disk_hotplug_handler *connected;
+    libxl_device_disk_hotplug_handler *disconnect;
+    libxl_device_disk_hotplug_handler *disconnected;
+    libxl_device_disk_hotplug_handler *force_disconnect;
+    libxl_device_disk_hotplug_handler *error;
+} libxl_device_disk_hotplug_actions;
+
+typedef struct libxl_device_nic_hotplug_actions {
+    libxl_device_nic_hotplug_handler *init;
+    libxl_device_nic_hotplug_handler *connect;
+    libxl_device_nic_hotplug_handler *connected;
+    libxl_device_nic_hotplug_handler *disconnect;
+    libxl_device_nic_hotplug_handler *disconnected;
+    libxl_device_nic_hotplug_handler *force_disconnect;
+    libxl_device_nic_hotplug_handler *error;
+} libxl_device_nic_hotplug_actions;
+
 int libxl_setup_hotplug_listener(libxl_ctx *ctx);
+int libxl_hotplug_dispatch(libxl_ctx *ctx,
+                           libxl_device_disk_hotplug_actions *disk_handler,
+                           libxl_device_nic_hotplug_actions *nic_handler);
 
 /*
  * Parse a PCI BDF into a PCI device structure.
diff -r 1506b1f2ab0f -r 94532199f647 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 11:45:03 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 11:48:06 2012 +0100
@@ -384,6 +384,22 @@ typedef enum {
 _hidden int libxl__device_hotplug_disconnect(libxl__gc *gc, libxl__device *dev,
                                        libxl__hotplug_status disconnect_param);
 
+/* Generic handlers for hotplug devices */
+/* Hotplug */
+typedef int libxl__device_generic_hotplug_handler(libxl_ctx *ctx,
+                                                 uint32_t domid,
+                                                 void *device);
+
+typedef struct libxl__device_generic_hotplug_actions {
+    libxl__device_generic_hotplug_handler *init;
+    libxl__device_generic_hotplug_handler *connect;
+    libxl__device_generic_hotplug_handler *connected;
+    libxl__device_generic_hotplug_handler *disconnect;
+    libxl__device_generic_hotplug_handler *disconnected;
+    libxl__device_generic_hotplug_handler *force_disconnect;
+    libxl__device_generic_hotplug_handler *error;
+} libxl__device_generic_hotplug_actions;
+
 /*
  * libxl__device_hotplug - generic function to execute hotplug scripts
  * gc: allocation pool

Re: [PATCH 25 of 29 RFC] libxl: add libxl_hotplug_dispatch

[Stefano Stabellini]

On Thu, 2 Feb 2012, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1328179686 -3600
> # Node ID 94532199f647fc03816fc5ae50ab53c8c5b80cd8
> # Parent  1506b1f2ab0fe5f4cd5bcd86a566d5a7be5f838b
> libxl: add libxl_hotplug_dispatch
> 
> Added a new function to libxl API to handle device hotplug. Actions to
> execute upon hotplug device state changes are handled using the
> libxl_device_disk_hotplug_actions and libxl_device_nic_hotplug_actions
> depending on the type of device. Currently only VIF and VBD devices
> are handled by the hotplug mechanism.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> 
> diff -r 1506b1f2ab0f -r 94532199f647 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c       Thu Feb 02 11:45:03 2012 +0100
> +++ b/tools/libxl/libxl.c       Thu Feb 02 11:48:06 2012 +0100
> @@ -982,6 +982,188 @@ out:
>      return rc;
>  }
> 
> +static int libxl__hotplug_generic_dispatcher(libxl__gc *gc, char *path,
> +                                uint32_t domid,
> +                                libxl__hotplug_status state,
> +                                libxl__device_generic_hotplug_actions *action,
> +                                void *device)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    char *spath = libxl__sprintf(gc, "%s/state", path);
> +    int rc = 0;
> +
> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "processing device %s with state %d",
> +                                      path, state);
> +    switch(state) {
> +    case HOTPLUG_DEVICE_INIT:
> +        if (action->init) {
> +            rc = action->init(ctx, domid, device);
> +            if (rc < 0) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to init"
> +                                                  " device %s", path);
> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> +                                HOTPLUG_DEVICE_ERROR);
> +                break;
> +            }
> +        }
> +        libxl__xs_write(gc, XBT_NULL, spath, "%d", HOTPLUG_DEVICE_CONNECT);
> +        break;
> +    case HOTPLUG_DEVICE_CONNECT:
> +        if (action->connect) {
> +            rc = action->connect(ctx, domid, device);
> +            if (rc < 0) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to connect"
> +                                                  " device %s", path);
> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> +                                HOTPLUG_DEVICE_ERROR);
> +                break;
> +            }
> +        }
> +        libxl__xs_write(gc, XBT_NULL, spath, "%d", HOTPLUG_DEVICE_CONNECTED);
> +        break;
> +    case HOTPLUG_DEVICE_CONNECTED:
> +        if (action->connected) {
> +            rc = action->connected(ctx, domid, device);
> +            if (rc < 0) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute "
> +                                                  "connected action on"
> +                                                  " device %s", path);
> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> +                                HOTPLUG_DEVICE_ERROR);
> +                break;
> +            }
> +        }
> +        break;
> +    case HOTPLUG_DEVICE_DISCONNECT:
> +        if (action->disconnect) {
> +            rc = action->disconnect(ctx, domid, device);
> +            if (rc < 0) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to unplug"
> +                                                  " device %s", path);
> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> +                                HOTPLUG_DEVICE_ERROR);
> +                break;
> +            }
> +        }
> +        libxl__xs_write(gc, XBT_NULL, spath, "%d",
> +                        HOTPLUG_DEVICE_DISCONNECTED);
> +        break;
> +    case HOTPLUG_DEVICE_DISCONNECTED:
> +        if (action->disconnected) {
> +            rc = action->disconnected(ctx, domid, device);
> +            if (rc < 0) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute "
> +                                                  "unpluged action on"
> +                                                  " device %s", path);
> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> +                                HOTPLUG_DEVICE_ERROR);
> +                break;
> +            }
> +        }
> +        break;
> +    case HOTPLUG_DEVICE_FORCE_DISCONNECT:
> +        if (action->force_disconnect) {
> +            rc = action->force_disconnect(ctx, domid, device);
> +            if (rc < 0) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to force "
> +                                                  "disconnect device "
> +                                                  "%s", path);
> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> +                                HOTPLUG_DEVICE_ERROR);
> +                break;
> +            }
> +        }
> +        libxl__xs_write(gc, XBT_NULL, spath, "%d",
> +                        HOTPLUG_DEVICE_DISCONNECTED);
> +        break;
> +    case HOTPLUG_DEVICE_ERROR:
> +        if (action->error)
> +            rc = action->error(ctx, domid, device);
> +        break;
> +    }
> +    return rc;

I can see that we are writing an error code to xenstore in case of
errors, but I fail to see where we are writing back a positive response.
Of course the caller of libxl_device_disk/nic_hotplug_add could watch
the backend state waiting for it to come up, but I think that having an
explicit ACK under /hotplug would be much better.

I haven't reviewed all the patches in detail but I think that this
series is going in the right thing direction, from the architectural
point of view.
The main requests for changes will probably be regarding event handling
and the usage of the new APIs.

Re: [PATCH 25 of 29 RFC] libxl: add libxl_hotplug_dispatch

[Roger Pau Monné]

2012/2/2 Stefano Stabellini <stefano.stabellini@eu.citrix.com>:
> On Thu, 2 Feb 2012, Roger Pau Monne wrote:
>> # HG changeset patch
>> # User Roger Pau Monne <roger.pau@entel.upc.edu>
>> # Date 1328179686 -3600
>> # Node ID 94532199f647fc03816fc5ae50ab53c8c5b80cd8
>> # Parent  1506b1f2ab0fe5f4cd5bcd86a566d5a7be5f838b
>> libxl: add libxl_hotplug_dispatch
>>
>> Added a new function to libxl API to handle device hotplug. Actions to
>> execute upon hotplug device state changes are handled using the
>> libxl_device_disk_hotplug_actions and libxl_device_nic_hotplug_actions
>> depending on the type of device. Currently only VIF and VBD devices
>> are handled by the hotplug mechanism.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
>>
>> diff -r 1506b1f2ab0f -r 94532199f647 tools/libxl/libxl.c
>> --- a/tools/libxl/libxl.c       Thu Feb 02 11:45:03 2012 +0100
>> +++ b/tools/libxl/libxl.c       Thu Feb 02 11:48:06 2012 +0100
>> @@ -982,6 +982,188 @@ out:
>>      return rc;
>>  }
>>
>> +static int libxl__hotplug_generic_dispatcher(libxl__gc *gc, char *path,
>> +                                uint32_t domid,
>> +                                libxl__hotplug_status state,
>> +                                libxl__device_generic_hotplug_actions *action,
>> +                                void *device)
>> +{
>> +    libxl_ctx *ctx = libxl__gc_owner(gc);
>> +    char *spath = libxl__sprintf(gc, "%s/state", path);
>> +    int rc = 0;
>> +
>> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "processing device %s with state %d",
>> +                                      path, state);
>> +    switch(state) {
>> +    case HOTPLUG_DEVICE_INIT:
>> +        if (action->init) {
>> +            rc = action->init(ctx, domid, device);
>> +            if (rc < 0) {
>> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to init"
>> +                                                  " device %s", path);
>> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
>> +                                HOTPLUG_DEVICE_ERROR);
>> +                break;
>> +            }
>> +        }
>> +        libxl__xs_write(gc, XBT_NULL, spath, "%d", HOTPLUG_DEVICE_CONNECT);
>> +        break;
>> +    case HOTPLUG_DEVICE_CONNECT:
>> +        if (action->connect) {
>> +            rc = action->connect(ctx, domid, device);
>> +            if (rc < 0) {
>> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to connect"
>> +                                                  " device %s", path);
>> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
>> +                                HOTPLUG_DEVICE_ERROR);
>> +                break;
>> +            }
>> +        }
>> +        libxl__xs_write(gc, XBT_NULL, spath, "%d", HOTPLUG_DEVICE_CONNECTED);
>> +        break;
>> +    case HOTPLUG_DEVICE_CONNECTED:
>> +        if (action->connected) {
>> +            rc = action->connected(ctx, domid, device);
>> +            if (rc < 0) {
>> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute "
>> +                                                  "connected action on"
>> +                                                  " device %s", path);
>> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
>> +                                HOTPLUG_DEVICE_ERROR);
>> +                break;
>> +            }
>> +        }
>> +        break;
>> +    case HOTPLUG_DEVICE_DISCONNECT:
>> +        if (action->disconnect) {
>> +            rc = action->disconnect(ctx, domid, device);
>> +            if (rc < 0) {
>> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to unplug"
>> +                                                  " device %s", path);
>> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
>> +                                HOTPLUG_DEVICE_ERROR);
>> +                break;
>> +            }
>> +        }
>> +        libxl__xs_write(gc, XBT_NULL, spath, "%d",
>> +                        HOTPLUG_DEVICE_DISCONNECTED);
>> +        break;
>> +    case HOTPLUG_DEVICE_DISCONNECTED:
>> +        if (action->disconnected) {
>> +            rc = action->disconnected(ctx, domid, device);
>> +            if (rc < 0) {
>> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute "
>> +                                                  "unpluged action on"
>> +                                                  " device %s", path);
>> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
>> +                                HOTPLUG_DEVICE_ERROR);
>> +                break;
>> +            }
>> +        }
>> +        break;
>> +    case HOTPLUG_DEVICE_FORCE_DISCONNECT:
>> +        if (action->force_disconnect) {
>> +            rc = action->force_disconnect(ctx, domid, device);
>> +            if (rc < 0) {
>> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to force "
>> +                                                  "disconnect device "
>> +                                                  "%s", path);
>> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
>> +                                HOTPLUG_DEVICE_ERROR);
>> +                break;
>> +            }
>> +        }
>> +        libxl__xs_write(gc, XBT_NULL, spath, "%d",
>> +                        HOTPLUG_DEVICE_DISCONNECTED);
>> +        break;
>> +    case HOTPLUG_DEVICE_ERROR:
>> +        if (action->error)
>> +            rc = action->error(ctx, domid, device);
>> +        break;
>> +    }
>> +    return rc;
>
> I can see that we are writing an error code to xenstore in case of
> errors, but I fail to see where we are writing back a positive response.
> Of course the caller of libxl_device_disk/nic_hotplug_add could watch
> the backend state waiting for it to come up, but I think that having an
> explicit ACK under /hotplug would be much better.

The Sequence is the following, but I'm not really sure this is the
most correct way. All states can go to HOTPLUG_DEVICE_ERROR, and the
following state changes:

HOTPLUG_DEVICE_INIT no state change
HOTPLUG_DEVICE_CONNECT -> HOTPLUG_DEVICE_CONNECTED (on success)
HOTPLUG_DEVICE_CONNECTED no change
HOTPLUG_DEVICE_DISCONNECT -> HOTPLUG_DEVICE_DISCONNECTED (on success)
HOTPLUG_DEVICE_FORCE_DISCONNECT -> HOTPLUG_DEVICE_DISCONNECTED (on success)


> I haven't reviewed all the patches in detail but I think that this
> series is going in the right thing direction, from the architectural
> point of view.
> The main requests for changes will probably be regarding event handling
> and the usage of the new APIs.

I guess so, when Ian Jackson event API is all in I will probably have
to rewrite some parts of the code.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 25 of 29 RFC] libxl: add libxl_hotplug_dispatch

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 7603 bytes --]

On Thu, 2 Feb 2012, Roger Pau Monné wrote:
> 2012/2/2 Stefano Stabellini <stefano.stabellini@eu.citrix.com>:
> > On Thu, 2 Feb 2012, Roger Pau Monne wrote:
> >> # HG changeset patch
> >> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> >> # Date 1328179686 -3600
> >> # Node ID 94532199f647fc03816fc5ae50ab53c8c5b80cd8
> >> # Parent  1506b1f2ab0fe5f4cd5bcd86a566d5a7be5f838b
> >> libxl: add libxl_hotplug_dispatch
> >>
> >> Added a new function to libxl API to handle device hotplug. Actions to
> >> execute upon hotplug device state changes are handled using the
> >> libxl_device_disk_hotplug_actions and libxl_device_nic_hotplug_actions
> >> depending on the type of device. Currently only VIF and VBD devices
> >> are handled by the hotplug mechanism.
> >>
> >> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> >>
> >> diff -r 1506b1f2ab0f -r 94532199f647 tools/libxl/libxl.c
> >> --- a/tools/libxl/libxl.c       Thu Feb 02 11:45:03 2012 +0100
> >> +++ b/tools/libxl/libxl.c       Thu Feb 02 11:48:06 2012 +0100
> >> @@ -982,6 +982,188 @@ out:
> >>      return rc;
> >>  }
> >>
> >> +static int libxl__hotplug_generic_dispatcher(libxl__gc *gc, char *path,
> >> +                                uint32_t domid,
> >> +                                libxl__hotplug_status state,
> >> +                                libxl__device_generic_hotplug_actions *action,
> >> +                                void *device)
> >> +{
> >> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> >> +    char *spath = libxl__sprintf(gc, "%s/state", path);
> >> +    int rc = 0;
> >> +
> >> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "processing device %s with state %d",
> >> +                                      path, state);
> >> +    switch(state) {
> >> +    case HOTPLUG_DEVICE_INIT:
> >> +        if (action->init) {
> >> +            rc = action->init(ctx, domid, device);
> >> +            if (rc < 0) {
> >> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to init"
> >> +                                                  " device %s", path);
> >> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> >> +                                HOTPLUG_DEVICE_ERROR);
> >> +                break;
> >> +            }
> >> +        }
> >> +        libxl__xs_write(gc, XBT_NULL, spath, "%d", HOTPLUG_DEVICE_CONNECT);
> >> +        break;
> >> +    case HOTPLUG_DEVICE_CONNECT:
> >> +        if (action->connect) {
> >> +            rc = action->connect(ctx, domid, device);
> >> +            if (rc < 0) {
> >> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to connect"
> >> +                                                  " device %s", path);
> >> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> >> +                                HOTPLUG_DEVICE_ERROR);
> >> +                break;
> >> +            }
> >> +        }
> >> +        libxl__xs_write(gc, XBT_NULL, spath, "%d", HOTPLUG_DEVICE_CONNECTED);
> >> +        break;
> >> +    case HOTPLUG_DEVICE_CONNECTED:
> >> +        if (action->connected) {
> >> +            rc = action->connected(ctx, domid, device);
> >> +            if (rc < 0) {
> >> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute "
> >> +                                                  "connected action on"
> >> +                                                  " device %s", path);
> >> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> >> +                                HOTPLUG_DEVICE_ERROR);
> >> +                break;
> >> +            }
> >> +        }
> >> +        break;
> >> +    case HOTPLUG_DEVICE_DISCONNECT:
> >> +        if (action->disconnect) {
> >> +            rc = action->disconnect(ctx, domid, device);
> >> +            if (rc < 0) {
> >> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to unplug"
> >> +                                                  " device %s", path);
> >> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> >> +                                HOTPLUG_DEVICE_ERROR);
> >> +                break;
> >> +            }
> >> +        }
> >> +        libxl__xs_write(gc, XBT_NULL, spath, "%d",
> >> +                        HOTPLUG_DEVICE_DISCONNECTED);
> >> +        break;
> >> +    case HOTPLUG_DEVICE_DISCONNECTED:
> >> +        if (action->disconnected) {
> >> +            rc = action->disconnected(ctx, domid, device);
> >> +            if (rc < 0) {
> >> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute "
> >> +                                                  "unpluged action on"
> >> +                                                  " device %s", path);
> >> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> >> +                                HOTPLUG_DEVICE_ERROR);
> >> +                break;
> >> +            }
> >> +        }
> >> +        break;
> >> +    case HOTPLUG_DEVICE_FORCE_DISCONNECT:
> >> +        if (action->force_disconnect) {
> >> +            rc = action->force_disconnect(ctx, domid, device);
> >> +            if (rc < 0) {
> >> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to force "
> >> +                                                  "disconnect device "
> >> +                                                  "%s", path);
> >> +                libxl__xs_write(gc, XBT_NULL, spath, "%d",
> >> +                                HOTPLUG_DEVICE_ERROR);
> >> +                break;
> >> +            }
> >> +        }
> >> +        libxl__xs_write(gc, XBT_NULL, spath, "%d",
> >> +                        HOTPLUG_DEVICE_DISCONNECTED);
> >> +        break;
> >> +    case HOTPLUG_DEVICE_ERROR:
> >> +        if (action->error)
> >> +            rc = action->error(ctx, domid, device);
> >> +        break;
> >> +    }
> >> +    return rc;
> >
> > I can see that we are writing an error code to xenstore in case of
> > errors, but I fail to see where we are writing back a positive response.
> > Of course the caller of libxl_device_disk/nic_hotplug_add could watch
> > the backend state waiting for it to come up, but I think that having an
> > explicit ACK under /hotplug would be much better.
> 
> The Sequence is the following, but I'm not really sure this is the
> most correct way. All states can go to HOTPLUG_DEVICE_ERROR, and the
> following state changes:
> 
> HOTPLUG_DEVICE_INIT no state change
> HOTPLUG_DEVICE_CONNECT -> HOTPLUG_DEVICE_CONNECTED (on success)
> HOTPLUG_DEVICE_CONNECTED no change
> HOTPLUG_DEVICE_DISCONNECT -> HOTPLUG_DEVICE_DISCONNECTED (on success)
> HOTPLUG_DEVICE_FORCE_DISCONNECT -> HOTPLUG_DEVICE_DISCONNECTED (on success)

I see: it is the same as before but under the hotplug path. Somehow I
missed the code that is handling all that, however it is not a bad idea
to keep the same protocol.

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

[PATCH 26 of 29 RFC] xldeviced: new daemon to execute hotplug scripts

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328179909 -3600
# Node ID e904f98a4445fee7489490b5ccc00b7bb62d796d
# Parent  94532199f647fc03816fc5ae50ab53c8c5b80cd8
xldeviced: new daemon to execute hotplug scripts

This patch introduces xldeviced, a new daemon that monitors
/hotplug/<domid> and is in charge of attaching the passed devices to
the desired DomU. Makes use of the previously introduced libxl hotplug
functions.

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

diff -r 94532199f647 -r e904f98a4445 tools/libxl/Makefile
--- a/tools/libxl/Makefile	Thu Feb 02 11:48:06 2012 +0100
+++ b/tools/libxl/Makefile	Thu Feb 02 11:51:49 2012 +0100
@@ -13,7 +13,7 @@ XLUMINOR = 0
 
 CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations \
 	-Wno-declaration-after-statement -Wformat-nonliteral
-CFLAGS += -I. -fPIC
+CFLAGS += -I. -fPIC -g
 
 ifeq ($(CONFIG_Linux),y)
 LIBUUID_LIBS += -luuid
@@ -60,12 +60,16 @@ LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_
 	libxlu_disk_l.o libxlu_disk.o
 $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
 
-CLIENTS = xl testidl
+CLIENTS = xl xldeviced testidl
 
 XL_OBJS = xl.o xl_cmdimpl.o xl_cmdtable.o
 $(XL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
 $(XL_OBJS): CFLAGS += $(CFLAGS_libxenlight)
 
+XLDEVICED_OBJS = xldeviced.o
+$(XLDEVICED_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
+$(XLDEVICED_OBJS): CFLAGS += $(CFLAGS_libxenlight)
+
 testidl.o: CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight)
 testidl.c: libxl_types.idl gentest.py libxl.h $(AUTOINCS)
 	$(PYTHON) gentest.py libxl_types.idl testidl.c.new
@@ -140,8 +144,12 @@ libxlutil.a: $(LIBXLU_OBJS)
 xl: $(XL_OBJS) libxlutil.so libxenlight.so
 	$(CC) $(LDFLAGS) -o $@ $(XL_OBJS) libxlutil.so $(LDLIBS_libxenlight) $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
+xldeviced: $(XLDEVICED_OBJS) libxlutil.so libxenlight.so
+	$(CC) $(LDFLAGS) -o $@ $(XLDEVICED_OBJS) libxlutil.so $(LDLIBS_libxenlight) $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+
 testidl: testidl.o libxlutil.so libxenlight.so
 	$(CC) $(LDFLAGS) -o $@ testidl.o libxlutil.so $(LDLIBS_libxenlight) $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+    
 
 .PHONY: install
 install: all
@@ -151,6 +159,7 @@ install: all
 	$(INSTALL_DIR) $(DESTDIR)$(BASH_COMPLETION_DIR)
 	$(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_DIR)
 	$(INSTALL_PROG) xl $(DESTDIR)$(SBINDIR)
+	$(INSTALL_PROG) xldeviced $(DESTDIR)$(SBINDIR)
 	$(INSTALL_PROG) libxenlight.so.$(MAJOR).$(MINOR) $(DESTDIR)$(LIBDIR)
 	ln -sf libxenlight.so.$(MAJOR).$(MINOR) $(DESTDIR)$(LIBDIR)/libxenlight.so.$(MAJOR)
 	ln -sf libxenlight.so.$(MAJOR) $(DESTDIR)$(LIBDIR)/libxenlight.so
diff -r 94532199f647 -r e904f98a4445 tools/libxl/xldeviced.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/xldeviced.c	Thu Feb 02 11:51:49 2012 +0100
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2012
+ * Author Roger Pau Monné <roger.pau@entel.upc.edu>
+ *
+ * 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.
+ */
+
+#include "libxl_osdeps.h"
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <assert.h>
+
+#include <xenctrl.h>
+#include <xs.h>
+
+#include "libxl.h"
+#include "libxl_utils.h"
+#include "libxlutil.h"
+#include "xldeviced.h"
+
+xentoollog_logger_stdiostream *logger;
+libxl_ctx *ctx;
+
+static xentoollog_level minmsglevel = XTL_PROGRESS;
+struct xs_handle *xs;
+
+libxl_device_disk_hotplug_actions disk_handler = {
+    .connect = libxl_device_disk_add,
+    .disconnect = libxl_device_disk_remove,
+    .force_disconnect = libxl_device_disk_destroy,
+};
+
+libxl_device_nic_hotplug_actions nic_handler = {
+    .connect = libxl_device_nic_add,
+    .disconnect = libxl_device_nic_remove,
+    .force_disconnect = libxl_device_nic_destroy,
+};
+
+static int create_pidfile(char *file)
+{
+    pid_t pid;
+    int rc, fd;
+    char *s;
+
+    fd = open(file, O_WRONLY|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR);
+    if (fd < 0) {
+        LOG("unable to open pidfile %s", file);
+        return -1;
+    }
+    pid = getpid();
+    rc = asprintf(&s, "%d\n", pid);
+    if (rc < 0)
+        return -1;
+    libxl_write_exactly(NULL, fd, s, rc, NULL, NULL);
+
+    close(fd);
+    return 0;
+}
+
+int main(int argc, char **argv)
+{
+    int opt = 0;
+    int daemonize = 1;
+    int status, rc = 0, ret = 0;
+    char *logfilename = "xldeviced";
+    char *pidfile = PIDFILE;
+    int logfile;
+
+    while ((opt = getopt(argc, argv, "+vfp:")) >= 0) {
+        switch (opt) {
+        case 'v':
+            if (minmsglevel > 0) minmsglevel--;
+            break;
+        case 'f':
+            daemonize = 0;
+            break;
+        case 'p':
+            pidfile = optarg;
+            break;
+        default:
+            fprintf(stderr, "unknown global option\n");
+            exit(2);
+        }
+    }
+
+    logger = xtl_createlogger_stdiostream(stderr, minmsglevel,  0);
+    if (!logger) exit(1);
+
+    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) {
+        fprintf(stderr, "cannot init xldeviced context\n");
+        exit(1);
+    }
+
+    if (daemonize) {
+        char *fullname;
+        pid_t child1, got_child;
+        int nullfd;
+
+        child1 = libxl_fork(ctx);
+        if (child1) {
+
+            for (;;) {
+                got_child = waitpid(child1, &status, 0);
+                if (got_child == child1) break;
+                assert(got_child == -1);
+                if (errno != EINTR) {
+                    perror("failed to wait for daemonizing child");
+                    ret = ERROR_FAIL;
+                    goto out;
+                }
+            }
+            if (status) {
+                libxl_report_child_exitstatus(ctx, XTL_ERROR,
+                           "daemonizing child", child1, status);
+                ret = ERROR_FAIL;
+                goto out;
+            }
+            ret = 0;
+            goto out;
+        }
+
+        rc = libxl_ctx_postfork(ctx);
+        if (rc) {
+            LOG("failed to reinitialise context after fork");
+            exit(-1);
+        }
+
+        rc = libxl_create_logfile(ctx, logfilename, &fullname);
+        if (rc) {
+            LOG("failed to open logfile %s: %s",fullname,strerror(errno));
+            exit(-1);
+        }
+
+        CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND,
+                                   0644) )<0);
+        free(fullname);
+
+        CHK_ERRNO(( nullfd = open("/dev/null", O_RDONLY) )<0);
+        dup2(nullfd, 0);
+        dup2(logfile, 1);
+        dup2(logfile, 2);
+
+        CHK_ERRNO(daemon(0, 1) < 0);
+    }
+    /* Write pid */
+    if (create_pidfile(pidfile) < 0) {
+        LOG("unable to write pid to %s", pidfile);
+        goto out_clean;
+    }
+
+    /* Watch /hotplug/<my_dom_id> for changes */
+    ret = libxl_setup_hotplug_listener(ctx);
+    if (ret < 0) {
+        LOG("Failed to setup xenstore watches");
+        goto out_clean;
+    }
+
+    ret = libxl_hotplug_dispatch(ctx, &disk_handler, &nic_handler);
+
+out_clean:
+    unlink(pidfile);
+out:
+    libxl_ctx_free(ctx);
+    xtl_logger_destroy((xentoollog_logger*)logger);
+    return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff -r 94532199f647 -r e904f98a4445 tools/libxl/xldeviced.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/xldeviced.h	Thu Feb 02 11:51:49 2012 +0100
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2012
+ * Author Roger Pau Monné <roger.pau@entel.upc.edu>
+ *
+ * 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.
+ */
+
+#ifndef XLDEVICED_H
+#define XLDEVICED_H
+
+#include "xentoollog.h"
+
+extern libxl_ctx *ctx;
+extern xentoollog_logger_stdiostream *logger;
+int logfile = 2;
+
+#define PIDFILE "/var/run/xldeviced.pid"
+
+#define LOG(_f, _a...)   dolog(__FILE__, __LINE__, __func__, _f "\n", ##_a)
+
+static void dolog(const char *file, int line, const char *func, char *fmt, ...)
+     __attribute__((format(printf,4,5)));
+
+static void dolog(const char *file, int line, const char *func, char *fmt, ...)
+{
+    va_list ap;
+    char *s;
+    int rc;
+
+    va_start(ap, fmt);
+    rc = vasprintf(&s, fmt, ap);
+    va_end(ap);
+    if (rc >= 0)
+        libxl_write_exactly(NULL, logfile, s, rc, NULL, NULL);
+}
+
+#define CHK_ERRNO( call ) ({                                            \
+        int chk_errno = (call);                                         \
+        if (chk_errno < 0) {                                            \
+            fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n",          \
+                    __FILE__,__LINE__, strerror(chk_errno), #call);     \
+            exit(-ERROR_FAIL);                                          \
+        }                                                               \
+    })
+
+#endif /* XLDEVICED_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 27 of 29 RFC] init: updated Linux and NetBSD init scripts to launch xldeviced

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328180098 -3600
# Node ID 42bd734ce75d232194372c329a9af9b73e9090fb
# Parent  e904f98a4445fee7489490b5ccc00b7bb62d796d
init: updated Linux and NetBSD init scripts to launch xldeviced

Updated both init scripts to launch xldeviced when starting
xencommons.

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

diff -r e904f98a4445 -r 42bd734ce75d tools/hotplug/Linux/init.d/xencommons
--- a/tools/hotplug/Linux/init.d/xencommons	Thu Feb 02 11:51:49 2012 +0100
+++ b/tools/hotplug/Linux/init.d/xencommons	Thu Feb 02 11:54:58 2012 +0100
@@ -27,6 +27,7 @@ fi
 test -f $xencommons_config/xencommons && . $xencommons_config/xencommons
 
 XENCONSOLED_PIDFILE=/var/run/xenconsoled.pid
+XLDEVICED_PIDFILE=/var/run/xldeviced.pid
 shopt -s extglob
 
 # not running in Xen dom0 or domU
@@ -95,13 +96,16 @@ do_start () {
 
 		echo Setting domain 0 name...
 		xenstore-write "/local/domain/0/name" "Domain-0"
+		echo Setting domain 0 id...
+		xenstore-write "/local/domain/0/domid" "0"
 	fi
 
 	echo Starting xenconsoled...
 	test -z "$XENCONSOLED_TRACE" || XENCONSOLED_ARGS=" --log=$XENCONSOLED_TRACE"
 	xenconsoled --pid-file=$XENCONSOLED_PIDFILE $XENCONSOLED_ARGS
-	test -z "$XENBACKENDD_DEBUG" || XENBACKENDD_ARGS="-d"
-	test "`uname`" != "NetBSD" || xenbackendd $XENBACKENDD_ARGS
+	echo Starting xldeviced...
+	test -z "$XLDEVICED_DEBUG" || XLDEVICED_ARGS="-vvv"
+	xldeviced -p $XLDEVICED_PIDFILE $XLDEVICED_ARGS
 }
 do_stop () {
         echo Stopping xenconsoled
@@ -110,7 +114,12 @@ do_stop () {
 		while kill -9 $pid >/dev/null 2>&1; do sleep 0.1; done
 		rm -f $XENCONSOLED_PIDFILE
 	fi
-
+	echo Stopping xldeviced
+	if read 2>/dev/null <$XLDEVICED_PIDFILE pid; then
+		kill $pid
+		while kill -9 $pid >/dev/null 2>&1; do sleep 0.1; done
+		rm -f $XLDEVICED_PIDFILE
+	fi
 	echo WARNING: Not stopping xenstored, as it cannot be restarted.
 }
 
diff -r e904f98a4445 -r 42bd734ce75d tools/hotplug/NetBSD/rc.d/xencommons
--- a/tools/hotplug/NetBSD/rc.d/xencommons	Thu Feb 02 11:51:49 2012 +0100
+++ b/tools/hotplug/NetBSD/rc.d/xencommons	Thu Feb 02 11:54:58 2012 +0100
@@ -22,6 +22,7 @@ required_files="/kern/xen/privcmd"
 
 XENSTORED_PIDFILE="/var/run/xenstored.pid"
 XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid"
+XLDEVICED_PIDFILE="/var/run/xldeviced.pid"
 #XENCONSOLED_TRACE="/var/log/xen/xenconsole-trace.log"
 #XENSTORED_TRACE="/var/log/xen/xenstore-trace.log"
 
@@ -42,7 +43,7 @@ xen_startcmd()
 			XENSTORED_ROOTDIR="/var/lib/xenstored"
 		fi
 		rm -f ${XENSTORED_ROOTDIR}/tdb* >/dev/null 2>&1
-		printf "Starting xenservices: xenstored, xenconsoled."
+		printf "Starting xenservices: xenstored, xenconsoled, xldeviced."
 		XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}"
 		if [ -n "${XENSTORED_TRACE}" ]; then
 			XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log"
@@ -54,7 +55,7 @@ xen_startcmd()
 			sleep 1
 		done
 	else
-		printf "Starting xenservices: xenconsoled."
+		printf "Starting xenservices: xenconsoled, xldeviced."
 	fi
 
 	XENCONSOLED_ARGS=""
@@ -63,21 +64,26 @@ xen_startcmd()
 	fi
 
 	${SBINDIR}/xenconsoled ${XENCONSOLED_ARGS}
+	${SBINDIR}/xldeviced -p ${XLDEVICED_PIDFILE}
 
 	printf "\n"
 
 	printf "Setting domain 0 name.\n"
 	${BINDIR}/xenstore-write "/local/domain/0/name" "Domain-0"
+	printf "Setting domain 0 id.\n"
+	${BINDIR}/xenstore-write "/local/domain/0/domid" "0"
 }
 
 xen_stop()
 {
 	pids=""
-	printf "Stopping xencommons.\n"
+	printf "Stopping xencommons, xldeviced.\n"
 	printf "WARNING: Not stopping xenstored, as it cannot be restarted.\n"
 
 	rc_pid=$(check_pidfile ${XENCONSOLED_PIDFILE} ${SBINDIR}/xenconsoled)
 	pids="$pids $rc_pid"
+	rc_pid=$(check_pidfile ${XLDEVICED_PIDFILE} ${SBINDIR}/xldeviced)
+	pids="$pids $rc_pid"
 
 	kill -${sig_stop:-TERM} $pids
 	wait_for_pids $pids
@@ -95,12 +101,17 @@ xen_status()
 		pids="$pids $xenconsoled_pid"
 	fi
 
-	if test -n "$xenconsoled_pid" -a -n "$xenstored_pid";
+	xldeviced_pid=$(check_pidfile ${XLDEVICED_PIDFILE} ${SBINDIR}/xldeviced)
+	if test -n ${xldeviced_pid}; then
+		pids="$pids $xldeviced_pid"
+	fi
+
+	if test -n "$xenconsoled_pid" -a -n "$xenstored_pid" -a -n "$xldeviced_pid";
 	then
 		echo "xencommons are running as pids $pids."
 		return 0
 	fi
-	if test -z "$xenconsoled_pid" -a -z "$xenstored_pid";
+	if test -z "$xenconsoled_pid" -a -z "$xenstored_pid" -a -z "$xldeviced_pid";
 	then
 		echo "xencommons are not running."
 		return 0
@@ -116,6 +127,11 @@ xen_status()
 	else
 		echo "xenconsoled is not running."
 	fi
+	if test -n $xldeviced_pid; then
+		echo "xldeviced is running as pid $xldeviced_pid."
+	else
+		echo "xldeviced is not running."
+	fi
 }
 
 load_rc_config $name

[PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328189112 -3600
# Node ID 35164add2785b8606c0e9b771206d83862afd2c6
# Parent  42bd734ce75d232194372c329a9af9b73e9090fb
libxl: add libxl__find_free_vdev

Add a function that returns the first free xvd<x> device, used to
attach a DomU image and execute the bootloader against that.

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

diff -r 42bd734ce75d -r 35164add2785 tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Thu Feb 02 11:54:58 2012 +0100
+++ b/tools/libxl/libxl_bootloader.c	Thu Feb 02 14:25:12 2012 +0100
@@ -318,6 +318,39 @@ static void parse_bootloader_result(libx
     }
 }
 
+static char *libxl__find_free_vdev(libxl__gc *gc)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *vdev;
+    libxl_device_disk *list;
+    int num;
+    int used;
+
+    list = libxl_device_disk_list(ctx, 0, &num);
+    if (!list || num == 0) {
+        return "xvda";
+    }
+
+    for (char device = 'a'; device <= 'z'; device += 1) {
+        used = 0;
+        vdev = libxl__sprintf(gc, "xvd%c", device);
+        if (!vdev) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to allocate memory");
+            return NULL;
+        }
+        for (int i = 0; i < num; i++) {
+            if (strcmp(list[0].vdev, vdev) == 0) {
+                used = 1;
+                break;
+            }
+        }
+        if (!used)
+            return vdev;
+    }
+    LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to find a free device");
+    return NULL;
+}
+
 int libxl_run_bootloader(libxl_ctx *ctx,
                          libxl_domain_build_info *info,
                          libxl_device_disk *disk,
@@ -328,6 +361,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
     char *fifo = NULL;
     char *diskpath = NULL;
     char **args = NULL;
+    char *vdev_save = NULL;
 
     char tempdir_template[] = "/var/run/libxl/bl.XXXXXX";
     char *tempdir;
@@ -378,6 +412,11 @@ int libxl_run_bootloader(libxl_ctx *ctx,
         goto out_close;
     }
 
+    vdev_save = disk->vdev;
+    disk->vdev = libxl__find_free_vdev(gc);
+    if (!disk->vdev)
+        goto out_close;
+
     diskpath = libxl_device_disk_local_attach(ctx, disk);
     if (!diskpath) {
         goto out_close;
@@ -446,6 +485,8 @@ int libxl_run_bootloader(libxl_ctx *ctx,
 out_close:
     if (diskpath) {
         libxl_device_disk_local_detach(ctx, disk);
+        if (vdev_save)
+            disk->vdev = vdev_save;
         free(diskpath);
     }
     if (fifo_fd > -1)

Re: [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev

[Ian Jackson]

Roger Pau Monne writes ("[Xen-devel] [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev"):
> libxl: add libxl__find_free_vdev
> 
> Add a function that returns the first free xvd<x> device, used to
> attach a DomU image and execute the bootloader against that.

I'm afraid I don't understand the purpose of this at all.

Also the comment "add <some function>" is not accurate because the
real change is to run the bootloader on some other disk.

Ian.

Re: [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev

[Roger Pau Monné]

2012/2/9 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev"):
>> libxl: add libxl__find_free_vdev
>>
>> Add a function that returns the first free xvd<x> device, used to
>> attach a DomU image and execute the bootloader against that.
>
> I'm afraid I don't understand the purpose of this at all.

The main purpose of this series is to separate the control code from
the device code (make xl able to plug devices from a domain different
than dom0). Since the root image might not be stored on the dom0, we
need to be able to make this image available to the dom0 somehow, to
extract the kernel and ramdisk. One option is to execute pygrub from
the device domain, the other option is to plug the domU hard drive to
the dom0, run the bootloader to extract the kernel/initrd and unplug
the hd.

The later is the one implemented here partially, since
libxl_device_disk_local_attach still doesn't call the driver domain to
attach the device, this is done on a later patch (yes, it's probably a
mess).

> Also the comment "add <some function>" is not accurate because the
> real change is to run the bootloader on some other disk.

Not really, the bootloader was currently run against the vbd/phy
partition currently (eg: pygrub /home/xen/disk.img), not against the
vdev of the hd, because the hd was never locally attached
(libxl_device_disk_local_attach just returned the path to the phy
partition or vbd image, but never attached it to the dom0).

Following changes in this series make libxl_device_disk_local_attach
honor it's name, and truly attach the device to the dom0, creating a
new vdev in the dom0. Since the user might be using vdevs (xvda,
xvdb...) for it's own purposes, we need to find a free vdev in the
dom0 and attach the vbd to that, that's why this function is needed.

Hope this is clarifies things a little bit, and sorry for the delay.

Regards, Roger.

> Ian.

Re: [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev

[Ian Jackson]

Roger Pau Monné writes ("Re: [Xen-devel] [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev"):
> The main purpose of this series is to separate the control code from
> the device code (make xl able to plug devices from a domain different
> than dom0). Since the root image might not be stored on the dom0, we
> need to be able to make this image available to the dom0 somehow, to
> extract the kernel and ramdisk. One option is to execute pygrub from
> the device domain, the other option is to plug the domU hard drive to
> the dom0, run the bootloader to extract the kernel/initrd and unplug
> the hd.

Wow.  So this relies on a frontend in dom0 ?

I think for now it might be wise to simply expect the kernel/ramdisk
to exist as files in dom0 somewhere.

Ian.

Re: [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev

[Roger Pau Monné]

2012/2/15 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> Roger Pau Monné writes ("Re: [Xen-devel] [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev"):
>> The main purpose of this series is to separate the control code from
>> the device code (make xl able to plug devices from a domain different
>> than dom0). Since the root image might not be stored on the dom0, we
>> need to be able to make this image available to the dom0 somehow, to
>> extract the kernel and ramdisk. One option is to execute pygrub from
>> the device domain, the other option is to plug the domU hard drive to
>> the dom0, run the bootloader to extract the kernel/initrd and unplug
>> the hd.
>
> Wow.  So this relies on a frontend in dom0 ?

Yes, that's been discussed before, and seemed like the way to go:

http://lists.xen.org/archives/html/xen-devel/2012-01/msg02663.html
http://lists.xen.org/archives/html/xen-devel/2012-01/msg02761.html

I've been playing with this implementation and seems to work ok.

> I think for now it might be wise to simply expect the kernel/ramdisk
> to exist as files in dom0 somewhere.

Well, that is already archived using kernel and ramdisk parameters in
configuration file right?

> Ian.

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

[PATCH 29 of 29 RFC] libxl: delegate plug/unplug of disk and nic devices to xldeviced

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1328189192 -3600
# Node ID 7de94a26474f869177ce16b59b61421bf342f820
# Parent  35164add2785b8606c0e9b771206d83862afd2c6
libxl: delegate plug/unplug of disk and nic devices to xldeviced

Change the calls to libxl_device_<type>_<action> to
libxl_device_<type>_hotplug_<action> for disk and nic device types.
Alsoe enables hotplug script calling from libxl itself, by disabling
some udev rules and removing the commented code in
libxl__device_hotplug.

Bootloading is handled attaching the requested device to Dom0 (xvda),
and executing pygrub against that device (/dev/xvda). This should be
fixed in future versions, since xvda might already be in use.

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

diff -r 35164add2785 -r 7de94a26474f tools/hotplug/Linux/xen-backend.rules
--- a/tools/hotplug/Linux/xen-backend.rules	Thu Feb 02 14:25:12 2012 +0100
+++ b/tools/hotplug/Linux/xen-backend.rules	Thu Feb 02 14:26:32 2012 +0100
@@ -1,11 +1,11 @@
-SUBSYSTEM=="xen-backend", KERNEL=="tap*", RUN+="/etc/xen/scripts/blktap $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vbd*", RUN+="/etc/xen/scripts/block $env{ACTION}"
+# SUBSYSTEM=="xen-backend", KERNEL=="tap*", RUN+="/etc/xen/scripts/blktap $env{ACTION}"
+# SUBSYSTEM=="xen-backend", KERNEL=="vbd*", RUN+="/etc/xen/scripts/block $env{ACTION}"
 SUBSYSTEM=="xen-backend", KERNEL=="vtpm*", RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
 SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
+# SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
+# SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
 SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
-SUBSYSTEM=="xen-backend", ACTION=="remove", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
+# SUBSYSTEM=="xen-backend", ACTION=="remove", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
 KERNEL=="evtchn", NAME="xen/%k"
 SUBSYSTEM=="xen", KERNEL=="blktap[0-9]*", NAME="xen/%k", MODE="0600"
 SUBSYSTEM=="blktap2", KERNEL=="blktap[0-9]*", NAME="xen/blktap-2/%k", MODE="0600"
diff -r 35164add2785 -r 7de94a26474f tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 14:25:12 2012 +0100
+++ b/tools/libxl/libxl.c	Thu Feb 02 14:26:32 2012 +0100
@@ -1714,12 +1714,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
 
     ret = 0;
 
-    libxl_device_disk_remove(ctx, domid, disks + i);
-    libxl_device_disk_add(ctx, domid, disk);
+    libxl_device_disk_hotplug_remove(ctx, domid, disks + i);
+    libxl_device_disk_hotplug_add(ctx, domid, disk);
     stubdomid = libxl_get_stubdom_id(ctx, domid);
     if (stubdomid) {
-        libxl_device_disk_remove(ctx, stubdomid, disks + i);
-        libxl_device_disk_add(ctx, stubdomid, disk);
+        libxl_device_disk_hotplug_remove(ctx, stubdomid, disks + i);
+        libxl_device_disk_hotplug_add(ctx, stubdomid, disk);
     }
 out:
     for (i = 0; i < num; i++)
@@ -1735,52 +1735,13 @@ char * libxl_device_disk_local_attach(li
     char *ret = NULL;
     int rc;
 
-    rc = libxl__device_disk_set_backend(gc, disk);
-    if (rc) goto out;
-
-    switch (disk->backend) {
-        case LIBXL_DISK_BACKEND_PHY:
-            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY disk %s",
-                       disk->pdev_path);
-            dev = disk->pdev_path;
-            break;
-        case LIBXL_DISK_BACKEND_TAP:
-            switch (disk->format) {
-            case LIBXL_DISK_FORMAT_RAW:
-                /* optimise away the early tapdisk attach in this case */
-                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching"
-                           " tap disk %s directly (ie without using blktap)",
-                           disk->pdev_path);
-                dev = disk->pdev_path;
-                break;
-            case LIBXL_DISK_FORMAT_VHD:
-                dev = libxl__blktap_devpath(gc, disk->pdev_path,
-                                            disk->format);
-                break;
-            case LIBXL_DISK_FORMAT_QCOW:
-            case LIBXL_DISK_FORMAT_QCOW2:
-                abort(); /* prevented by libxl__device_disk_set_backend */
-            default:
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                           "unrecognized disk format: %d", disk->format);
-                break;
-            }
-            break;
-        case LIBXL_DISK_BACKEND_QDISK:
-            if (disk->format != LIBXL_DISK_FORMAT_RAW) {
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally"
-                           " attach a qdisk image if the format is not raw");
-                break;
-            }
-            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
-                       disk->pdev_path);
-            dev = disk->pdev_path;
-            break;
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
-                "type: %d", disk->backend);
-            break;
+    rc = libxl_device_disk_hotplug_add(ctx, 0, disk);
+    if (rc < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to attach disk %s to Dom0",
+                                          disk->pdev_path);
+        goto out;
     }
+    dev = libxl__sprintf(gc, "/dev/%s", disk->vdev);
 
  out:
     if (dev != NULL)
@@ -1791,14 +1752,17 @@ char * libxl_device_disk_local_attach(li
 
 int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk)
 {
-    /* Nothing to do for PHYSTYPE_PHY. */
-
-    /*
-     * For other device types assume that the blktap2 process is
-     * needed by the soon to be started domain and do nothing.
-     */
-
-    return 0;
+    GC_INIT(ctx);
+    int rc;
+
+    rc = libxl_device_disk_hotplug_destroy(ctx, 0, disk);
+    if (rc < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to attach disk %s to Dom0",
+                                          disk->pdev_path);
+    }
+
+    GC_FREE;
+    return rc;
 }
 
 /******************************************************************************/
diff -r 35164add2785 -r 7de94a26474f tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Thu Feb 02 14:25:12 2012 +0100
+++ b/tools/libxl/libxl_create.c	Thu Feb 02 14:26:32 2012 +0100
@@ -500,12 +500,11 @@ static int do_domain_create(libxl__gc *g
             goto error_out;
     }
 
-
+#if 0
     for (i = 0; i < d_config->num_disks; i++) {
-        ret = libxl__device_disk_set_backend(gc, &d_config->disks[i]);
-        if (ret) goto error_out;
+        d_config->disks[i].backend = LIBXL_DISK_BACKEND_PHY;
     }
-
+#endif
     if ( restore_fd < 0 ) {
         ret = libxl_run_bootloader(ctx, &d_config->b_info, d_config->num_disks > 0 ? &d_config->disks[0] : NULL, domid);
         if (ret) {
@@ -534,7 +533,7 @@ static int do_domain_create(libxl__gc *g
     store_libxl_entry(gc, domid, dm_info);
 
     for (i = 0; i < d_config->num_disks; i++) {
-        ret = libxl_device_disk_add(ctx, domid, &d_config->disks[i]);
+        ret = libxl_device_disk_hotplug_add(ctx, domid, &d_config->disks[i]);
         if (ret) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                        "cannot add disk %d to domain: %d", i, ret);
@@ -543,7 +542,7 @@ static int do_domain_create(libxl__gc *g
         }
     }
     for (i = 0; i < d_config->num_vifs; i++) {
-        ret = libxl_device_nic_add(ctx, domid, &d_config->vifs[i]);
+        ret = libxl_device_nic_hotplug_add(ctx, domid, &d_config->vifs[i]);
         if (ret) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                        "cannot add nic %d to domain: %d", i, ret);
diff -r 35164add2785 -r 7de94a26474f tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Feb 02 14:25:12 2012 +0100
+++ b/tools/libxl/libxl_device.c	Thu Feb 02 14:26:32 2012 +0100
@@ -805,7 +805,20 @@ int libxl__devices_destroy(libxl__gc *gc
                 dev.kind = kind;
                 dev.devid = atoi(devs[j]);
 
-                libxl__device_destroy(gc, &dev);
+                switch(dev.kind) {
+                case LIBXL__DEVICE_KIND_VIF:
+                case LIBXL__DEVICE_KIND_VBD:
+                case LIBXL__DEVICE_KIND_QDISK:
+                    libxl__device_hotplug_disconnect(gc, &dev,
+                             HOTPLUG_DEVICE_FORCE_DISCONNECT);
+                    break;
+                case LIBXL__DEVICE_KIND_PCI:
+                case LIBXL__DEVICE_KIND_VFB:
+                case LIBXL__DEVICE_KIND_VKBD:
+                case LIBXL__DEVICE_KIND_CONSOLE:
+                    libxl__device_destroy(gc, &dev);
+                    break;
+                }
             }
         }
     }
diff -r 35164add2785 -r 7de94a26474f tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Thu Feb 02 14:25:12 2012 +0100
+++ b/tools/libxl/libxl_dm.c	Thu Feb 02 14:26:32 2012 +0100
@@ -674,12 +674,12 @@ retry_transaction:
             goto retry_transaction;
 
     for (i = 0; i < num_disks; i++) {
-        ret = libxl_device_disk_add(ctx, domid, &disks[i]);
+        ret = libxl_device_disk_hotplug_add(ctx, domid, &disks[i]);
         if (ret)
             goto out_free;
     }
     for (i = 0; i < num_vifs; i++) {
-        ret = libxl_device_nic_add(ctx, domid, &vifs[i]);
+        ret = libxl_device_nic_hotplug_add(ctx, domid, &vifs[i]);
         if (ret)
             goto out_free;
     }
diff -r 35164add2785 -r 7de94a26474f tools/libxl/libxl_linux.c
--- a/tools/libxl/libxl_linux.c	Thu Feb 02 14:25:12 2012 +0100
+++ b/tools/libxl/libxl_linux.c	Thu Feb 02 14:26:32 2012 +0100
@@ -27,7 +27,7 @@ int libxl__try_phy_backend(mode_t st_mod
 }
 
 /* Hotplug scripts helpers */
-#if 0
+
 static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -175,12 +175,12 @@ out:
     free(args);
     return rc;
 }
-#endif /* 0 */
+
 int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
                           libxl__hotplug_action action)
 {
     int rc = 0;
-#if 0
+
     switch (dev->backend_kind) {
     case LIBXL__DEVICE_KIND_VIF:
         rc = libxl__hotplug_nic(gc, dev, action);
@@ -192,6 +192,6 @@ int libxl__device_hotplug(libxl__gc *gc,
         rc = 0;
         break;
     }
-#endif /* 0 */
+
     return rc;
 }
diff -r 35164add2785 -r 7de94a26474f tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 14:25:12 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 14:26:32 2012 +0100
@@ -4574,7 +4574,7 @@ int main_networkattach(int argc, char **
             return 1;
         }
     }
-    if (libxl_device_nic_add(ctx, domid, &nic)) {
+    if (libxl_device_nic_hotplug_add(ctx, domid, &nic)) {
         fprintf(stderr, "libxl_device_nic_add failed.\n");
         return 1;
     }
@@ -4684,7 +4684,7 @@ int main_blockattach(int argc, char **ar
         return 0;
     }
 
-    if (libxl_device_disk_add(ctx, fe_domid, &disk)) {
+    if (libxl_device_disk_hotplug_add(ctx, fe_domid, &disk)) {
         fprintf(stderr, "libxl_device_disk_add failed.\n");
     }
     return 0;
@@ -4742,7 +4742,7 @@ int main_blockdetach(int argc, char **ar
         fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
         return 1;
     }
-    if (libxl_device_disk_remove(ctx, domid, &disk)) {
+    if (libxl_device_disk_hotplug_remove(ctx, domid, &disk) < 0) {
         fprintf(stderr, "libxl_device_disk_remove failed.\n");
     }
     return 0;

Re: [PATCH 29 of 29 RFC] libxl: delegate plug/unplug of disk and nic devices to xldeviced

[Ian Jackson]

Roger Pau Monne writes ("[Xen-devel] [PATCH 29 of 29 RFC] libxl: delegate plug/u> libxl: delegate plug/unplug of disk and nic devices to xldeviced
> 
> Change the calls to libxl_device_<type>_<action> to
> libxl_device_<type>_hotplug_<action> for disk and nic device types.
> Alsoe enables hotplug script calling from libxl itself, by disabling
> some udev rules and removing the commented code in
> libxl__device_hotplug.

Again, I don't think this is the right way to organise the change to
this behaviour.

The right thing to do is probably to change individual device types
one at a type, and to make the new functionality have the same
libxl_device_disk_add name (etc.) as before.

Ian.

Re: [PATCH 29 of 29 RFC] libxl: delegate plug/unplug of disk and nic devices to xldeviced

[Roger Pau Monné]

2012/2/9 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 29 of 29 RFC] libxl: delegate plug/u> libxl: delegate plug/unplug of disk and nic devices to xldeviced
>>
>> Change the calls to libxl_device_<type>_<action> to
>> libxl_device_<type>_hotplug_<action> for disk and nic device types.
>> Alsoe enables hotplug script calling from libxl itself, by disabling
>> some udev rules and removing the commented code in
>> libxl__device_hotplug.
>
> Again, I don't think this is the right way to organise the change to
> this behaviour.
>
> The right thing to do is probably to change individual device types
> one at a type,

True, will refactor this.

> and to make the new functionality have the same
> libxl_device_disk_add name (etc.) as before.

I would like to keep the original libxl_device_<type>_add functions,
since they are called from xldeviced and perform the actual plug of
the device. libxl_device_<type>_hotplug_add are called from xl to
trigger the plug (pass the command to xldeviced). Probably the names
should be changed to something more descriptive of what they actually
do, I'm open to suggestion there.

How does renaming libxl_device_<type>_add to
libxl_device_<type>_plug/unplug (which will be called from xldeviced)
and libxl_device_<type>_hotplug_add to libxl_device_<type>_add, this
will mean less modifications in xl code since it will call the same
functions?

Roger.

> Ian.

Re: [PATCH 29 of 29 RFC] libxl: delegate plug/unplug of disk and nic devices to xldeviced

[Ian Jackson]

Roger Pau Monné writes ("Re: [Xen-devel] [PATCH 29 of 29 RFC] libxl: delegate plug/unplug of disk and nic devices to xldeviced"):
> 2012/2/9 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> > and to make the new functionality have the same
> > libxl_device_disk_add name (etc.) as before.
> 
> I would like to keep the original libxl_device_<type>_add functions,
> since they are called from xldeviced and perform the actual plug of
> the device. libxl_device_<type>_hotplug_add are called from xl to
> trigger the plug (pass the command to xldeviced). Probably the names
> should be changed to something more descriptive of what they actually
> do, I'm open to suggestion there.

Something like that.  But if xldeviced is an internal feature of
libxl, rather than a user of libxl's public API (which is how I think
it should be) then all of these functions should be called
libxl__something and not be expored in libxl.h.

> How does renaming libxl_device_<type>_add to
> libxl_device_<type>_plug/unplug (which will be called from xldeviced)
> and libxl_device_<type>_hotplug_add to libxl_device_<type>_add, this
> will mean less modifications in xl code since it will call the same
> functions?

Yes, exactly.  I don't think this move should involve changing xl
(unless of course we discover bugs in the api...)

Ian.

Re: [PATCH 00 of 29 RFC] libxl: move device plug/unplug to a separate daemon

[Roger Pau Monné]

2012/2/2 Roger Pau Monne <roger.pau@entel.upc.edu>:
> This is the first attempt to delegate hotplug device plug/unplug to a
> separate daemon. This will also allow to plug/unplug devices from a
> domain different than Dom0, usually called a "driver domain".
>
> This series contain a mix of bug fixes, mainly for device
> unplug/destroy synchronization, and a set of new functions that
> describe a new protocol to be used when delegating the plug/unplug of
> devices to a daemon, called xldeviced (xl-device-daemon).
>
> As a very important note, I would like to add that this has only been
> tested with PV DomUs, and that qdisk support is missing (xldeviced is
> not prepared to launch a qemu-dm to support qdisk devices). I will

I've added qdisk support to xldeviced, but I will wait a little before
posting an updated version of the series, to have some comments and
address initial issues before sending so much chunk. I will also try
to test this against HVM DomUs the next week.

> work on that, but I think there's already a lot of stuff on this that
> needs proper review.
>
> The series has the following order:
>
>  * patches 1-15: set everything necessary to execute hotplug scripts
>   from libxl directly.
>
>  * patches 16-25: add necessary libxl functions to delegate hotplug
>   addition to a different daemon
>
>  * patches 26-29: introduce xldeviced and start using it by default.
>
> Patch 20 contains a good description of the interaction between xl and
> xldeviced, and the xenstore entries used to accomplish this task.

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