Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 00 of 29 RFC] libxl: move device plug/unplug to a separate daemon
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.
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 01 of 29 RFC] libxl: Atomicaly check backend state and set it to 5 at device_remove
# 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)) {
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 02 of 29 RFC] hotplug/block: get the type of block device from file path (NetBSD)
# 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
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 03 of 29 RFC] libxl: allow libxl__exec to take a parameter containing the env variables
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 04 of 29 RFC] libxl: add libxl__forkexec function to libxl_exec
# 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,
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 05 of 29 RFC] libxl: wait for devices to initialize upon addition to the domain
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 06 of 29 RFC] hotplug NetBSD: pass an action instead of a state to hotplug scripts
# 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:
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 07 of 29 RFC] libxl: perform xenstore device cleanup from libxl
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 08 of 29 RFC] libxl: remove force parameter from libxl__device_remove
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 09 of 29 RFC] libxl: add better error checking on libxl__device_remove
# 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:
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 10 of 29 RFC] libxl: destroy devices before device model
# 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)
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 11 of 29 RFC] libxl: execute hotplug scripts directly from libxl
# 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; +}
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 12 of 29 RFC] libxl: add hotplug script calling for NetBSD
# 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; }
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 13 of 29 RFC] libxl: add hotplug script calls for Linux
# 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; }
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 14 of 29 RFC] rc.d NetBSD: don''t start xenbackendd by default, only when xend needs it
# 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
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 15 of 29 RFC] NetBSD/xencommons: remove xend precmd folder creation
# 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"
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 16 of 29 RFC] libxl: introduce libxl__device_hotplug_path
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 17 of 29 RFC] libxl: add enum with possible hotplug state
# 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
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 18 of 29 RFC] libxl: introduce libxl__device_generic_hotplug_add
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 19 of 29 RFC] libxl: add libxl__device_hotplug_disconnect
# 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
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
# 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,
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 21 of 29 RFC] libxl: add libxl__parse_hotplug_path
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 22 of 29 RFC] libxl: add libxl__parse_disk_hotplug_path
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 23 of 29 RFC] libxl: add libxl__parse_nic_hotplug_path
# 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);
Roger Pau Monne
2012-Feb-02 13:26 UTC
[PATCH 24 of 29 RFC] libxl: add libxl_setup_hotplug_listener
# 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. */
Roger Pau Monne
2012-Feb-02 13:27 UTC
[PATCH 25 of 29 RFC] libxl: add libxl_hotplug_dispatch
# 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
Roger Pau Monne
2012-Feb-02 13:27 UTC
[PATCH 26 of 29 RFC] xldeviced: new daemon to execute hotplug scripts
# 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
Roger Pau Monne
2012-Feb-02 13:27 UTC
[PATCH 27 of 29 RFC] init: updated Linux and NetBSD init scripts to launch xldeviced
# 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
Roger Pau Monne
2012-Feb-02 13:27 UTC
[PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev
# 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)
Roger Pau Monne
2012-Feb-02 13:27 UTC
[PATCH 29 of 29 RFC] libxl: delegate plug/unplug of disk and nic devices to xldeviced
# 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;
Stefano Stabellini
2012-Feb-02 18:37 UTC
Re: [PATCH 25 of 29 RFC] libxl: add libxl_hotplug_dispatch
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.
Roger Pau Monné
2012-Feb-02 18:45 UTC
Re: [PATCH 25 of 29 RFC] libxl: add libxl_hotplug_dispatch
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
Roger Pau Monné
2012-Feb-03 14:06 UTC
Re: [PATCH 00 of 29 RFC] libxl: move device plug/unplug to a separate daemon
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 willI'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
Shriram Rajagopalan
2012-Feb-07 07:02 UTC
Re: [PATCH 13 of 29 RFC] libxl: add hotplug script calls for Linux
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 >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monné
2012-Feb-07 09:04 UTC
Re: [PATCH 13 of 29 RFC] libxl: add hotplug script calls for Linux
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
Ian Jackson
2012-Feb-08 16:42 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Ian Jackson
2012-Feb-08 16:42 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Ian Jackson
2012-Feb-08 16:43 UTC
Re: [PATCH 23 of 29 RFC] libxl: add libxl__parse_nic_hotplug_path
Roger Pau Monne writes ("[Xen-devel] [PATCH 23 of 29 RFC] libxl: add libxl__parse_nic_hotplug_path"):> libxl: add libxl__parse_nic_hotplug_pathThis 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.
Stefano Stabellini
2012-Feb-09 10:02 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Stefano Stabellini
2012-Feb-09 10:03 UTC
Re: [PATCH 25 of 29 RFC] libxl: add libxl_hotplug_dispatch
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. --8323329-394858887-1328781850=:3196 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --8323329-394858887-1328781850=:3196--
Ian Jackson
2012-Feb-09 15:22 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Stefano Stabellini
2012-Feb-09 15:32 UTC
Re: [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!!> 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.
Ian Jackson
2012-Feb-09 15:33 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Ian Jackson
2012-Feb-09 15:34 UTC
Re: [PATCH 15 of 29 RFC] NetBSD/xencommons: remove xend precmd folder creation
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.
Ian Jackson
2012-Feb-09 15:40 UTC
Re: [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev
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.
Ian Jackson
2012-Feb-09 15:41 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Ian Jackson
2012-Feb-09 15:43 UTC
Re: [PATCH 29 of 29 RFC] libxl: delegate plug/unplug of disk and nic devices to xldeviced
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.
Stefano Stabellini
2012-Feb-09 15:43 UTC
Re: [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"): > > 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 meansee 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.
Ian Campbell
2012-Feb-09 15:49 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Stefano Stabellini
2012-Feb-09 16:00 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Ian Campbell
2012-Feb-09 16:01 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Stefano Stabellini
2012-Feb-09 16:18 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Ian Campbell
2012-Feb-09 16:40 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Stefano Stabellini
2012-Feb-09 17:28 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Roger Pau Monné
2012-Feb-14 13:36 UTC
Re: [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev
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.
Roger Pau Monné
2012-Feb-14 14:05 UTC
Re: [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>:> 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.
Roger Pau Monné
2012-Feb-14 14:23 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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
Roger Pau Monné
2012-Feb-14 14:25 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Roger Pau Monné
2012-Feb-14 14:38 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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. >
Ian Campbell
2012-Feb-14 15:48 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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
Ian Jackson
2012-Feb-15 17:18 UTC
Re: [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev
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.
Roger Pau Monné
2012-Feb-16 08:53 UTC
Re: [PATCH 28 of 29 RFC] libxl: add libxl__find_free_vdev
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
Ian Jackson
2012-Feb-17 15:45 UTC
Re: [PATCH 29 of 29 RFC] libxl: delegate plug/unplug of disk and nic devices to xldeviced
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.
Ian Jackson
2012-Feb-20 18:52 UTC
Re: [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"):> 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.
Ian Jackson
2012-Feb-20 18:55 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Stefano Stabellini
2012-Feb-21 10:38 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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. --8323329-679958368-1329820706=:23091 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xensource.com/xen-devel --8323329-679958368-1329820706=:23091--
Ian Campbell
2012-Feb-21 11:04 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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
Ian Jackson
2012-Feb-21 16:42 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.
Ian Campbell
2012-Feb-21 17:02 UTC
Re: [PATCH 20 of 29 RFC] libxl: introduce libxl hotplug public API functions
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.