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.
Thanks for the comments, Roger.
Branch available in the git repository at:
git://xenbits.xen.org/people/royger/xen.git iscsi_v2
* 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 | 179 +++++++++++++++++++
tools/hotplug/Linux/Makefile | 1 +
tools/hotplug/Linux/block-iscsi | 278 ++++++++++++++++++++++++++++++
tools/libxl/libxl.c | 302 ++++++++++++++++++++++++++++++---
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 | 71 ++++++---
tools/libxl/libxl_netbsd.c | 4 +-
tools/libxl/libxl_types.idl | 1 +
tools/libxl/libxl_types_internal.idl | 5 +
12 files changed, 1106 insertions(+), 72 deletions(-)
create mode 100644 docs/misc/libxl-hotplug-interface.txt
create mode 100644 tools/hotplug/Linux/block-iscsi
Roger Pau Monne
2013-Mar-18 11:47 UTC
[PATCH v2 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 8be086d..bc468a6 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1837,6 +1837,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
@@ -1855,6 +1856,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;
@@ -1873,7 +1878,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;
};
@@ -2054,18 +2059,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 d34fbb3..d4ee58a 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -227,7 +227,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;
@@ -240,7 +240,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;
}
@@ -251,12 +251,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-Mar-18 11:47 UTC
[PATCH v2 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>
---
docs/misc/libxl-hotplug-interface.txt | 179 +++++++++++++++++++++++++++++++++
tools/libxl/libxl_device.c | 16 +++-
tools/libxl/libxl_internal.h | 20 ++++
tools/libxl/libxl_linux.c | 61 +++++++++---
tools/libxl/libxl_types_internal.idl | 5 +
5 files changed, 265 insertions(+), 16 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..357f42d
--- /dev/null
+++ b/docs/misc/libxl-hotplug-interface.txt
@@ -0,0 +1,179 @@
+ -----------------------
+ 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 in not storing 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.
+
+
+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 bc468a6..fe0b20d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1853,11 +1853,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 {
@@ -1867,6 +1876,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 */
@@ -2050,6 +2061,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 d4ee58a..896fa9f 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -191,32 +191,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;
+ const int env_arraysize = 5;
+ const int arg_arraysize = 3;
- 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) {
+ 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;
@@ -244,7 +275,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-Mar-18 11:47 UTC
[PATCH v2 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 a09c0fa..e6d4a98 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2011,6 +2011,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
@@ -2070,8 +2183,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());
@@ -2167,6 +2287,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;
@@ -2260,6 +2382,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)
@@ -2278,6 +2401,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 2ea628a..ad5f8c4 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -592,6 +592,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);
@@ -638,7 +642,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;
@@ -671,6 +674,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 fe0b20d..d957ed2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2018,6 +2018,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,
@@ -2070,6 +2073,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.
*
@@ -2092,6 +2112,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;
@@ -2465,6 +2507,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 5b080ed..ac8361b 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -370,6 +370,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-Mar-18 11:47 UTC
[PATCH v2 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 e6d4a98..b15cb96 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2306,6 +2306,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)
@@ -3553,7 +3586,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
*/
@@ -3576,9 +3608,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-Mar-18 11:47 UTC
[PATCH v2 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 | 35 +++++++++++++++++++++++++++++++++--
1 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b15cb96..987723b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3557,11 +3557,41 @@ out:
return AO_INPROGRESS; \
}
+/* Specific for disk devices, that have to set aodev->hotplug_version */
+#define DEFINE_DISK_REMOVE(type, removedestroy, f) \
+ int libxl_device_##type##_##removedestroy(libxl_ctx *ctx, \
+ uint32_t domid, libxl_device_##type *type, \
+ const libxl_asyncop_how *ao_how) \
+ { \
+ AO_CREATE(ctx, domid, ao_how); \
+ libxl__device *device; \
+ libxl__ao_device *aodev; \
+ int rc; \
+ \
+ GCNEW(device); \
+ rc = libxl__device_from_##type(gc, domid, type, device); \
+ if (rc != 0) goto out; \
+ \
+ GCNEW(aodev); \
+ libxl__prepare_ao_device(ao, aodev); \
+ aodev->action = LIBXL__DEVICE_ACTION_REMOVE; \
+ aodev->dev = device; \
+ aodev->callback = device_addrm_aocomplete; \
+ aodev->force = f; \
+ aodev->hotplug.version = type->hotplug_version; \
+ LOG(DEBUG, "hotplug version: %d", aodev->hotplug.version);
\
+ libxl__initiate_device_remove(egc, aodev); \
+ \
+ out: \
+ if (rc) return AO_ABORT(rc); \
+ return AO_INPROGRESS; \
+ }
+
/* 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)
@@ -3580,6 +3610,7 @@ 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
/******************************************************************************/
--
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-Mar-18 11:47 UTC
[PATCH v2 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 987723b..9d0d15a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2718,6 +2718,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);
@@ -2734,6 +2735,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:
@@ -2796,7 +2814,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;
@@ -2809,20 +2828,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;
@@ -2846,11 +2890,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 d957ed2..3228ab2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2151,6 +2151,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-Mar-18 11:47 UTC
[PATCH v2 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
Ian Jackson
2013-Apr-11 16:25 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers
Roger Pau Monne writes ("[Xen-devel] [PATCH v2 2/7] libxl: add new hotplug
interface support to hotplug script callers"):> +The hotplug script should take special care in not storing arbitrary
> +values in the following entries:
> +
> +HOTPLUG_PATH/version
> +HOTPLUG_PATH/pdev
Do we want to allow any scope for introducing new toolstack-reserved
entries later ?
"... take special care not to store ..." would be correct grammar.
Thanks,
Ian.
Ian Jackson
2013-Apr-11 16:33 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers
Roger Pau Monne writes ("[Xen-devel] [PATCH v2 2/7] libxl: add new hotplug
interface support to hotplug script callers"):> +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.
> +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.
But in fact in your implementation this is only called right before
add, isn''t it ?
Also, IIRC part of the point of this new interface is that for target
devices which can''t be accessed simultaneously from different hosts,
during migration you''d be able to do costly setup during the initial
part of the migration and merely "activate" the devices later.
Am I missing something ?
Thanks,
Ian.
Roger Pau Monné
2013-Apr-12 13:51 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers
On 11/04/13 18:33, Ian Jackson wrote:> Roger Pau Monne writes ("[Xen-devel] [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers"): >> +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. > >> +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. > > But in fact in your implementation this is only called right before > add, isn''t it ?It depends on the caller, for example if using libxl_device_disk_add, libxl will take care of calling prepare before add (so external API is not changed), on the other hand, when creating a new domain, the prepare functions will be called much before calling add, so we can prepare the device before the domain is paused when doing migration. If you take a look at patch 3/7, you will see that on domain create disks are prepared at initiate_domain_create.> > Also, IIRC part of the point of this new interface is that for target > devices which can''t be accessed simultaneously from different hosts, > during migration you''d be able to do costly setup during the initial > part of the migration and merely "activate" the devices later.Yes, that''s right, prepare will be called while the domain is still running on the sender side, so we can offload as much work as possible to this phase.> Am I missing something ? > > Thanks, > Ian. >
Roger Pau Monné
2013-Apr-16 11:30 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers
On 11/04/13 18:25, Ian Jackson wrote:> Roger Pau Monne writes ("[Xen-devel] [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers"): >> +The hotplug script should take special care in not storing arbitrary >> +values in the following entries: >> + >> +HOTPLUG_PATH/version >> +HOTPLUG_PATH/pdev > > Do we want to allow any scope for introducing new toolstack-reserved > entries later ?Would you agree to say that the script is not able to use entries beginning with libxl_*, so we can reserve that for later use? Also, if you could take a look at the rest of the series, I would like to try to respin the series before the end of the week.> "... take special care not to store ..." would be correct grammar. > > Thanks, > Ian. >
Ian Campbell
2013-Apr-16 12:06 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers
On Tue, 2013-04-16 at 12:30 +0100, Roger Pau Monne wrote:> On 11/04/13 18:25, Ian Jackson wrote: > > Roger Pau Monne writes ("[Xen-devel] [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers"): > >> +The hotplug script should take special care in not storing arbitrary > >> +values in the following entries: > >> + > >> +HOTPLUG_PATH/version > >> +HOTPLUG_PATH/pdev > > > > Do we want to allow any scope for introducing new toolstack-reserved > > entries later ? > > Would you agree to say that the script is not able to use entries > beginning with libxl_*, so we can reserve that for later use?Doesn''t the toolstack have its own prefix, under which it could duplicate HOTPLUG_PATH if it wanted? Or maybe there is more benefit to having all the hotplug info about a device in the same place to having things be nicely separated?> > Also, if you could take a look at the rest of the series, I would like > to try to respin the series before the end of the week. > > > "... take special care not to store ..." would be correct grammar. > > > > Thanks, > > Ian. > > >
Roger Pau Monné
2013-Apr-16 14:42 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers
On 16/04/13 14:06, Ian Campbell wrote:> On Tue, 2013-04-16 at 12:30 +0100, Roger Pau Monne wrote: >> On 11/04/13 18:25, Ian Jackson wrote: >>> Roger Pau Monne writes ("[Xen-devel] [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers"): >>>> +The hotplug script should take special care in not storing arbitrary >>>> +values in the following entries: >>>> + >>>> +HOTPLUG_PATH/version >>>> +HOTPLUG_PATH/pdev >>> >>> Do we want to allow any scope for introducing new toolstack-reserved >>> entries later ? >> >> Would you agree to say that the script is not able to use entries >> beginning with libxl_*, so we can reserve that for later use? > > Doesn''t the toolstack have its own prefix, under which it could > duplicate HOTPLUG_PATH if it wanted? > > Or maybe there is more benefit to having all the hotplug info about a > device in the same place to having things be nicely separated?One solution would be to say that the hotplug script can only write to the following entries: HOTPLUG_PATH/version HOTPLUG_PATH/pdev And then provide the script with another path that could be used to store whatever private entries the hotplug script needs: PRIVATE_PATH This would solve the problem of adding new toolstack-reserved entries later in HOTPLUG_PATH. The private path could for example be: /local/domain/<local_domid>/libxl/private/vbd/<guest_domid>/<device_id> Which is basically the same as HOTPLUG_PATH replacing "hotplug" with "private".>> >> Also, if you could take a look at the rest of the series, I would like >> to try to respin the series before the end of the week. >> >>> "... take special care not to store ..." would be correct grammar. >>> >>> Thanks, >>> Ian. >>> >> > >
Ian Jackson
2013-Apr-16 15:04 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH v2 2/7] libxl: add new
hotplug interface support to hotplug script
callers"):> On 11/04/13 18:25, Ian Jackson wrote:
> > Do we want to allow any scope for introducing new toolstack-reserved
> > entries later ?
>
> Would you agree to say that the script is not able to use entries
> beginning with libxl_*, so we can reserve that for later use?
Yes.
> Also, if you could take a look at the rest of the series, I would like
> to try to respin the series before the end of the week.
OK.
Ian.
Ian Jackson
2013-Apr-18 17:23 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers [and 1 more messages]
Ian Jackson writes ("Re: [Xen-devel] [PATCH v2 2/7] libxl: add new hotplug
interface support to hotplug script callers"):> Roger Pau Monne writes ("[Xen-devel] [PATCH v2 2/7] libxl: add new
hotplug interface support to hotplug script callers"):
...> > +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.
>
> But in fact in your implementation this is only called right before
> add, isn''t it ?
>
> Also, IIRC part of the point of this new interface is that for target
> devices which can''t be accessed simultaneously from different
hosts,
> during migration you''d be able to do costly setup during the
initial
> part of the migration and merely "activate" the devices later.
>
> Am I missing something ?
This question needs to be answered by the documentation, I think.
What are the guarantees that the hotplug scripts rely on ?
AFAICT what happens after your series is applied is that we always
call prepare just before add, and unprepare just after remove (if I
may neglect local attach for now). So I don''t see why these new
functions are necessary to your new scripts.
Ian.
Roger Pau Monné
2013-Apr-18 17:40 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers [and 1 more messages]
On 18/04/13 19:23, Ian Jackson wrote:> Ian Jackson writes ("Re: [Xen-devel] [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers"): >> Roger Pau Monne writes ("[Xen-devel] [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers"): > ... >>> +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. >> >> But in fact in your implementation this is only called right before >> add, isn''t it ? >> >> Also, IIRC part of the point of this new interface is that for target >> devices which can''t be accessed simultaneously from different hosts, >> during migration you''d be able to do costly setup during the initial >> part of the migration and merely "activate" the devices later. >> >> Am I missing something ? > > This question needs to be answered by the documentation, I think. > > What are the guarantees that the hotplug scripts rely on ? > > AFAICT what happens after your series is applied is that we always > call prepare just before add, and unprepare just after remove (if I > may neglect local attach for now). So I don''t see why these new > functions are necessary to your new scripts.Do you mean that both prepare and add are called after the domain on the sender side is paused? Disks are prepared on "initiate_domain_create", which is the first function that gets called on domain creation, on the other hand they are added much later.
Ian Jackson
2013-Apr-18 17:52 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers [and 1 more messages]
Roger Pau Monné writes ("Re: [Xen-devel] [PATCH v2 2/7] libxl: add new
hotplug interface support to hotplug script callers [and 1 more
messages]"):> On 18/04/13 19:23, Ian Jackson wrote:
> > AFAICT what happens after your series is applied is that we always
> > call prepare just before add, and unprepare just after remove (if I
> > may neglect local attach for now). So I don''t see why these
new
> > functions are necessary to your new scripts.
>
> Do you mean that both prepare and add are called after the domain on the
> sender side is paused?
I don''t know exactly what I mean :-).
> Disks are prepared on "initiate_domain_create", which is the
first
> function that gets called on domain creation, on the other hand they are
> added much later.
You can''t write that in the documentation because it''s in
terms of
the internal workings of libxl.
What is the purpose of the distinction between prepare and add ?
Ian.
Roger Pau Monné
2013-Apr-19 08:10 UTC
Re: [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers [and 1 more messages]
On 18/04/13 19:52, Ian Jackson wrote:> Roger Pau Monné writes ("Re: [Xen-devel] [PATCH v2 2/7] libxl: add new hotplug interface support to hotplug script callers [and 1 more messages]"): >> On 18/04/13 19:23, Ian Jackson wrote: >>> AFAICT what happens after your series is applied is that we always >>> call prepare just before add, and unprepare just after remove (if I >>> may neglect local attach for now). So I don''t see why these new >>> functions are necessary to your new scripts. >> >> Do you mean that both prepare and add are called after the domain on the >> sender side is paused? > > I don''t know exactly what I mean :-). > >> Disks are prepared on "initiate_domain_create", which is the first >> function that gets called on domain creation, on the other hand they are >> added much later. > > You can''t write that in the documentation because it''s in terms of > the internal workings of libxl. > > What is the purpose of the distinction between prepare and add ?The main purpose of this distinction is that during migration the "prepare" action is called before the guest on the sender side is paused, so it allows to offload work from the "add" action, thus reducing the blackout phase during migration. I guess you want me to add this to the Documentation? Thanks, Roger.