Roger Pau Monne
2013-Apr-17 15:47 UTC
[PATCH v3 0/7] libxl: new hotplug calling convention
This series implements a new hoplug calling convention for libxl. The aim of this new convention is to reduce the blackout phase of migration when using complex hotplug scripts, like iSCSI or other kind of storage backends that might have a non trivial setup time. Major changes from v2 are the addition of HOTPLUG_PATH/libxl_* to the list of reserved entries, and the rebase of the whole series on top of current xen-unstable commit. Thanks for the comments, Roger. Branch available in the git repository at: git://xenbits.xen.org/people/royger/xen.git iscsi_v3 * Acked Roger Pau Monne (7): *libxl: group hotplug related variables libxl: add new hotplug interface support to hotplug script callers libxl: add support for hotplug interface v2 in domain creation/destroy libxl: chain prepare and attach in libxl_device_disk_add libxl: add disk specific remove functions libxl: add local attach support for new hotplug scripts hotplug/Linux: add iscsi block hotplug script docs/misc/libxl-hotplug-interface.txt | 184 +++++++++++++++++++++ tools/hotplug/Linux/Makefile | 1 + tools/hotplug/Linux/block-iscsi | 278 ++++++++++++++++++++++++++++++++ tools/libxl/libxl.c | 286 ++++++++++++++++++++++++++++++--- tools/libxl/libxl_bootloader.c | 1 + tools/libxl/libxl_create.c | 32 ++++- tools/libxl/libxl_device.c | 221 +++++++++++++++++++++++--- tools/libxl/libxl_internal.h | 83 +++++++++- tools/libxl/libxl_linux.c | 73 ++++++--- tools/libxl/libxl_netbsd.c | 4 +- tools/libxl/libxl_types.idl | 1 + tools/libxl/libxl_types_internal.idl | 5 + 12 files changed, 1095 insertions(+), 74 deletions(-) create mode 100644 docs/misc/libxl-hotplug-interface.txt create mode 100644 tools/hotplug/Linux/block-iscsi
Roger Pau Monne
2013-Apr-17 15:47 UTC
[PATCH v3 1/7] libxl: group hotplug related variables
Create a new struct to hold hotplug related variables that are scattered in libxl__ao_device. We will later expand the number of hotplug related variables, so it's best to have them grouped. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> --- Changes since v1: * s/packed/grouped in commit message --- tools/libxl/libxl_device.c | 6 +++--- tools/libxl/libxl_internal.h | 15 ++++++++++----- tools/libxl/libxl_linux.c | 10 +++++----- tools/libxl/libxl_netbsd.c | 4 ++-- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index eeea9d9..044cac5 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -409,7 +409,7 @@ void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev) aodev->ao = ao; aodev->rc = 0; aodev->dev = NULL; - aodev->num_exec = 0; + aodev->hotplug.num_exec = 0; /* Initialize timer for QEMU Bodge and hotplug execution */ libxl__ev_time_init(&aodev->timeout); aodev->active = 1; @@ -892,7 +892,7 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev) * and return the necessary args/env vars for execution */ hotplug = libxl__get_hotplug_script_info(gc, aodev->dev, &args, &env, aodev->action, - aodev->num_exec); + &aodev->hotplug); switch (hotplug) { case 0: /* no hotplug script to execute */ @@ -994,7 +994,7 @@ static void device_hotplug_child_death_cb(libxl__egc *egc, * If no more executions are needed, device_hotplug will call * device_hotplug_done breaking the loop. */ - aodev->num_exec++; + aodev->hotplug.num_exec++; device_hotplug(egc, aodev); return; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 3ba3a21..8507ac3 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1838,6 +1838,7 @@ _hidden const char *libxl__run_dir_path(void); typedef struct libxl__ao_device libxl__ao_device; typedef struct libxl__multidev libxl__multidev; +typedef struct libxl__hotplug libxl__hotplug; typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*); /* This functions sets the necessary libxl__ao_device struct values to use @@ -1856,6 +1857,10 @@ typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*); */ _hidden void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev); +struct libxl__hotplug { + int num_exec; +}; + struct libxl__ao_device { /* filled in by user */ libxl__ao *ao; @@ -1874,7 +1879,7 @@ struct libxl__ao_device { libxl__ev_time timeout; /* device hotplug execution */ const char *what; - int num_exec; + libxl__hotplug hotplug; libxl__ev_child child; }; @@ -2055,18 +2060,18 @@ _hidden void libxl__initiate_device_remove(libxl__egc *egc, * 0: No need to execute hotplug script * 1: Execute hotplug script * - * The last parameter, "num_exec" refeers to the number of times hotplug - * scripts have been called for this device. + * The last parameter, libxl__hotplug contains information related to hotplug + * execution. * * The main body of libxl will, for each device, keep calling * libxl__get_hotplug_script_info, with incrementing values of - * num_exec, and executing the resulting script accordingly, + * hotplug->num_exec, and executing the resulting script accordingly, * until libxl__get_hotplug_script_info returns<=0. */ _hidden int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, char ***args, char ***env, libxl__device_action action, - int num_exec); + libxl__hotplug *hotplug); /*----- local disk attach: attach a disk locally to run the bootloader -----*/ diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c index 37815eb..be76045 100644 --- a/tools/libxl/libxl_linux.c +++ b/tools/libxl/libxl_linux.c @@ -234,7 +234,7 @@ error: int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, char ***args, char ***env, libxl__device_action action, - int num_exec) + libxl__hotplug *hotplug) { char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH); int rc; @@ -247,7 +247,7 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, switch (dev->backend_kind) { case LIBXL__DEVICE_KIND_VBD: - if (num_exec != 0) { + if (hotplug->num_exec != 0) { rc = 0; goto out; } @@ -258,12 +258,12 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, * If domain has a stubdom we don't have to execute hotplug scripts * for emulated interfaces */ - if ((num_exec > 1) || - (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) { + if ((hotplug->num_exec > 1) || + (libxl_get_stubdom_id(CTX, dev->domid) && hotplug->num_exec)) { rc = 0; goto out; } - rc = libxl__hotplug_nic(gc, dev, args, env, action, num_exec); + rc = libxl__hotplug_nic(gc, dev, args, env, action, hotplug->num_exec); break; default: /* No need to execute any hotplug scripts */ diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c index 898e160..48c87eb 100644 --- a/tools/libxl/libxl_netbsd.c +++ b/tools/libxl/libxl_netbsd.c @@ -62,13 +62,13 @@ out: int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, char ***args, char ***env, libxl__device_action action, - int num_exec) + libxl__hotplug *hotplug) { char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH); int rc; /* Check if we have to run hotplug scripts */ - if (!disable_udev || num_exec > 0) { + if (!disable_udev || hotplug->num_exec > 0) { rc = 0; goto out; } -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monne
2013-Apr-17 15:47 UTC
[PATCH v3 2/7] libxl: add new hotplug interface support to hotplug script callers
Add support to call hotplug scripts that use the new hotplug interface to the low-level functions in charge of calling hotplug scripts. libxl__prepare_ao_device sets the hotplug version to 1, in later patches hotplug version will be set to 2 by the caller to use this new hotplug calling convention. Also add a document describing the new interface. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@citrix.com> --- Changes since v2: * Reserve HOTPLUG_PATH/libxl_* for future toolstack entries. * Fix grammar fault in documentation. --- docs/misc/libxl-hotplug-interface.txt | 184 +++++++++++++++++++++++++++++++++ tools/libxl/libxl_device.c | 16 +++- tools/libxl/libxl_internal.h | 20 ++++ tools/libxl/libxl_linux.c | 63 +++++++++--- tools/libxl/libxl_types_internal.idl | 5 + 5 files changed, 271 insertions(+), 17 deletions(-) create mode 100644 docs/misc/libxl-hotplug-interface.txt diff --git a/docs/misc/libxl-hotplug-interface.txt b/docs/misc/libxl-hotplug-interface.txt new file mode 100644 index 0000000..6d67025 --- /dev/null +++ b/docs/misc/libxl-hotplug-interface.txt @@ -0,0 +1,184 @@ + ----------------------- + LIBXL HOTPLUG INTERFACE + ----------------------- + +This document specifies the new libxl hotplug interface. This new +interface has been designed to operate better with complex hotplug +scripts, that need to perform several operations and can take a +considerable time to execute. + +Hotplug scripts are expected to take a parameter, passed by the caller +and provide a block device as output, that will be attached to the guest. + + + +====================+ENVIRONMENT VARIABLES +====================+ + +The following environment variables will be set before calling +the hotplug script. + +The hotplug script will have write permissions on this xenstore paths. + + +HOTPLUG_PATH +------------ + +Points to the xenstore directory that holds information relative +to this hotplug script. At present only one parameter is passed by +the toolstack, the "params" xenstore entry which contains the "target" +line specified in the diskspec xl disk configuration (pdev_path in +libxl_device_disk struct). + +This xenstore directory will be used to communicate between the +hotplug script and the toolstack, and it can also be used by the +hotplug script to store temporary information. This directory is created +before calling the "prepare" operation, and the toolstack guarantees +that it will not be removed before the "unprepare" operation has been +finished. After that, the toolstack will take the appropriate actions +to remove it. The toolstack guarantees that HOTPLUG_PATH will always +point to a valid xenstore path for all operations. + +The path of this directory follows the syntax: + +/local/domain/<local_domid>/libxl/hotplug/vbd/<guest_domid>/<device_id> + +(Note that there is no end slash appended to HOTPLUG_PATH) + +The hotplug script should take special care not to store arbitrary +values in the following entries: + +HOTPLUG_PATH/version +HOTPLUG_PATH/pdev + +This are used for communication between the hotplug script and the +toolstack, and should only be written to when the parameter passed +to the hotplug script requires it. + +Also, the toolstack reserves the right to use any of the following entries, +that should not be used to store arbitrary data from the hotplug script: + +HOTPLUG_PATH/libxl_* + + +BACKEND_PATH +------------ + +Points to the xenstore backend path of the corresponding block device. +Since hotplug scripts are always executed in the Domain that acts as +backend for a device, it will always have the following syntax: + +/local/domain/<local_domain>/backend/vbd/<guest_domid>/<device_id> + +(Note that there is no end slash appended to BACKEND_PATH) + +This environment variable is not set for all operations, since some +hotplug operations are executed before the backend xenstore is set up. + + + +======================+COMMAND LINE PARAMETERS +======================+ + +Script will be called with only one parameter, that is either prepare, +add, remove, unprepare, localattach or localdetach. + +VERSION +------- + +Operation used to request the version this hotplug script supports. Failure +to perform this operation will result in hotplug script detected as version 1. + +BACKEND_PATH: not valid + +Expected output: +HOTPLGU_PATH/version = version supported by the hotplug script + + +PREPARE +------- + +This is the first operation that the hotplug script will be requested to +execute. This operation is executed before the disk is connected, to +give the hotplug script the chance to offload some work from the "add" +operation, that is performed later. + +BACKEND_PATH: not valid + +Expected output: +None + + +ADD +--- + +This operation should connect the device to the domain. Will only be called +after the "prepare" operation has finished successfully. + +BACKEND_PATH: valid + +Expected output: +BACKEND_PATH/physical-device = block device major:minor +BACKEND_PATH/params = block device path +HOTPLUG_PATH/pdev = block device path + + +REMOVE +------ + + +Disconnects a block device from a domain. Will only be called +after the "prepare" operation has finished successfully. Implementations +should take into account that the "remove" operation will also be called if +the "add" operation has failed. + +BACKEND_PATH: valid + +Expected output: +None + + +LOCALATTACH +----------- + + +Creates a block device in the current domain that points to the guest +disk device. Will only be called after the "prepare" operation has +finished successfully. + +BACKEND_PATH: not valid + +Expected output: +HOTPLUG_PATH/pdev = block device path + + +LOCALDETACH +----------- + + +Disconnects a device (previously connected with the localattach +operation) from the current domain. Will only be called after +the "prepare" operation has finished successfully. Implementations +should take into account that the "localdetach" operation will +also be called if the "localattach" operation has failed. + +BACKEND_PATH: not valid + +Expected output: +None + + +UNPREPARE +--------- + + +Performs the necessary actions to unprepare the device. + +BACKEND_PATH: not valid + +Expected output: +None diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index 044cac5..024bc1f 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -83,6 +83,14 @@ out: return rc; } +char *libxl__device_xs_hotplug_path(libxl__gc *gc, libxl__device *dev) +{ + return GCSPRINTF("/local/domain/%u/libxl/hotplug/%u/%s/%u", + dev->backend_domid, dev->domid, + libxl__device_kind_to_string(dev->backend_kind), + dev->devid); +} + int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t, libxl__device *device, char **bents, char **fents) { @@ -410,6 +418,7 @@ void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev) aodev->rc = 0; aodev->dev = NULL; aodev->hotplug.num_exec = 0; + aodev->hotplug.version = 1; /* Initialize timer for QEMU Bodge and hotplug execution */ libxl__ev_time_init(&aodev->timeout); aodev->active = 1; @@ -974,7 +983,12 @@ static void device_hotplug_child_death_cb(libxl__egc *egc, device_hotplug_clean(gc, aodev); - if (status) { + if (status && aodev->action != LIBXL__DEVICE_ACTION_VERSION) { + /* If a hotplug script returns an error when called + * with the "version" argument it means it is using the old + * hotplug calling convention, and we don't have to print + * an error message. + */ libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR, aodev->what, pid, status); hotplug_error = libxl__xs_read(gc, XBT_NULL, diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 8507ac3..aa0102c 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1854,11 +1854,20 @@ typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*); * Once _prepare has been called on a libxl__ao_device, it is safe to just * discard this struct, there's no need to call any destroy function. * _prepare can also be called multiple times with the same libxl__ao_device. + * + * If hotplug.version field is 2, the new hotplug script calling convention + * will be used to call the hotplug script. This new convention provides + * new actions to hotplug scripts, "prepare", "unprepare", "localattach", + * "localdetach" and "version". This new actions have been added to offload + * work done by hotplug scripts during the blackout phase of migration. + * "prepare" is called before the remote domain is paused, so as much + * operations as possible should be done in this phase. */ _hidden void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev); struct libxl__hotplug { int num_exec; + int version; }; struct libxl__ao_device { @@ -1868,6 +1877,8 @@ struct libxl__ao_device { libxl__device *dev; int force; libxl__device_callback *callback; + /* used by the VERISON and LOCALATTACH actions, set by caller */ + libxl_device_disk *disk; /* return value, zeroed by user on entry, is valid on callback */ int rc; /* private for multidev */ @@ -2051,6 +2062,15 @@ _hidden void libxl__initiate_device_remove(libxl__egc *egc, libxl__ao_device *aodev); /* + * libxl__device_xs_hotplug_path returns the xenstore hotplug + * path that is used to share information with the hotplug + * script. + * + * This path is only used by hotplug scripts v2. + */ +_hidden char *libxl__device_xs_hotplug_path(libxl__gc *gc, libxl__device *dev); + +/* * libxl__get_hotplug_script_info returns the args and env that should * be passed to the hotplug script for the requested device. * diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c index be76045..e331ae6 100644 --- a/tools/libxl/libxl_linux.c +++ b/tools/libxl/libxl_linux.c @@ -198,32 +198,63 @@ out: static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev, char ***args, char ***env, - libxl__device_action action) + libxl__device_action action, + int hotplug_version) { char *be_path = libxl__device_backend_path(gc, dev); + char *hotplug_path = libxl__device_xs_hotplug_path(gc, dev); char *script; int nr = 0, rc = 0; - - script = libxl__xs_read(gc, XBT_NULL, - GCSPRINTF("%s/%s", be_path, "script")); - if (!script) { - LOGEV(ERROR, errno, "unable to read script from %s", be_path); - rc = ERROR_FAIL; - goto error; - } - - *env = get_hotplug_env(gc, script, dev); - if (!*env) { + const int env_arraysize = 5; + const int arg_arraysize = 3; + + switch (hotplug_version) { + case 1: + script = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/script", be_path)); + if (!script) { + LOGEV(ERROR, errno, "unable to read script from %s", be_path); + rc = ERROR_FAIL; + goto error; + } + *env = get_hotplug_env(gc, script, dev); + if (!*env) { + rc = ERROR_FAIL; + goto error; + } + break; + case 2: + /* The new hotplug calling convention only uses two ENV variables: + * BACKEND_PATH: path to xenstore backend of the related device. + * HOTPLUG_PATH: path to the xenstore directory that can be used to + * pass extra parameters to the script. + */ + script = libxl__xs_read(gc, XBT_NULL, + GCSPRINTF("%s/script", hotplug_path)); + if (!script) { + LOGEV(ERROR, errno, "unable to read script from %s", hotplug_path); + rc = ERROR_FAIL; + goto error; + } + GCNEW_ARRAY(*env, env_arraysize); + (*env)[nr++] = "BACKEND_PATH"; + (*env)[nr++] = be_path; + (*env)[nr++] = "HOTPLUG_PATH"; + (*env)[nr++] = hotplug_path; + (*env)[nr++] = NULL; + assert(nr == env_arraysize); + nr = 0; + break; + default: + LOG(ERROR, "unknown hotplug script version %d", hotplug_version); rc = ERROR_FAIL; goto error; } - const int arraysize = 3; - GCNEW_ARRAY(*args, arraysize); + GCNEW_ARRAY(*args, arg_arraysize); (*args)[nr++] = script; (*args)[nr++] = (char *) libxl__device_action_to_string(action); (*args)[nr++] = NULL; - assert(nr == arraysize); + assert(nr == arg_arraysize); rc = 1; @@ -251,7 +282,7 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, rc = 0; goto out; } - rc = libxl__hotplug_disk(gc, dev, args, env, action); + rc = libxl__hotplug_disk(gc, dev, args, env, action, hotplug->version); break; case LIBXL__DEVICE_KIND_VIF: /* diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl index cb9444f..253bb18 100644 --- a/tools/libxl/libxl_types_internal.idl +++ b/tools/libxl/libxl_types_internal.idl @@ -37,4 +37,9 @@ libxl__device_console = Struct("device_console", [ libxl__device_action = Enumeration("device_action", [ (1, "ADD"), (2, "REMOVE"), + (3, "PREPARE"), + (4, "UNPREPARE"), + (5, "LOCALATTACH"), + (6, "LOCALDETACH"), + (7, "VERSION"), ]) -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monne
2013-Apr-17 15:47 UTC
[PATCH v3 3/7] libxl: add support for hotplug interface v2 in domain creation/destroy
Add a new variable to libxl_device_disk "hotplug_version", that will be automatically set to the detected hotplug script interface version. This patch adds the basic support to handle this new scripts, by adding the prepare private libxl function, and modifying the current domain creation/destruction to call this functions at the appropriate spots. The hotplug interface detection is done in the _prepare call. This patch also adds some helpers, that will be used here and in later patches. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@citrix.com> --- Changes since v1: * Keep track of hotplug status in xenstore --- tools/libxl/libxl.c | 144 ++++++++++++++++++++++++++++++- tools/libxl/libxl_create.c | 32 +++++++- tools/libxl/libxl_device.c | 199 +++++++++++++++++++++++++++++++++++++---- tools/libxl/libxl_internal.h | 47 ++++++++++ tools/libxl/libxl_types.idl | 1 + 5 files changed, 400 insertions(+), 23 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 03a9782..444e039 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2012,6 +2012,119 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid, return 0; } +/* Callbacks for libxl__device_disk_prepare */ + +static void device_disk_prepare_cb(libxl__egc *, libxl__ao_device *); + +void libxl__device_disk_prepare(libxl__egc *egc, uint32_t domid, + libxl_device_disk *disk, + libxl__ao_device *prepare_aodev) +{ + STATE_AO_GC(prepare_aodev->ao); + char *hotplug_path; + char *script_path; + int rc; + xs_transaction_t t = XBT_NULL; + libxl__disk_prepare *dp; + libxl__ao_device *aodev; + + if (!disk->script) { + /* If script is not set assume v1 */ + disk->hotplug_version = 1; + prepare_aodev->callback(egc, prepare_aodev); + return; + } + + GCNEW(dp); + dp->disk = disk; + /* Save original aodev, since we will call it once hotplug + * script detection has finished + */ + dp->aodev = prepare_aodev; + aodev = &dp->version_aodev; + libxl__prepare_ao_device(ao, aodev); + + GCNEW(aodev->dev); + rc = libxl__device_from_disk(gc, domid, disk, aodev->dev); + if (rc != 0) { + LOG(ERROR, "Invalid or unsupported virtual disk identifier %s", + disk->vdev); + goto error; + } + + script_path = libxl__abs_path(gc, disk->script, + libxl__xen_script_dir_path()); + + hotplug_path = libxl__device_xs_hotplug_path(gc, aodev->dev); + for (;;) { + rc = libxl__xs_transaction_start(gc, &t); + if (rc) goto error; + + rc = libxl__xs_write_checked(gc, t, + GCSPRINTF("%s/params", hotplug_path), + disk->pdev_path); + if (rc) + goto error; + + rc = libxl__xs_write_checked(gc, t, + GCSPRINTF("%s/script", hotplug_path), + script_path); + if (rc) + goto error; + + rc = libxl__xs_transaction_commit(gc, &t); + if (!rc) break; + if (rc < 0) goto error; + } + + aodev->callback = device_disk_prepare_cb; + aodev->action = LIBXL__DEVICE_ACTION_VERSION; + aodev->disk = disk; + aodev->hotplug.version = 2; + libxl__device_hotplug(egc, aodev); + return; + +error: + assert(rc); + libxl__xs_transaction_abort(gc, &t); + prepare_aodev->rc = rc; + prepare_aodev->callback(egc, prepare_aodev); + return; +} + +static void device_disk_prepare_cb(libxl__egc *egc, + libxl__ao_device *version_aodev) +{ + STATE_AO_GC(version_aodev->ao); + libxl__disk_prepare *dp = CONTAINER_OF(version_aodev, *dp, version_aodev); + libxl__ao_device *aodev = dp->aodev; + + if (dp->disk->hotplug_version == 1) { + aodev->callback(egc, aodev); + return; + } + + aodev->action = LIBXL__DEVICE_ACTION_PREPARE; + aodev->hotplug.version = dp->disk->hotplug_version; + aodev->dev = version_aodev->dev; + + /* + * Write status information to xenstore, so we know this device + * has been prepared + */ + aodev->rc = libxl__xs_write_checked(gc, XBT_NULL, + GCSPRINTF("%s/backend_domid", + libxl__device_xs_hotplug_status_path(gc, aodev->dev)), + GCSPRINTF("%u", aodev->dev->backend_domid)); + if (aodev->rc) { + aodev->callback(egc, aodev); + return; + } + + libxl__device_hotplug(egc, aodev); + return; +} + /* Specific function called directly only by local disk attach, * all other users should instead use the regular * libxl__device_disk_add wrapper @@ -2071,8 +2184,15 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, dev = disk->pdev_path; do_backend_phy: - flexarray_append(back, "params"); - flexarray_append(back, dev); + if (disk->hotplug_version == 1) { + /* + * If the new hotplug version is used params is + * stored under a private path, since it can contain + * data that the guest should not see. + */ + flexarray_append(back, "params"); + flexarray_append(back, dev); + } script = libxl__abs_path(gc, disk->script?: "block", libxl__xen_script_dir_path()); @@ -2168,6 +2288,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, aodev->dev = device; aodev->action = LIBXL__DEVICE_ACTION_ADD; + aodev->hotplug.version = disk->hotplug_version; + aodev->disk = disk; libxl__wait_device_connection(egc, aodev); rc = 0; @@ -2261,6 +2383,7 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid, GC_INIT(ctx); char *dompath, *path; int devid = libxl__device_disk_dev_number(vdev, NULL, NULL); + libxl__device dev; int rc = ERROR_FAIL; if (devid < 0) @@ -2279,6 +2402,23 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid, goto out; rc = libxl__device_disk_from_xs_be(gc, path, disk); + if (rc) { + LOG(ERROR, "unable to parse disk device from path %s", path); + goto out; + } + + /* Check if the device is using the new hotplug interface */ + rc = libxl__device_from_disk(gc, domid, disk, &dev); + if (rc) { + LOG(ERROR, "invalid or unsupported virtual disk identifier %s", + disk->vdev); + goto out; + } + disk->hotplug_version = 1; + path = libxl__device_xs_hotplug_path(gc, &dev); + if (libxl__xs_read(gc, XBT_NULL, path)) + disk->hotplug_version = 2; + out: GC_FREE; return rc; diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index ae72f21..708e14e 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -595,6 +595,10 @@ static int store_libxl_entry(libxl__gc *gc, uint32_t domid, */ /* Event callbacks, in this order: */ + +static void domcreate_launch_bootloader(libxl__egc *egc, + libxl__multidev *multidev, + int ret); static void domcreate_devmodel_started(libxl__egc *egc, libxl__dm_spawn_state *dmss, int rc); @@ -641,7 +645,6 @@ static void initiate_domain_create(libxl__egc *egc, /* convenience aliases */ libxl_domain_config *const d_config = dcs->guest_config; - const int restore_fd = dcs->restore_fd; memset(&dcs->build_state, 0, sizeof(dcs->build_state)); domid = 0; @@ -674,6 +677,33 @@ static void initiate_domain_create(libxl__egc *egc, if (ret) goto error_out; } + libxl__multidev_begin(ao, &dcs->multidev); + dcs->multidev.callback = domcreate_launch_bootloader; + libxl__prepare_disks(egc, ao, domid, d_config, &dcs->multidev); + libxl__multidev_prepared(egc, &dcs->multidev, 0); + return; + +error_out: + assert(ret); + domcreate_complete(egc, dcs, ret); +} + +static void domcreate_launch_bootloader(libxl__egc *egc, + libxl__multidev *multidev, + int ret) +{ + libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev); + STATE_AO_GC(dcs->ao); + + /* convenience aliases */ + const int restore_fd = dcs->restore_fd; + libxl_domain_config *const d_config = dcs->guest_config; + + if (ret) { + LOG(ERROR, "unable to prepare devices"); + goto error_out; + } + dcs->bl.ao = ao; libxl_device_disk *bootdisk d_config->num_disks > 0 ? &d_config->disks[0] : NULL; diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index 024bc1f..27a411b 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -58,6 +58,37 @@ int libxl__parse_backend_path(libxl__gc *gc, return libxl__device_kind_from_string(strkind, &dev->backend_kind); } +int libxl__parse_hotplug_status_path(libxl__gc *gc, + const char *path, + libxl__device *dev) +{ + /* /libxl/<domid>/hotplug/status/<kind>/<devid> */ + char strkind[16]; + char *xs_path; + const char *backend_domid; + int rc = sscanf(path, "/libxl/%u/hotplug/status/%15[^/]/%u", + &dev->domid, + strkind, + &dev->devid); + + if (rc != 3) + return ERROR_FAIL; + + xs_path = GCSPRINTF("%s/backend_domid", path); + rc = libxl__xs_read_checked(gc, XBT_NULL, xs_path, &backend_domid); + if (rc) + return rc; + + rc = libxl__device_kind_from_string(strkind, &dev->backend_kind); + if (rc) + return rc; + + dev->backend_domid = atoi(backend_domid); + dev->kind = dev->backend_kind; + + return 0; +} + int libxl__nic_type(libxl__gc *gc, libxl__device *dev, libxl_nic_type *nictype) { char *snictype, *be_path; @@ -91,6 +122,14 @@ char *libxl__device_xs_hotplug_path(libxl__gc *gc, libxl__device *dev) dev->devid); } +char *libxl__device_xs_hotplug_status_path(libxl__gc *gc, libxl__device *dev) +{ + return GCSPRINTF("/libxl/%u/hotplug/status/%s/%u", + dev->domid, + libxl__device_kind_to_string(dev->backend_kind), + dev->devid); +} + int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t, libxl__device *device, char **bents, char **fents) { @@ -500,18 +539,20 @@ void libxl__multidev_prepared(libxl__egc *egc, /******************************************************************************/ -/* Macro for defining the functions that will add a bunch of disks when +/* Macro for defining the functions that will operate a bunch of devices when * inside an async op with multidev. * This macro is added to prevent repetition of code. * * The following functions are defined: + * libxl__prepare_disks * libxl__add_disks * libxl__add_nics * libxl__add_vtpms */ -#define DEFINE_DEVICES_ADD(type) \ - void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid, \ +#define DEFINE_DEVICES_FUNC(type, op) \ + void libxl__##op##_##type##s(libxl__egc *egc, libxl__ao *ao, \ + uint32_t domid, \ libxl_domain_config *d_config, \ libxl__multidev *multidev) \ { \ @@ -519,16 +560,18 @@ void libxl__multidev_prepared(libxl__egc *egc, int i; \ for (i = 0; i < d_config->num_##type##s; i++) { \ libxl__ao_device *aodev = libxl__multidev_prepare(multidev); \ - libxl__device_##type##_add(egc, domid, &d_config->type##s[i], \ + libxl__device_##type##_##op(egc, domid, &d_config->type##s[i], \ aodev); \ } \ } -DEFINE_DEVICES_ADD(disk) -DEFINE_DEVICES_ADD(nic) -DEFINE_DEVICES_ADD(vtpm) +DEFINE_DEVICES_FUNC(disk, add) +DEFINE_DEVICES_FUNC(nic, add) +DEFINE_DEVICES_FUNC(vtpm, add) + +DEFINE_DEVICES_FUNC(disk, prepare) -#undef DEFINE_DEVICES_ADD +#undef DEFINE_DEVICES_FUNC /******************************************************************************/ @@ -536,7 +579,9 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev) { const char *be_path = libxl__device_backend_path(gc, dev); const char *fe_path = libxl__device_frontend_path(gc, dev); + const char *hotplug_path = libxl__device_xs_hotplug_path(gc, dev); const char *tapdisk_path = GCSPRINTF("%s/%s", be_path, "tapdisk-params"); + const char *hotplug_status = libxl__device_xs_hotplug_status_path(gc, dev); const char *tapdisk_params; xs_transaction_t t = 0; int rc; @@ -551,6 +596,8 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev) libxl__xs_path_cleanup(gc, t, fe_path); libxl__xs_path_cleanup(gc, t, be_path); + libxl__xs_path_cleanup(gc, t, hotplug_path); + libxl__xs_path_cleanup(gc, t, hotplug_status); rc = libxl__xs_transaction_commit(gc, &t); if (!rc) break; @@ -578,6 +625,7 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs) char *path; unsigned int num_kinds, num_dev_xsentries; char **kinds = NULL, **devs = NULL; + const char *backend; int i, j, rc = 0; libxl__device *dev; libxl__multidev *multidev = &drs->multidev; @@ -622,6 +670,19 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs) libxl__device_destroy(gc, dev); continue; } + /* + * Check if the device has hotplug entries in xenstore, + * which would mean it's using the new hotplug calling + * convention. + */ + path = libxl__device_xs_hotplug_path(gc, dev); + if (libxl__xs_read(gc, XBT_NULL, path) != NULL) + /* + * This device will be cleaned in the next loop, + * since it's using the new hotplug interface. + */ + continue; + aodev = libxl__multidev_prepare(multidev); aodev->action = LIBXL__DEVICE_ACTION_REMOVE; aodev->dev = dev; @@ -631,6 +692,33 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs) } } + /* Remove devices that are using hotplug interface v2 */ + path = GCSPRINTF("/libxl/%u/hotplug/status/vbd", domid); + devs = libxl__xs_directory(gc, XBT_NULL, path, &num_dev_xsentries); + if (devs) { + for (j = 0; j < num_dev_xsentries; j++) { + path = GCSPRINTF("/libxl/%u/hotplug/status/vbd/%s", domid, devs[j]); + GCNEW(dev); + if (libxl__parse_hotplug_status_path(gc, path, dev) == 0) { + aodev = libxl__multidev_prepare(multidev); + aodev->dev = dev; + aodev->force = drs->force; + aodev->hotplug.version = 2; + /* Check if the device has been connected */ + path = GCSPRINTF("/local/domain/%u/device/vbd/%u/backend", + dev->domid, dev->devid); + rc = libxl__xs_read_checked(gc, XBT_NULL, path, &backend); + if (rc) { + aodev->action = LIBXL__DEVICE_ACTION_UNPREPARE; + libxl__device_hotplug(egc, aodev); + } else { + aodev->action = LIBXL__DEVICE_ACTION_REMOVE; + libxl__initiate_device_remove(egc, aodev); + } + } + } + } + /* console 0 frontend directory is not under /local/domain/<domid>/device */ path = libxl__sprintf(gc, "/local/domain/%d/console/backend", domid); path = libxl__xs_read(gc, XBT_NULL, path); @@ -675,8 +763,6 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds, static void device_backend_cleanup(libxl__gc *gc, libxl__ao_device *aodev); -static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev); - static void device_hotplug_timeout_cb(libxl__egc *egc, libxl__ev_time *ev, const struct timeval *requested_abs); @@ -711,7 +797,7 @@ void libxl__wait_device_connection(libxl__egc *egc, libxl__ao_device *aodev) * If Qemu is running, it will set the state of the device to * 4 directly, without waiting in state 2 for any hotplug execution. */ - device_hotplug(egc, aodev); + libxl__device_hotplug(egc, aodev); return; } @@ -843,7 +929,7 @@ static void device_qemu_timeout(libxl__egc *egc, libxl__ev_time *ev, rc = libxl__xs_write_checked(gc, XBT_NULL, state_path, "6"); if (rc) goto out; - device_hotplug(egc, aodev); + libxl__device_hotplug(egc, aodev); return; out: @@ -873,7 +959,7 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds, goto out; } - device_hotplug(egc, aodev); + libxl__device_hotplug(egc, aodev); return; out: @@ -888,7 +974,7 @@ static void device_backend_cleanup(libxl__gc *gc, libxl__ao_device *aodev) libxl__ev_devstate_cancel(gc, &aodev->backend_ds); } -static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev) +void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aodev) { STATE_AO_GC(aodev->ao); char *be_path = libxl__device_backend_path(gc, aodev->dev); @@ -996,10 +1082,13 @@ static void device_hotplug_child_death_cb(libxl__egc *egc, if (hotplug_error) LOG(ERROR, "script: %s", hotplug_error); aodev->rc = ERROR_FAIL; - if (aodev->action == LIBXL__DEVICE_ACTION_ADD) + if (aodev->action == LIBXL__DEVICE_ACTION_ADD || + aodev->action == LIBXL__DEVICE_ACTION_PREPARE || + aodev->action == LIBXL__DEVICE_ACTION_LOCALATTACH) /* - * Only fail on device connection, on disconnection - * ignore error, and continue with the remove process + * Only fail on device connection, prepare or localattach, + * on other cases ignore error, and continue with the remove + * process. */ goto error; } @@ -1009,7 +1098,7 @@ static void device_hotplug_child_death_cb(libxl__egc *egc, * device_hotplug_done breaking the loop. */ aodev->hotplug.num_exec++; - device_hotplug(egc, aodev); + libxl__device_hotplug(egc, aodev); return; @@ -1021,17 +1110,87 @@ error: static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev) { STATE_AO_GC(aodev->ao); + char *hotplug_path; + const char *pdev, *hotplug_version; int rc; device_hotplug_clean(gc, aodev); - /* Clean xenstore if it's a disconnection */ - if (aodev->action == LIBXL__DEVICE_ACTION_REMOVE) { + switch (aodev->action) { + case LIBXL__DEVICE_ACTION_REMOVE: + switch (aodev->hotplug.version) { + case 1: + /* Clean xenstore if it's a disconnection */ + rc = libxl__device_destroy(gc, aodev->dev); + if (!aodev->rc) + aodev->rc = rc; + break; + case 2: + /* + * Chain unprepare hotplug execution + * after disconnection of device. + */ + aodev->hotplug.num_exec = 0; + aodev->action = LIBXL__DEVICE_ACTION_UNPREPARE; + libxl__device_hotplug(egc, aodev); + return; + default: + LOG(ERROR, "unknown hotplug script version (%d)", + aodev->hotplug.version); + if (!aodev->rc) + aodev->rc = ERROR_FAIL; + break; + } + break; + case LIBXL__DEVICE_ACTION_UNPREPARE: + /* Clean hotplug xenstore path */ rc = libxl__device_destroy(gc, aodev->dev); if (!aodev->rc) aodev->rc = rc; + break; + case LIBXL__DEVICE_ACTION_ADD: + if (aodev->hotplug.version == 1) + goto out; + /* + * Update pdev_path in libxl_device_disk to point to the block + * device + */ + hotplug_path = libxl__device_xs_hotplug_path(gc, aodev->dev); + rc = libxl__xs_read_checked(gc, XBT_NULL, + GCSPRINTF("%s/pdev", hotplug_path), + &pdev); + if (rc || !pdev) { + LOG(ERROR, "unable to get physical disk from %s", + GCSPRINTF("%s/pdev", hotplug_path)); + aodev->rc = rc ? rc : ERROR_FAIL; + goto out; + } + + free(aodev->disk->pdev_path); + aodev->disk->pdev_path = libxl__strdup(NOGC, pdev); + break; + case LIBXL__DEVICE_ACTION_VERSION: + /* Fetch hotplug version output */ + hotplug_path = libxl__device_xs_hotplug_path(gc, aodev->dev); + rc = libxl__xs_read_checked(gc, XBT_NULL, + GCSPRINTF("%s/version", hotplug_path), + &hotplug_version); + if (rc) { + LOG(ERROR, "unable to get hotplug_version from %s", + GCSPRINTF("%s/version", hotplug_path)); + aodev->rc = rc; + goto out; + } + if (!hotplug_version) + aodev->disk->hotplug_version = 1; + else + aodev->disk->hotplug_version = atoi(hotplug_version); + break; + default: + break; } +out: aodev->callback(egc, aodev); return; } diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index aa0102c..00d9232 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -2019,6 +2019,9 @@ struct libxl__multidev { _hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid, libxl_device_disk *disk, libxl__ao_device *aodev); +_hidden void libxl__device_disk_prepare(libxl__egc *egc, uint32_t domid, + libxl_device_disk *disk, + libxl__ao_device *aodev); /* AO operation to connect a nic device */ _hidden void libxl__device_nic_add(libxl__egc *egc, uint32_t domid, @@ -2071,6 +2074,23 @@ _hidden void libxl__initiate_device_remove(libxl__egc *egc, _hidden char *libxl__device_xs_hotplug_path(libxl__gc *gc, libxl__device *dev); /* + * libxl__device_xs_hotplug_status_path returns the xenstore hotplug + * path that is used to store status information about a device that + * is using the v2 hotplug interface. + */ +_hidden char *libxl__device_xs_hotplug_status_path(libxl__gc *gc, + libxl__device *dev); + +/* + * libxl__parse_hotplug_status_path returns a device that is filled + * with the information stored in xenstore. This should only be used + * on devices that use the v2 hotplug interface + */ +_hidden int libxl__parse_hotplug_status_path(libxl__gc *gc, + const char *path, + libxl__device *dev); + +/* * libxl__get_hotplug_script_info returns the args and env that should * be passed to the hotplug script for the requested device. * @@ -2093,6 +2113,28 @@ _hidden int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, libxl__device_action action, libxl__hotplug *hotplug); +/* + * libxl__device_hotplug runs the hotplug script associated + * with the device passed in aodev->dev. + * + * The libxl__ao_device passed to this function should be + * prepared using libxl__prepare_ao_device prior to calling + * this function. + * + * Once finished, aodev->callback will be executed. + */ +_hidden void libxl__device_hotplug(libxl__egc *egc, + libxl__ao_device *aodev); + +/* Internal structure to hold device_disk_prepare specific information */ +typedef struct libxl__disk_prepare libxl__disk_prepare; +struct libxl__disk_prepare { + libxl__ao_device version_aodev; + libxl__ao_device *aodev; + libxl_device_disk *disk; + libxl__device_callback *callback; +}; + /*----- local disk attach: attach a disk locally to run the bootloader -----*/ typedef struct libxl__disk_local_state libxl__disk_local_state; @@ -2466,6 +2508,11 @@ _hidden void libxl__add_disks(libxl__egc *egc, libxl__ao *ao, uint32_t domid, libxl_domain_config *d_config, libxl__multidev *multidev); +_hidden void libxl__prepare_disks(libxl__egc *egc, libxl__ao *ao, + uint32_t domid, + libxl_domain_config *d_config, + libxl__multidev *multidev); + _hidden void libxl__add_nics(libxl__egc *egc, libxl__ao *ao, uint32_t domid, libxl_domain_config *d_config, libxl__multidev *multidev); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index fcb1ecd..a5470f0 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -372,6 +372,7 @@ libxl_device_disk = Struct("device_disk", [ ("removable", integer), ("readwrite", integer), ("is_cdrom", integer), + ("hotplug_version", integer), ]) libxl_device_nic = Struct("device_nic", [ -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monne
2013-Apr-17 15:47 UTC
[PATCH v3 4/7] libxl: chain prepare and attach in libxl_device_disk_add
Chaining the prepare and attach functions in the public libxl_device_disk_add allows libxl to keep the same public API while allowing hotplug scripts v2 to be used. libxl takes care of detecting the hotplug version of the script and executing the necessary actions. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@citrix.com> --- Changes since v1: * Remove public _prepare/_unprepare functions and do all the logic inside libxl_device_disk_add, this prevents an API change. --- tools/libxl/libxl.c | 37 +++++++++++++++++++++++++++++++++---- 1 files changed, 33 insertions(+), 4 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 444e039..32e2696 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2307,6 +2307,39 @@ void libxl__device_disk_add(libxl__egc *egc, uint32_t domid, device_disk_add(egc, domid, disk, aodev, NULL, NULL); } +/* Callbacks for device_disk_add */ + +static void device_disk_add_prepared(libxl__egc *, libxl__ao_device *); + +int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, + libxl_device_disk *disk, + const libxl_asyncop_how *ao_how) +{ + AO_CREATE(ctx, domid, ao_how); + libxl__ao_device *aodev; + + GCNEW(aodev); + libxl__prepare_ao_device(ao, aodev); + aodev->callback = device_disk_add_prepared; + aodev->disk = disk; + libxl__device_disk_prepare(egc, domid, disk, aodev); + + return AO_INPROGRESS; +} + +static void device_disk_add_prepared(libxl__egc *egc, libxl__ao_device *aodev) +{ + if (aodev->rc) { + device_addrm_aocomplete(egc, aodev); + return; + } + + aodev->hotplug.num_exec = 0; + aodev->callback = device_addrm_aocomplete; + libxl__device_disk_add(egc, aodev->dev->domid, aodev->disk, aodev); + return; +} + static int libxl__device_disk_from_xs_be(libxl__gc *gc, const char *be_path, libxl_device_disk *disk) @@ -3558,7 +3591,6 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1) /* Macro for defining device addition functions in a compact way */ /* The following functions are defined: - * libxl_device_disk_add * libxl_device_nic_add * libxl_device_vtpm_add */ @@ -3581,9 +3613,6 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1) /* Define alladd functions and undef the macro */ -/* disk */ -DEFINE_DEVICE_ADD(disk) - /* nic */ DEFINE_DEVICE_ADD(nic) -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monne
2013-Apr-17 15:47 UTC
[PATCH v3 5/7] libxl: add disk specific remove functions
Add a specific macro to generate libxl_device_disk_{remove/destroy} functions that passes the hotplug_version field down to the aodev struct. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@citrix.com> --- tools/libxl/libxl.c | 19 ++++++++++++++++--- 1 files changed, 16 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 32e2696..f9502a8 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -3535,7 +3535,7 @@ out: * libxl_device_vfb_remove * libxl_device_vfb_destroy */ -#define DEFINE_DEVICE_REMOVE(type, removedestroy, f) \ +#define DEFINE_GENERIC_REMOVE(type, removedestroy, f, extra) \ int libxl_device_##type##_##removedestroy(libxl_ctx *ctx, \ uint32_t domid, libxl_device_##type *type, \ const libxl_asyncop_how *ao_how) \ @@ -3555,6 +3555,7 @@ out: aodev->dev = device; \ aodev->callback = device_addrm_aocomplete; \ aodev->force = f; \ + extra; \ libxl__initiate_device_remove(egc, aodev); \ \ out: \ @@ -3562,11 +3563,21 @@ out: return AO_INPROGRESS; \ } +/* Specific for disk devices, that have to set aodev->hotplug_version */ +#define DEFINE_DISK_REMOVE(type, removedestroy, f) \ + DEFINE_GENERIC_REMOVE(type, removedestroy, f, { \ + aodev->hotplug.version = type->hotplug_version; \ + LOG(DEBUG, "hotplug version: %d", aodev->hotplug.version); \ + }) + +#define DEFINE_DEVICE_REMOVE(type, removedestroy, f) \ + DEFINE_GENERIC_REMOVE(type, removedestroy, f, ) + /* Define all remove/destroy functions and undef the macro */ /* disk */ -DEFINE_DEVICE_REMOVE(disk, remove, 0) -DEFINE_DEVICE_REMOVE(disk, destroy, 1) +DEFINE_DISK_REMOVE(disk, remove, 0) +DEFINE_DISK_REMOVE(disk, destroy, 1) /* nic */ DEFINE_DEVICE_REMOVE(nic, remove, 0) @@ -3585,7 +3596,9 @@ DEFINE_DEVICE_REMOVE(vfb, destroy, 1) DEFINE_DEVICE_REMOVE(vtpm, remove, 0) DEFINE_DEVICE_REMOVE(vtpm, destroy, 1) +#undef DEFINE_DISK_REMOVE #undef DEFINE_DEVICE_REMOVE +#undef DEFINE_GENERIC_REMOVE /******************************************************************************/ -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monne
2013-Apr-17 15:47 UTC
[PATCH v3 6/7] libxl: add local attach support for new hotplug scripts
Adds support for locally attaching disks that use the new hotplug script interface, by calling the localattach/localdetach operations and returning a block device that can be used to execute pygrub. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@citrix.com> --- tools/libxl/libxl.c | 86 ++++++++++++++++++++++++++++++++++------ tools/libxl/libxl_bootloader.c | 1 + tools/libxl/libxl_internal.h | 1 + 3 files changed, 76 insertions(+), 12 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index f9502a8..fdf9e16 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2719,6 +2719,7 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc, const libxl_device_disk *in_disk = dls->in_disk; libxl_device_disk *disk = &dls->disk; const char *blkdev_start = dls->blkdev_start; + uint32_t domid = dls->domid; assert(in_disk->pdev_path); @@ -2735,6 +2736,23 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc, case LIBXL_DISK_BACKEND_PHY: LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY disk %s", disk->pdev_path); + if (disk->hotplug_version == 2) { + disk->vdev = libxl__strdup(gc, in_disk->vdev); + libxl__prepare_ao_device(ao, &dls->aodev); + GCNEW(dls->aodev.dev); + rc = libxl__device_from_disk(gc, domid, disk, + dls->aodev.dev); + if (rc != 0) { + LOG(ERROR, "Invalid or unsupported virtual disk " + "identifier %s", disk->vdev); + goto out; + } + dls->aodev.action = LIBXL__DEVICE_ACTION_LOCALATTACH; + dls->aodev.hotplug.version = disk->hotplug_version; + dls->aodev.callback = local_device_attach_cb; + libxl__device_hotplug(egc, &dls->aodev); + return; + } dev = disk->pdev_path; break; case LIBXL_DISK_BACKEND_TAP: @@ -2797,7 +2815,8 @@ static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev) { STATE_AO_GC(aodev->ao); libxl__disk_local_state *dls = CONTAINER_OF(aodev, *dls, aodev); - char *dev = NULL, *be_path = NULL; + char *dev = NULL, *be_path = NULL, *hotplug_path; + const char *pdev = NULL; int rc; libxl__device device; libxl_device_disk *disk = &dls->disk; @@ -2810,20 +2829,45 @@ static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev) aodev->dev->devid); goto out; } + switch (disk->backend) { + case LIBXL_DISK_BACKEND_PHY: + /* Fetch hotplug output to obtain the block device */ + rc = libxl__device_from_disk(gc, dls->domid, disk, &device); + if (rc) + goto out; + hotplug_path = libxl__device_xs_hotplug_path(gc, &device); + rc = libxl__xs_read_checked(gc, XBT_NULL, + GCSPRINTF("%s/pdev", hotplug_path), + &pdev); + if (rc) + goto out; + if (!pdev) { + rc = ERROR_FAIL; + goto out; + } + LOG(DEBUG, "attached local block device %s", pdev); + dls->diskpath = libxl__strdup(gc, pdev); + break; + case LIBXL_DISK_BACKEND_QDISK: + dev = GCSPRINTF("/dev/%s", disk->vdev); + LOG(DEBUG, "locally attaching qdisk %s", dev); - dev = GCSPRINTF("/dev/%s", disk->vdev); - LOG(DEBUG, "locally attaching qdisk %s", dev); + rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, disk, &device); + if (rc < 0) + goto out; + be_path = libxl__device_backend_path(gc, &device); + rc = libxl__wait_for_backend(gc, be_path, "4"); + if (rc < 0) + goto out; - rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, disk, &device); - if (rc < 0) - goto out; - be_path = libxl__device_backend_path(gc, &device); - rc = libxl__wait_for_backend(gc, be_path, "4"); - if (rc < 0) + if (dev != NULL) + dls->diskpath = libxl__strdup(gc, dev); + break; + default: + LOG(ERROR, "invalid disk backend for local attach callback"); + rc = ERROR_FAIL; goto out; - - if (dev != NULL) - dls->diskpath = libxl__strdup(gc, dev); + } dls->callback(egc, dls, 0); return; @@ -2847,11 +2891,29 @@ void libxl__device_disk_local_initiate_detach(libxl__egc *egc, libxl_device_disk *disk = &dls->disk; libxl__device *device; libxl__ao_device *aodev = &dls->aodev; + uint32_t domid = dls->domid; libxl__prepare_ao_device(ao, aodev); if (!dls->diskpath) goto out; switch (disk->backend) { + case LIBXL_DISK_BACKEND_PHY: + if (disk->hotplug_version == 2) { + GCNEW(aodev->dev); + rc = libxl__device_from_disk(gc, domid, disk, + aodev->dev); + if (rc != 0) { + LOG(ERROR, "Invalid or unsupported virtual disk " + "identifier %s", disk->vdev); + goto out; + } + aodev->action = LIBXL__DEVICE_ACTION_LOCALDETACH; + aodev->hotplug.version = disk->hotplug_version; + aodev->callback = local_device_detach_cb; + libxl__device_hotplug(egc, aodev); + return; + } + break; case LIBXL_DISK_BACKEND_QDISK: if (disk->vdev != NULL) { GCNEW(device); diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c index ed12b2c..5d889fe 100644 --- a/tools/libxl/libxl_bootloader.c +++ b/tools/libxl/libxl_bootloader.c @@ -381,6 +381,7 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl) bl->dls.ao = ao; bl->dls.in_disk = bl->disk; bl->dls.blkdev_start = info->blkdev_start; + bl->dls.domid = domid; bl->dls.callback = bootloader_disk_attached_cb; libxl__device_disk_local_initiate_attach(egc, &bl->dls); return; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 00d9232..333a797 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -2152,6 +2152,7 @@ struct libxl__disk_local_state { libxl_device_disk disk; const char *blkdev_start; libxl__disk_local_state_callback *callback; + uint32_t domid; /* filled by libxl__device_disk_local_initiate_attach */ char *diskpath; /* private for implementation of local detach */ -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monne
2013-Apr-17 15:47 UTC
[PATCH v3 7/7] hotplug/Linux: add iscsi block hotplug script
This hotplug script has been tested with IET and NetBSD iSCSI targets, without authentication. Authentication parameters, including the password are passed as parameters to iscsiadm, which is not recommended because other users of the system can see them. This parameters could also be set by editing a corresponding file directly, but the location of this file seems to be different depending on the distribution used. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@citrix.com> --- Changes since v1: * Add -e to script shebang, and use 'set +e' if we know hotplug execution might fail. --- tools/hotplug/Linux/Makefile | 1 + tools/hotplug/Linux/block-iscsi | 278 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 279 insertions(+), 0 deletions(-) create mode 100644 tools/hotplug/Linux/block-iscsi diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile index 0605559..98c7738 100644 --- a/tools/hotplug/Linux/Makefile +++ b/tools/hotplug/Linux/Makefile @@ -21,6 +21,7 @@ XEN_SCRIPTS += blktap XEN_SCRIPTS += xen-hotplug-cleanup XEN_SCRIPTS += external-device-migrate XEN_SCRIPTS += vscsi +XEN_SCRIPTS += block-iscsi XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh XEN_SCRIPT_DATA += block-common.sh diff --git a/tools/hotplug/Linux/block-iscsi b/tools/hotplug/Linux/block-iscsi new file mode 100644 index 0000000..badfa52 --- /dev/null +++ b/tools/hotplug/Linux/block-iscsi @@ -0,0 +1,278 @@ +#!/bin/sh -e +# +# Open-iSCSI Xen block device hotplug script +# +# Author Roger Pau Monné <roger.pau@citrix.com> +# +# 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. + +remove_label() +{ + echo $1 | sed "s/^\("$2"\)//" +} + +check_tools() +{ + if ! type iscsiadm > /dev/null 2>&1; then + echo "Unable to find iscsiadm tool" + return 1 + fi + if [ "$multipath" = "y" ] && ! type multipath > /dev/null 2>&1; then + echo "Unable to find multipath" + return 1 + fi +} + +# Sets the following global variables based on the params field passed in as +# a parameter: iqn, portal, auth_method, user, multipath, password +parse_target() +{ + # set multipath default value + multipath="n" + for param in $(echo "$1" | tr "," "\n") + do + if [ -n "$password" ]; then + password="$password,$param" + continue + fi + case $param in + iqn=*) + iqn=$(remove_label $param "iqn=") + ;; + portal=*) + portal=$(remove_label $param "portal=") + ;; + auth_method=*) + auth_method=$(remove_label $param "auth_method=") + ;; + user=*) + user=$(remove_label $param "user=") + ;; + multipath=*) + multipath=$(remove_label $param "multipath=") + ;; + password=*) + password=$(remove_label $param "password=") + ;; + esac + done + if [ -z "$iqn" ] || [ -z "$portal" ]; then + echo "iqn= and portal= are required parameters" + return 1 + fi + if [ "$multipath" != "y" ] && [ "$multipath" != "n" ]; then + echo "multipath valid values are y and n, $multipath not a valid value" + return 1 + fi + return 0 +} + +# Outputs the block device major:minor +device_major_minor() +{ + stat -L -c %t:%T "$1" +} + +# Sets $dev to point to the device associated with the value in iqn +find_device() +{ + while [ ! -e /dev/disk/by-path/*"$iqn"-lun-0 ]; do + sleep 0.1 + done + set +e + sddev=$(readlink -f /dev/disk/by-path/*"$iqn"-lun-0) + set -e + if [ ! -b "$sddev" ]; then + echo "Unable to find attached device path" + return 1 + fi + if [ "$multipath" = "y" ]; then + mdev=$(multipath -ll "$sddev" | head -1 | awk '{ print $1}') + if [ ! -b /dev/mapper/"$mdev" ]; then + echo "Unable to find attached device multipath" + return 1 + fi + dev="/dev/mapper/$mdev" + else + dev="$sddev" + fi + return 0 +} + +# Attaches the target $iqn in $portal and sets $dev to point to the +# multipath device +attach() +{ + set +e + iscsiadm -m node --targetname "$iqn" -p "$portal" --login > /dev/null 2>&1 + rc=$? + set -e + if [ $rc -ne 0 ]; then + echo "Unable to connect to target" + return 1 + fi + find_device + if [ $? -ne 0 ]; then + echo "Unable to find iSCSI device" + return 1 + fi + return 0 +} + +# Discovers targets in $portal and checks that $iqn is one of those targets +# Also sets the auth parameters to attach the device +prepare() +{ + # Check if target is already opened + set +e + iscsiadm -m session 2>&1 | grep -q "$iqn" + rc=$? + set -e + if [ $rc -eq 0 ]; then + echo "Device already opened" + return 1 + fi + # Discover portal targets + set +e + iscsiadm -m discovery -t st -p $portal 2>&1 | grep -q "$iqn" + rc=$? + set -e + if [ $rc -ne 0 ]; then + echo "No matching target iqn found" + return 1 + fi + # Set auth method if necessary + if [ -n "$auth_method" ] || [ -n "$user" ] || [ -n "$password" ]; then + set +e + iscsiadm -m node --targetname "$iqn" -p "$portal" --op=update --name \ + node.session.auth.authmethod --value="$auth_method" \ + > /dev/null 2>&1 && \ + iscsiadm -m node --targetname "$iqn" -p "$portal" --op=update --name \ + node.session.auth.username --value="$user" \ + > /dev/null 2>&1 && \ + iscsiadm -m node --targetname "$iqn" -p "$portal" --op=update --name \ + node.session.auth.password --value="$password" \ + > /dev/null 2>&1 + rc=$? + set -e + if [ $rc -ne 0 ]; then + echo "Unable to set authentication parameters" + return 1 + fi + fi + return 0 +} + +# Attaches the device and writes xenstore backend entries to connect +# the device +add() +{ + localattach + if [ $? -ne 0 ]; then + echo "Failed to attach device" + return 1 + fi + mm=$(device_major_minor "$dev") + xenstore-write "$BACKEND_PATH/physical-device" "$mm" + xenstore-write "$BACKEND_PATH/params" "$dev" + return 0 +} + +# Disconnects the device +remove() +{ + find_device + if [ $? -ne 0 ]; then + echo "Unable to find device" + return 1 + fi + set +e + iscsiadm -m node --targetname "$iqn" -p "$portal" --logout > /dev/null 2>&1 + rc=$? + set -e + if [ $rc -ne 0 ]; then + echo "Unable to disconnect target" + return 1 + fi + return 0 +} + +# Removes the auth params set by prepare +unprepare() +{ + if [ -n "$auth_method" ] || [ -n "$user" ] || [ -n "$password" ]; then + set +e + iscsiadm -m node --targetname "$iqn" -p "$portal" --op=delete --name \ + node.session.auth.authmethod > /dev/null 2>&1 + iscsiadm -m node --targetname "$iqn" -p "$portal" --op=delete --name \ + node.session.auth.username > /dev/null 2>&1 + iscsiadm -m node --targetname "$iqn" -p "$portal" --op=delete --name \ + node.session.auth.password > /dev/null 2>&1 + set -e + fi +} + +# Attaches the device to the current domain, and writes the resulting +# block device to xenstore +localattach() +{ + attach + if [ $? -ne 0 ]; then + echo "Failed to attach device" + return 1 + fi + xenstore-write "$HOTPLUG_PATH/pdev" "$dev" + return 0 +} + +command=$1 +set +e +target=$(xenstore-read $HOTPLUG_PATH/params) +set -e +if [ -z "$target" ]; then + echo "No information about the target" + exit 1 +fi + +check_tools || exit 1 + +parse_target "$target" + +case $command in +prepare) + prepare + exit $? + ;; +add) + add + exit $? + ;; +remove) + remove + exit $? + ;; +unprepare) + unprepare + exit $? + ;; +localattach) + localattach + exit $? + ;; +localdetach) + remove + exit $? + ;; +version) + xenstore-write "$HOTPLUG_PATH/version" "2" + exit $? + ;; +esac -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
George Dunlap
2013-Apr-18 16:28 UTC
Re: [PATCH v3 0/7] libxl: new hotplug calling convention
On Wed, Apr 17, 2013 at 4:47 PM, Roger Pau Monne <roger.pau@citrix.com> wrote:> This series implements a new hoplug calling convention for libxl. > > The aim of this new convention is to reduce the blackout phase of > migration when using complex hotplug scripts, like iSCSI or other kind > of storage backends that might have a non trivial setup time. > > Major changes from v2 are the addition of HOTPLUG_PATH/libxl_* to the > list of reserved entries, and the rebase of the whole series on top of > current xen-unstable commit. > > Thanks for the comments, Roger.Based on the argument that this will be heavily tested (on every domain creation), and will be fairly easy to revert if it looks like it''s causing trouble, then from a release perspective: Acked-by: George Dunlap <george.dunlap@eu.citrix.com>> > Branch available in the git repository at: > git://xenbits.xen.org/people/royger/xen.git iscsi_v3 > > * Acked > > Roger Pau Monne (7): > *libxl: group hotplug related variables > libxl: add new hotplug interface support to hotplug script callers > libxl: add support for hotplug interface v2 in domain creation/destroy > libxl: chain prepare and attach in libxl_device_disk_add > libxl: add disk specific remove functions > libxl: add local attach support for new hotplug scripts > hotplug/Linux: add iscsi block hotplug script > > docs/misc/libxl-hotplug-interface.txt | 184 +++++++++++++++++++++ > tools/hotplug/Linux/Makefile | 1 + > tools/hotplug/Linux/block-iscsi | 278 ++++++++++++++++++++++++++++++++ > tools/libxl/libxl.c | 286 ++++++++++++++++++++++++++++++--- > tools/libxl/libxl_bootloader.c | 1 + > tools/libxl/libxl_create.c | 32 ++++- > tools/libxl/libxl_device.c | 221 +++++++++++++++++++++++--- > tools/libxl/libxl_internal.h | 83 +++++++++- > tools/libxl/libxl_linux.c | 73 ++++++--- > tools/libxl/libxl_netbsd.c | 4 +- > tools/libxl/libxl_types.idl | 1 + > tools/libxl/libxl_types_internal.idl | 5 + > 12 files changed, 1095 insertions(+), 74 deletions(-) > create mode 100644 docs/misc/libxl-hotplug-interface.txt > create mode 100644 tools/hotplug/Linux/block-iscsi > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Ian Jackson
2013-Apr-18 17:30 UTC
Re: [PATCH v3 3/7] libxl: add support for hotplug interface v2 in domain creation/destroy
Roger Pau Monne writes ("[PATCH v3 3/7] libxl: add support for hotplug interface v2 in domain creation/destroy"):> Add a new variable to libxl_device_disk "hotplug_version", that will > be automatically set to the detected hotplug script interface version.Why does this need to be in the public interface ? Is it just there for reporting ? The rest of this looks OK assuming that this is roughly the right approach, but see my previous mail. Ian.