* [PATCH 0/13] execute hotplug scripts from libxl
@ 2012-05-16 16:11 Roger Pau Monne
2012-05-16 16:11 ` [PATCH 01/13] libxl: pass env vars to libxl__exec Roger Pau Monne
` (12 more replies)
0 siblings, 13 replies; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel
This series have been splitted in several patches, to make them easier
to review. Also the amount of changes introduced is quite important,
since apart from all the hotplug necessary functions and
modifications, libxl_domain_destroy has been converted to an async op.
This was necessary in order to have async operations during device
removal.
Also, as an important change, disk and nics are added at different
points for HVM and device model based guests, since we need the disk
in order to start Qemu, but the nic hotplug scripts should be called
at a later point, when Qemu has created the corresponding tap device.
[PATCH 01/13] libxl: pass env vars to libxl__exec
[PATCH 02/13] libxl: fix libxl__xs_directory usage of transaction
[PATCH 03/13] libxl: add libxl__xs_path_cleanup
[PATCH 04/13] libxl: move libxl_device_disk_add to libxl_device
[PATCH 05/13] libxl: move libxl_device_nic_add to libxl_device
[PATCH 06/13] libxl: cleanup libxl__device_{disk,nic}_add
[PATCH 10/13] libxl: add option to choose who executes hotplug
[PATCH 11/13] libxl: set nic type to VIF by default
All the changes above are bug fixes, code motion or helper functions,
that will be used by the other patches in the list.
[PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op
[PATCH 08/13] libxl: convert libxl_device_disk_add to an async
[PATCH 09/13] libxl: convert libxl_device_nic_add to an async
[PATCH 12/13] libxl: call hotplug scripts for disk devices from
[PATCH 13/13] libxl: call hotplug scripts for nic devices from libxl
This patches contain the meat, specially 07/13. The other ones become
quite trivial once 07/13 is applied.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/13] libxl: pass env vars to libxl__exec
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:02 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 02/13] libxl: fix libxl__xs_directory usage of transaction Roger Pau Monne
` (11 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Add another parameter to libxl__exec call that contains the
environment variables to use when performing the execvp call.
Changes since v4:
* Unify error checking.
Changes since v3:
* Use CTX instead of defining libxl_ctx *ctx.
* Error checking on setenv.
* Better comment on header for libxl__exec function.
* Added some const-correctness.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl.c | 2 +-
tools/libxl/libxl_bootloader.c | 4 ++--
tools/libxl/libxl_dm.c | 2 +-
tools/libxl/libxl_exec.c | 14 ++++++++++++--
tools/libxl/libxl_internal.h | 20 ++++++++++++++++++--
5 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4d01cf8..2a09192 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1261,7 +1261,7 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass)
args[2] = "-autopass";
}
- libxl__exec(autopass_fd, -1, -1, args[0], args);
+ libxl__exec(gc, autopass_fd, -1, -1, args[0], args, NULL);
abort();
x_fail:
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index fb1302b..0021a7b 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -359,6 +359,7 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
libxl__bootloader_state *bl = CONTAINER_OF(op, *bl, openpty);
STATE_AO_GC(bl->ao);
int rc, r;
+ char *const env[] = { "TERM", "vt100", NULL };
if (bl->openpty.rc) {
rc = bl->openpty.rc;
@@ -452,8 +453,7 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
/* child */
r = login_tty(libxl__carefd_fd(bl->ptys[0].slave));
if (r) { LOGE(ERROR, "login_tty failed"); exit(-1); }
- setenv("TERM", "vt100", 1);
- libxl__exec(-1, -1, -1, bl->args[0], (char**)bl->args);
+ libxl__exec(gc, -1, -1, -1, bl->args[0], (char **) bl->args, env);
exit(-1);
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4ad7d02..95fd0ac 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1015,7 +1015,7 @@ retry_transaction:
goto out_close;
if (!rc) { /* inner child */
setsid();
- libxl__exec(null, logfile_w, logfile_w, dm, args);
+ libxl__exec(gc, null, logfile_w, logfile_w, dm, args, NULL);
}
rc = 0;
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index b46b893..082bf44 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -66,8 +66,8 @@ static void check_open_fds(const char *what)
if (badness) abort();
}
-void libxl__exec(int stdinfd, int stdoutfd, int stderrfd, const char *arg0,
- char **args)
+void libxl__exec(libxl__gc *gc, int stdinfd, int stdoutfd, int stderrfd,
+ const char *arg0, char *const args[], char *const env[])
/* call this in the child */
{
if (stdinfd != -1)
@@ -90,8 +90,18 @@ void libxl__exec(int stdinfd, int stdoutfd, int stderrfd, const char *arg0,
/* in case our caller set it to IGN. subprocesses are entitled
* to assume they got DFL. */
+ if (env != NULL) {
+ for (int i = 0; env[i] != NULL && env[i+1] != NULL; i += 2) {
+ if (setenv(env[i], env[i+1], 1) < 0) {
+ LOGEV(ERROR, errno, "setting env vars (%s = %s)",
+ env[i], env[i+1]);
+ goto out;
+ }
+ }
+ }
execvp(arg0, args);
+out:
fprintf(stderr, "libxl: cannot execute %s: %s\n", arg0, strerror(errno));
_exit(-1);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f71e76a..aaaf644 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1138,8 +1138,24 @@ _hidden int libxl__wait_for_offspring(libxl__gc *gc,
/* low-level stuff, for synchronous subprocesses etc. */
-_hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
- const char *arg0, char **args); // logs errors, never returns
+/*
+ * env should be passed using the following format,
+ *
+ * env[0]: name of env variable
+ * env[1]: value of env variable
+ * env[n]: ...
+ *
+ * So it efectively becomes something like:
+ * export env[n]=env[n+1]
+ * (where n%2 = 0)
+ *
+ * The last entry of the array always has to be NULL.
+ *
+ * Logs errors, never returns.
+ */
+_hidden void libxl__exec(libxl__gc *gc, int stdinfd, int stdoutfd,
+ int stderrfd, const char *arg0, char *const args[],
+ char *const env[]);
/* from xl_create */
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 02/13] libxl: fix libxl__xs_directory usage of transaction
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
2012-05-16 16:11 ` [PATCH 01/13] libxl: pass env vars to libxl__exec Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:03 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 03/13] libxl: add libxl__xs_path_cleanup Roger Pau Monne
` (10 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
libxl__xs_directory takes a transaction parameter, but completely
ignores it, passing XBT_NULL unconditionally to xs_directory.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl_xshelp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index 3ea8d08..6ca1afe 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -111,7 +111,7 @@ char **libxl__xs_directory(libxl__gc *gc, xs_transaction_t t,
{
libxl_ctx *ctx = libxl__gc_owner(gc);
char **ret = NULL;
- ret = xs_directory(ctx->xsh, XBT_NULL, path, nb);
+ ret = xs_directory(ctx->xsh, t, path, nb);
libxl__ptr_add(gc, ret);
return ret;
}
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 03/13] libxl: add libxl__xs_path_cleanup
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
2012-05-16 16:11 ` [PATCH 01/13] libxl: pass env vars to libxl__exec Roger Pau Monne
2012-05-16 16:11 ` [PATCH 02/13] libxl: fix libxl__xs_directory usage of transaction Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:06 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 04/13] libxl: move libxl_device_disk_add to libxl_device Roger Pau Monne
` (9 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Add a function which behaves like "xenstore-rm -t", and which will be
used to clean xenstore after unplug since we will be no longer
executing xen-hotplug-cleanup script, that used to do that for us.
Changes since v4:
* Caller must supply a transaction.
Changes since v3:
* Better error checking and function description.
Changes since v2:
* Moved the function to libxl_xshelp.c and added the prototype to
libxl_internal.h.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl_internal.h | 8 ++++++++
tools/libxl/libxl_xshelp.c | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index aaaf644..ae5527b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -494,6 +494,14 @@ _hidden bool libxl__xs_mkdir(libxl__gc *gc, xs_transaction_t t,
_hidden char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid);
+/*
+ * This is a recursive delete, from top to bottom. What this function does
+ * is remove empty folders that contained the deleted entry.
+ *
+ * It mimics xenstore-rm -t behaviour.
+ */
+_hidden int libxl__xs_path_cleanup(libxl__gc *gc, xs_transaction_t t,
+ char *user_path);
/*
* Event generation functions provided by the libxl event core to the
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index 6ca1afe..0667fae 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -135,6 +135,48 @@ char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid)
return s;
}
+int libxl__xs_path_cleanup(libxl__gc *gc, xs_transaction_t t, char *user_path)
+{
+ unsigned int nb = 0;
+ char *path, *last, *val;
+ int rc;
+
+ if (!user_path) {
+ LOGE(ERROR, "null path provided");
+ return ERROR_INVAL;
+ }
+ if (!t) {
+ LOGE(ERROR, "null transaction provided");
+ return ERROR_INVAL;
+ }
+
+ path = libxl__strdup(gc, user_path);
+ if (!xs_rm(CTX->xsh, t, path)) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ for (last = strrchr(path, '/'); last != NULL; last = strrchr(path, '/')) {
+ *last = '\0';
+
+ if (!strlen(path)) break;
+
+ val = libxl__xs_read(gc, t, path);
+ if (!val || strlen(val) != 0) break;
+
+ if (!libxl__xs_directory(gc, t, path, &nb) || nb != 0) break;
+
+ if (!xs_rm(CTX->xsh, t, path)) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ }
+ rc = 0;
+
+out:
+ return rc;
+}
+
/*
* Local variables:
* mode: C
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 04/13] libxl: move libxl_device_disk_add to libxl_device
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (2 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 03/13] libxl: add libxl__xs_path_cleanup Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-16 16:11 ` [PATCH 05/13] libxl: move libxl_device_nic_add " Roger Pau Monne
` (8 subsequent siblings)
12 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Move the code of this function to libxl_device.c so it can be made
asyncronious later on the series. The static function
libxl__device_from_disk also has to be moved to libxl_device and it is
no longer static.
The code will be fixed in a latter patch, replacing libxl__sprintf,
LIBXL_LOG* and lines > 80.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl.c | 152 +---------------------------------------
tools/libxl/libxl_device.c | 159 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 5 ++
3 files changed, 166 insertions(+), 150 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2a09192..2d8abd0 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1281,161 +1281,13 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
return rc;
}
-static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
- libxl_device_disk *disk,
- libxl__device *device)
-{
- libxl_ctx *ctx = libxl__gc_owner(gc);
- int devid;
-
- devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
- if (devid==-1) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
- " virtual disk identifier %s", disk->vdev);
- return ERROR_INVAL;
- }
-
- device->backend_domid = disk->backend_domid;
- device->backend_devid = devid;
-
- switch (disk->backend) {
- case LIBXL_DISK_BACKEND_PHY:
- device->backend_kind = LIBXL__DEVICE_KIND_VBD;
- break;
- case LIBXL_DISK_BACKEND_TAP:
- device->backend_kind = LIBXL__DEVICE_KIND_VBD;
- break;
- case LIBXL_DISK_BACKEND_QDISK:
- device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
- break;
- default:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
- disk->backend);
- return ERROR_INVAL;
- }
-
- device->domid = domid;
- device->devid = devid;
- device->kind = LIBXL__DEVICE_KIND_VBD;
-
- return 0;
-}
-
int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
{
GC_INIT(ctx);
- flexarray_t *front;
- flexarray_t *back;
- char *dev;
- libxl__device device;
- int major, minor, rc;
-
- rc = libxl__device_disk_setdefault(gc, disk);
- if (rc) goto out;
-
- front = flexarray_make(16, 1);
- if (!front) {
- rc = ERROR_NOMEM;
- goto out;
- }
- back = flexarray_make(16, 1);
- if (!back) {
- rc = ERROR_NOMEM;
- goto out_free;
- }
-
- if (disk->script) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
- " not yet supported, sorry");
- rc = ERROR_INVAL;
- goto out_free;
- }
-
- rc = libxl__device_from_disk(gc, domid, disk, &device);
- if (rc != 0) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
- " virtual disk identifier %s", disk->vdev);
- goto out_free;
- }
-
- switch (disk->backend) {
- case LIBXL_DISK_BACKEND_PHY:
- dev = disk->pdev_path;
- do_backend_phy:
- libxl__device_physdisk_major_minor(dev, &major, &minor);
- flexarray_append(back, "physical-device");
- flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
-
- flexarray_append(back, "params");
- flexarray_append(back, dev);
-
- assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
- break;
- case LIBXL_DISK_BACKEND_TAP:
- dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
- if (!dev) {
- LOG(ERROR, "failed to get blktap devpath for %p\n",
- disk->pdev_path);
- rc = ERROR_FAIL;
- goto out_free;
- }
- flexarray_append(back, "tapdisk-params");
- flexarray_append(back, libxl__sprintf(gc, "%s:%s",
- libxl__device_disk_string_of_format(disk->format),
- disk->pdev_path));
-
- /* now create a phy device to export the device to the guest */
- goto do_backend_phy;
- case LIBXL_DISK_BACKEND_QDISK:
- flexarray_append(back, "params");
- flexarray_append(back, libxl__sprintf(gc, "%s:%s",
- libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
- assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
- break;
- default:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
- rc = ERROR_INVAL;
- goto out_free;
- }
-
- flexarray_append(back, "frontend-id");
- flexarray_append(back, libxl__sprintf(gc, "%d", domid));
- flexarray_append(back, "online");
- flexarray_append(back, "1");
- flexarray_append(back, "removable");
- flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
- flexarray_append(back, "bootable");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
- flexarray_append(back, "state");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
- flexarray_append(back, "dev");
- flexarray_append(back, disk->vdev);
- flexarray_append(back, "type");
- flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
- flexarray_append(back, "mode");
- flexarray_append(back, disk->readwrite ? "w" : "r");
- flexarray_append(back, "device-type");
- flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
-
- flexarray_append(front, "backend-id");
- flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
- flexarray_append(front, "state");
- flexarray_append(front, libxl__sprintf(gc, "%d", 1));
- flexarray_append(front, "virtual-device");
- flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
- flexarray_append(front, "device-type");
- flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
-
- libxl__device_generic_add(gc, &device,
- libxl__xs_kvs_of_flexarray(gc, back, back->count),
- libxl__xs_kvs_of_flexarray(gc, front, front->count));
+ int rc;
- rc = 0;
+ rc = libxl__device_disk_add(gc, domid, disk);
-out_free:
- flexarray_free(back);
- flexarray_free(front);
-out:
GC_FREE;
return rc;
}
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c7e057d..304929a 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -249,6 +249,165 @@ char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
}
}
+int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+ libxl_device_disk *disk,
+ libxl__device *device)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ int devid;
+
+ devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
+ if (devid==-1) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
+ " virtual disk identifier %s", disk->vdev);
+ return ERROR_INVAL;
+ }
+
+ device->backend_domid = disk->backend_domid;
+ device->backend_devid = devid;
+
+ switch (disk->backend) {
+ case LIBXL_DISK_BACKEND_PHY:
+ device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+ break;
+ case LIBXL_DISK_BACKEND_TAP:
+ device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+ break;
+ case LIBXL_DISK_BACKEND_QDISK:
+ device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
+ break;
+ default:
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
+ disk->backend);
+ return ERROR_INVAL;
+ }
+
+ device->domid = domid;
+ device->devid = devid;
+ device->kind = LIBXL__DEVICE_KIND_VBD;
+
+ return 0;
+}
+
+int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+ libxl_device_disk *disk)
+{
+ libxl_ctx *ctx = CTX;
+ flexarray_t *front;
+ flexarray_t *back;
+ char *dev;
+ libxl__device device;
+ int major, minor, rc;
+
+ rc = libxl__device_disk_setdefault(gc, disk);
+ if (rc) goto out;
+
+ front = flexarray_make(16, 1);
+ if (!front) {
+ rc = ERROR_NOMEM;
+ goto out;
+ }
+ back = flexarray_make(16, 1);
+ if (!back) {
+ rc = ERROR_NOMEM;
+ goto out_free;
+ }
+
+ if (disk->script) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
+ " not yet supported, sorry");
+ rc = ERROR_INVAL;
+ goto out_free;
+ }
+
+ rc = libxl__device_from_disk(gc, domid, disk, &device);
+ if (rc != 0) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
+ " virtual disk identifier %s", disk->vdev);
+ goto out_free;
+ }
+
+ switch (disk->backend) {
+ case LIBXL_DISK_BACKEND_PHY:
+ dev = disk->pdev_path;
+ do_backend_phy:
+ libxl__device_physdisk_major_minor(dev, &major, &minor);
+ flexarray_append(back, "physical-device");
+ flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
+
+ flexarray_append(back, "params");
+ flexarray_append(back, dev);
+
+ assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
+ break;
+ case LIBXL_DISK_BACKEND_TAP:
+ dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
+ if (!dev) {
+ LOG(ERROR, "failed to get blktap devpath for %p\n",
+ disk->pdev_path);
+ rc = ERROR_FAIL;
+ goto out_free;
+ }
+ flexarray_append(back, "tapdisk-params");
+ flexarray_append(back, libxl__sprintf(gc, "%s:%s",
+ libxl__device_disk_string_of_format(disk->format),
+ disk->pdev_path));
+
+ /* now create a phy device to export the device to the guest */
+ goto do_backend_phy;
+ case LIBXL_DISK_BACKEND_QDISK:
+ flexarray_append(back, "params");
+ flexarray_append(back, libxl__sprintf(gc, "%s:%s",
+ libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
+ assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
+ break;
+ default:
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
+ rc = ERROR_INVAL;
+ goto out_free;
+ }
+
+ flexarray_append(back, "frontend-id");
+ flexarray_append(back, libxl__sprintf(gc, "%d", domid));
+ flexarray_append(back, "online");
+ flexarray_append(back, "1");
+ flexarray_append(back, "removable");
+ flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
+ flexarray_append(back, "bootable");
+ flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(back, "state");
+ flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(back, "dev");
+ flexarray_append(back, disk->vdev);
+ flexarray_append(back, "type");
+ flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
+ flexarray_append(back, "mode");
+ flexarray_append(back, disk->readwrite ? "w" : "r");
+ flexarray_append(back, "device-type");
+ flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
+
+ flexarray_append(front, "backend-id");
+ flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
+ flexarray_append(front, "state");
+ flexarray_append(front, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(front, "virtual-device");
+ flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
+ flexarray_append(front, "device-type");
+ flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
+
+ libxl__device_generic_add(gc, &device,
+ libxl__xs_kvs_of_flexarray(gc, back, back->count),
+ libxl__xs_kvs_of_flexarray(gc, front, front->count));
+
+ rc = 0;
+
+out_free:
+ flexarray_free(back);
+ flexarray_free(front);
+out:
+ return rc;
+}
+
int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor)
{
struct stat buf;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index ae5527b..87c9366 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -780,6 +780,11 @@ _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
_hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
_hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
_hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*);
+_hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+ libxl_device_disk *disk,
+ libxl__device *device);
+_hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+ libxl_device_disk *disk);
_hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
_hidden int libxl__device_disk_dev_number(const char *virtpath,
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 05/13] libxl: move libxl_device_nic_add to libxl_device
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (3 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 04/13] libxl: move libxl_device_disk_add to libxl_device Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-16 16:11 ` [PATCH 06/13] libxl: cleanup libxl__device_{disk, nic}_add Roger Pau Monne
` (7 subsequent siblings)
12 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Move the code of this function to libxl_device.c so it can be made
asyncronious later on the series. The static function
libxl__device_from_nic also has to be moved to libxl_device and it is
no longer static.
The code will be fixed in a latter patch, replacing libxl__sprintf,
LIBXL_LOG* and lines > 80.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl.c | 108 +---------------------------------------
tools/libxl/libxl_device.c | 113 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 6 ++
3 files changed, 121 insertions(+), 106 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2d8abd0..d3b6a53 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1655,117 +1655,13 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
return 0;
}
-static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
- libxl_device_nic *nic,
- libxl__device *device)
-{
- device->backend_devid = nic->devid;
- device->backend_domid = nic->backend_domid;
- device->backend_kind = LIBXL__DEVICE_KIND_VIF;
- device->devid = nic->devid;
- device->domid = domid;
- device->kind = LIBXL__DEVICE_KIND_VIF;
-
- return 0;
-}
-
int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
{
GC_INIT(ctx);
- flexarray_t *front;
- flexarray_t *back;
- libxl__device device;
- char *dompath, **l;
- unsigned int nb, rc;
-
- rc = libxl__device_nic_setdefault(gc, nic);
- if (rc) goto out;
-
- front = flexarray_make(16, 1);
- if (!front) {
- rc = ERROR_NOMEM;
- goto out;
- }
- back = flexarray_make(16, 1);
- if (!back) {
- rc = ERROR_NOMEM;
- goto out_free;
- }
-
- if (nic->devid == -1) {
- if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
- rc = ERROR_FAIL;
- goto out_free;
- }
- if (!(l = libxl__xs_directory(gc, XBT_NULL,
- libxl__sprintf(gc, "%s/device/vif", dompath), &nb))) {
- nic->devid = 0;
- } else {
- nic->devid = strtoul(l[nb - 1], NULL, 10) + 1;
- }
- }
-
- rc = libxl__device_from_nic(gc, domid, nic, &device);
- if ( rc != 0 ) goto out_free;
-
- flexarray_append(back, "frontend-id");
- flexarray_append(back, libxl__sprintf(gc, "%d", domid));
- flexarray_append(back, "online");
- flexarray_append(back, "1");
- flexarray_append(back, "state");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
- if (nic->script) {
- flexarray_append(back, "script");
- flexarray_append(back, nic->script[0]=='/' ? nic->script
- : libxl__sprintf(gc, "%s/%s",
- libxl__xen_script_dir_path(),
- nic->script));
- }
-
- if (nic->ifname) {
- flexarray_append(back, "vifname");
- flexarray_append(back, nic->ifname);
- }
-
- flexarray_append(back, "mac");
- flexarray_append(back,libxl__sprintf(gc,
- LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
- if (nic->ip) {
- flexarray_append(back, "ip");
- flexarray_append(back, libxl__strdup(gc, nic->ip));
- }
-
- if (nic->rate_interval_usecs > 0) {
- flexarray_append(back, "rate");
- flexarray_append(back, libxl__sprintf(gc, "%"PRIu64",%"PRIu32"",
- nic->rate_bytes_per_interval,
- nic->rate_interval_usecs));
- }
-
- flexarray_append(back, "bridge");
- flexarray_append(back, libxl__strdup(gc, nic->bridge));
- flexarray_append(back, "handle");
- flexarray_append(back, libxl__sprintf(gc, "%d", nic->devid));
+ int rc;
- flexarray_append(front, "backend-id");
- flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domid));
- flexarray_append(front, "state");
- flexarray_append(front, libxl__sprintf(gc, "%d", 1));
- flexarray_append(front, "handle");
- flexarray_append(front, libxl__sprintf(gc, "%d", nic->devid));
- flexarray_append(front, "mac");
- flexarray_append(front, libxl__sprintf(gc,
- LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
- libxl__device_generic_add(gc, &device,
- libxl__xs_kvs_of_flexarray(gc, back, back->count),
- libxl__xs_kvs_of_flexarray(gc, front, front->count));
+ rc = libxl__device_nic_add(gc, domid, nic);
- /* FIXME: wait for plug */
- rc = 0;
-out_free:
- flexarray_free(back);
- flexarray_free(front);
-out:
GC_FREE;
return rc;
}
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 304929a..edc4ad1 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -408,6 +408,119 @@ out:
return rc;
}
+int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
+ libxl_device_nic *nic,
+ libxl__device *device)
+{
+ device->backend_devid = nic->devid;
+ device->backend_domid = nic->backend_domid;
+ device->backend_kind = LIBXL__DEVICE_KIND_VIF;
+ device->devid = nic->devid;
+ device->domid = domid;
+ device->kind = LIBXL__DEVICE_KIND_VIF;
+
+ return 0;
+}
+
+int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
+{
+ flexarray_t *front;
+ flexarray_t *back;
+ libxl__device device;
+ char *dompath, **l;
+ unsigned int nb, rc;
+
+ rc = libxl__device_nic_setdefault(gc, nic);
+ if (rc) goto out;
+
+ front = flexarray_make(16, 1);
+ if (!front) {
+ rc = ERROR_NOMEM;
+ goto out;
+ }
+ back = flexarray_make(16, 1);
+ if (!back) {
+ rc = ERROR_NOMEM;
+ goto out_free;
+ }
+
+ if (nic->devid == -1) {
+ if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
+ rc = ERROR_FAIL;
+ goto out_free;
+ }
+ if (!(l = libxl__xs_directory(gc, XBT_NULL,
+ libxl__sprintf(gc, "%s/device/vif", dompath), &nb))) {
+ nic->devid = 0;
+ } else {
+ nic->devid = strtoul(l[nb - 1], NULL, 10) + 1;
+ }
+ }
+
+ rc = libxl__device_from_nic(gc, domid, nic, &device);
+ if ( rc != 0 ) goto out_free;
+
+ flexarray_append(back, "frontend-id");
+ flexarray_append(back, libxl__sprintf(gc, "%d", domid));
+ flexarray_append(back, "online");
+ flexarray_append(back, "1");
+ flexarray_append(back, "state");
+ flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+ if (nic->script) {
+ flexarray_append(back, "script");
+ flexarray_append(back, nic->script[0]=='/' ? nic->script
+ : libxl__sprintf(gc, "%s/%s",
+ libxl__xen_script_dir_path(),
+ nic->script));
+ }
+
+ if (nic->ifname) {
+ flexarray_append(back, "vifname");
+ flexarray_append(back, nic->ifname);
+ }
+
+ flexarray_append(back, "mac");
+ flexarray_append(back,libxl__sprintf(gc,
+ LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
+ if (nic->ip) {
+ flexarray_append(back, "ip");
+ flexarray_append(back, libxl__strdup(gc, nic->ip));
+ }
+
+ if (nic->rate_interval_usecs > 0) {
+ flexarray_append(back, "rate");
+ flexarray_append(back, libxl__sprintf(gc, "%"PRIu64",%"PRIu32"",
+ nic->rate_bytes_per_interval,
+ nic->rate_interval_usecs));
+ }
+
+ flexarray_append(back, "bridge");
+ flexarray_append(back, libxl__strdup(gc, nic->bridge));
+ flexarray_append(back, "handle");
+ flexarray_append(back, libxl__sprintf(gc, "%d", nic->devid));
+
+ flexarray_append(front, "backend-id");
+ flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domid));
+ flexarray_append(front, "state");
+ flexarray_append(front, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(front, "handle");
+ flexarray_append(front, libxl__sprintf(gc, "%d", nic->devid));
+ flexarray_append(front, "mac");
+ flexarray_append(front, libxl__sprintf(gc,
+ LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
+ libxl__device_generic_add(gc, &device,
+ libxl__xs_kvs_of_flexarray(gc, back, back->count),
+ libxl__xs_kvs_of_flexarray(gc, front, front->count));
+
+ /* FIXME: wait for plug */
+ rc = 0;
+out_free:
+ flexarray_free(back);
+ flexarray_free(front);
+out:
+ return rc;
+}
+
int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor)
{
struct stat buf;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 87c9366..0ddfe72 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -786,6 +786,12 @@ _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
_hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
libxl_device_disk *disk);
+_hidden int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
+ libxl_device_nic *nic,
+ libxl__device *device);
+_hidden int libxl__device_nic_add(libxl__gc *gc, uint32_t domid,
+ libxl_device_nic *nic);
+
_hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
_hidden int libxl__device_disk_dev_number(const char *virtpath,
int *pdisk, int *ppartition);
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 06/13] libxl: cleanup libxl__device_{disk, nic}_add
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (4 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 05/13] libxl: move libxl_device_nic_add " Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:07 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op Roger Pau Monne
` (6 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Change libxl__sprintf for GCSPRINTF, LIBXL__LOG* for LOG and remove
use of ctx variable in favour of the CTX constant.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl_device.c | 114 +++++++++++++++++++++-----------------------
1 files changed, 55 insertions(+), 59 deletions(-)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index edc4ad1..1e34135 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -253,13 +253,12 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
libxl_device_disk *disk,
libxl__device *device)
{
- libxl_ctx *ctx = libxl__gc_owner(gc);
int devid;
devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
if (devid==-1) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
- " virtual disk identifier %s", disk->vdev);
+ LOG(ERROR, "Invalid or unsupported virtual disk identifier %s",
+ disk->vdev);
return ERROR_INVAL;
}
@@ -267,19 +266,18 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
device->backend_devid = devid;
switch (disk->backend) {
- case LIBXL_DISK_BACKEND_PHY:
- device->backend_kind = LIBXL__DEVICE_KIND_VBD;
- break;
- case LIBXL_DISK_BACKEND_TAP:
- device->backend_kind = LIBXL__DEVICE_KIND_VBD;
- break;
- case LIBXL_DISK_BACKEND_QDISK:
- device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
- break;
- default:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
- disk->backend);
- return ERROR_INVAL;
+ case LIBXL_DISK_BACKEND_PHY:
+ device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+ break;
+ case LIBXL_DISK_BACKEND_TAP:
+ device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+ break;
+ case LIBXL_DISK_BACKEND_QDISK:
+ device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
+ break;
+ default:
+ LOG(ERROR, "unrecognized disk backend type: %d\n", disk->backend);
+ return ERROR_INVAL;
}
device->domid = domid;
@@ -292,10 +290,9 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
libxl_device_disk *disk)
{
- libxl_ctx *ctx = CTX;
flexarray_t *front;
flexarray_t *back;
- char *dev;
+ char *dev, *format;
libxl__device device;
int major, minor, rc;
@@ -314,18 +311,18 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
}
if (disk->script) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
- " not yet supported, sorry");
+ LOG(ERROR, "External block scripts not yet supported, sorry");
rc = ERROR_INVAL;
goto out_free;
}
rc = libxl__device_from_disk(gc, domid, disk, &device);
if (rc != 0) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
- " virtual disk identifier %s", disk->vdev);
+ LOG(ERROR, "Invalid or unsupported virtual disk identifier %s",
+ disk->vdev);
goto out_free;
}
+ format = libxl__device_disk_string_of_format(disk->format);
switch (disk->backend) {
case LIBXL_DISK_BACKEND_PHY:
@@ -333,7 +330,7 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
do_backend_phy:
libxl__device_physdisk_major_minor(dev, &major, &minor);
flexarray_append(back, "physical-device");
- flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
+ flexarray_append(back, GCSPRINTF("%x:%x", major, minor));
flexarray_append(back, "params");
flexarray_append(back, dev);
@@ -349,34 +346,31 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
goto out_free;
}
flexarray_append(back, "tapdisk-params");
- flexarray_append(back, libxl__sprintf(gc, "%s:%s",
- libxl__device_disk_string_of_format(disk->format),
- disk->pdev_path));
+ flexarray_append(back, GCSPRINTF("%s:%s", format, disk->pdev_path));
/* now create a phy device to export the device to the guest */
goto do_backend_phy;
case LIBXL_DISK_BACKEND_QDISK:
flexarray_append(back, "params");
- flexarray_append(back, libxl__sprintf(gc, "%s:%s",
- libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
+ flexarray_append(back, GCSPRINTF("%s:%s", format, disk->pdev_path));
assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
break;
default:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
+ LOG(ERROR, "unrecognized disk backend type: %d\n", disk->backend);
rc = ERROR_INVAL;
goto out_free;
}
flexarray_append(back, "frontend-id");
- flexarray_append(back, libxl__sprintf(gc, "%d", domid));
+ flexarray_append(back, GCSPRINTF("%d", domid));
flexarray_append(back, "online");
flexarray_append(back, "1");
flexarray_append(back, "removable");
- flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
+ flexarray_append(back, GCSPRINTF("%d", (disk->removable) ? 1 : 0));
flexarray_append(back, "bootable");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(back, GCSPRINTF("%d", 1));
flexarray_append(back, "state");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(back, GCSPRINTF("%d", 1));
flexarray_append(back, "dev");
flexarray_append(back, disk->vdev);
flexarray_append(back, "type");
@@ -387,17 +381,17 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
flexarray_append(front, "backend-id");
- flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
+ flexarray_append(front, GCSPRINTF("%d", disk->backend_domid));
flexarray_append(front, "state");
- flexarray_append(front, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(front, GCSPRINTF("%d", 1));
flexarray_append(front, "virtual-device");
- flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
+ flexarray_append(front, GCSPRINTF("%d", device.devid));
flexarray_append(front, "device-type");
flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
libxl__device_generic_add(gc, &device,
- libxl__xs_kvs_of_flexarray(gc, back, back->count),
- libxl__xs_kvs_of_flexarray(gc, front, front->count));
+ libxl__xs_kvs_of_flexarray(gc, back, back->count),
+ libxl__xs_kvs_of_flexarray(gc, front, front->count));
rc = 0;
@@ -449,8 +443,10 @@ int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
rc = ERROR_FAIL;
goto out_free;
}
- if (!(l = libxl__xs_directory(gc, XBT_NULL,
- libxl__sprintf(gc, "%s/device/vif", dompath), &nb))) {
+ l = libxl__xs_directory(gc, XBT_NULL, GCSPRINTF(
+ "%s/device/vif", dompath),
+ &nb);
+ if (!l) {
nic->devid = 0;
} else {
nic->devid = strtoul(l[nb - 1], NULL, 10) + 1;
@@ -461,17 +457,18 @@ int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
if ( rc != 0 ) goto out_free;
flexarray_append(back, "frontend-id");
- flexarray_append(back, libxl__sprintf(gc, "%d", domid));
+ flexarray_append(back, GCSPRINTF("%d", domid));
flexarray_append(back, "online");
flexarray_append(back, "1");
flexarray_append(back, "state");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(back, GCSPRINTF("%d", 1));
if (nic->script) {
flexarray_append(back, "script");
- flexarray_append(back, nic->script[0]=='/' ? nic->script
- : libxl__sprintf(gc, "%s/%s",
- libxl__xen_script_dir_path(),
- nic->script));
+ flexarray_append(back, nic->script[0]=='/' ?
+ nic->script :
+ GCSPRINTF("%s/%s",
+ libxl__xen_script_dir_path(),
+ nic->script));
}
if (nic->ifname) {
@@ -480,8 +477,7 @@ int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
}
flexarray_append(back, "mac");
- flexarray_append(back,libxl__sprintf(gc,
- LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
+ flexarray_append(back, GCSPRINTF(LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
if (nic->ip) {
flexarray_append(back, "ip");
flexarray_append(back, libxl__strdup(gc, nic->ip));
@@ -489,28 +485,28 @@ int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
if (nic->rate_interval_usecs > 0) {
flexarray_append(back, "rate");
- flexarray_append(back, libxl__sprintf(gc, "%"PRIu64",%"PRIu32"",
- nic->rate_bytes_per_interval,
- nic->rate_interval_usecs));
+ flexarray_append(back, GCSPRINTF("%"PRIu64",%"PRIu32"",
+ nic->rate_bytes_per_interval,
+ nic->rate_interval_usecs));
}
flexarray_append(back, "bridge");
flexarray_append(back, libxl__strdup(gc, nic->bridge));
flexarray_append(back, "handle");
- flexarray_append(back, libxl__sprintf(gc, "%d", nic->devid));
+ flexarray_append(back, GCSPRINTF("%d", nic->devid));
flexarray_append(front, "backend-id");
- flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domid));
+ flexarray_append(front, GCSPRINTF("%d", nic->backend_domid));
flexarray_append(front, "state");
- flexarray_append(front, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(front, GCSPRINTF("%d", 1));
flexarray_append(front, "handle");
- flexarray_append(front, libxl__sprintf(gc, "%d", nic->devid));
+ flexarray_append(front, GCSPRINTF("%d", nic->devid));
flexarray_append(front, "mac");
- flexarray_append(front, libxl__sprintf(gc,
- LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
+ flexarray_append(front, GCSPRINTF(LIBXL_MAC_FMT,
+ LIBXL_MAC_BYTES(nic->mac)));
libxl__device_generic_add(gc, &device,
- libxl__xs_kvs_of_flexarray(gc, back, back->count),
- libxl__xs_kvs_of_flexarray(gc, front, front->count));
+ libxl__xs_kvs_of_flexarray(gc, back, back->count),
+ libxl__xs_kvs_of_flexarray(gc, front, front->count));
/* FIXME: wait for plug */
rc = 0;
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (5 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 06/13] libxl: cleanup libxl__device_{disk, nic}_add Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:19 ` Ian Jackson
2012-05-18 16:24 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 08/13] libxl: convert libxl_device_disk_add to an async operation Roger Pau Monne
` (5 subsequent siblings)
12 siblings, 2 replies; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
This change introduces some new structures, and breaks the mutual
dependency that libxl_domain_destroy and libxl__destroy_device_model
had. This is done by checking if the domid passed to
libxl_domain_destroy has a stubdom, and then having the bulk of the
destroy machinery in a separate function (libxl__destroy_domid) that
doesn't check for stubdom presence, since we check for it in the upper
level function. The reason behind this change is the need to use
structures for ao operations, and it was impossible to have two
different self-referencing structs.
All uses of libxl_domain_destroy have been changed, and either
replaced by the new libxl_domain_destroy ao function or by the
internal libxl__domain_destroy that can be used inside an already
running ao.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl.c | 180 +++++++++++-----------
tools/libxl/libxl.h | 3 +-
tools/libxl/libxl_create.c | 30 +++-
tools/libxl/libxl_device.c | 308 ++++++++++++++++++++++++++++---------
tools/libxl/libxl_dm.c | 83 +++++-----
tools/libxl/libxl_dom.c | 170 ++++++++++++++++++++
tools/libxl/libxl_internal.h | 224 +++++++++++++++++++++------
tools/libxl/xl_cmdimpl.c | 12 +-
tools/python/xen/lowlevel/xl/xl.c | 2 +-
9 files changed, 752 insertions(+), 260 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index d3b6a53..dabe92b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1070,80 +1070,34 @@ void libxl_evdisable_disk_eject(libxl_ctx *ctx, libxl_evgen_disk_eject *evg) {
GC_FREE;
}
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
-{
- GC_INIT(ctx);
- char *dom_path;
- char *vm_path;
- char *pid;
- int rc, dm_present;
-
- rc = libxl_domain_info(ctx, NULL, domid);
- switch(rc) {
- case 0:
- break;
- case ERROR_INVAL:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "non-existant domain %d", domid);
- default:
- return rc;
- }
-
- switch (libxl__domain_type(gc, domid)) {
- case LIBXL_DOMAIN_TYPE_HVM:
- dm_present = 1;
- break;
- case LIBXL_DOMAIN_TYPE_PV:
- pid = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/%d/image/device-model-pid", domid));
- dm_present = (pid != NULL);
- break;
- default:
- abort();
- }
-
- dom_path = libxl__xs_get_dompath(gc, domid);
- if (!dom_path) {
- rc = ERROR_FAIL;
- goto out;
- }
+/* Callback for domain destruction */
- if (libxl__device_pci_destroy_all(gc, domid) < 0)
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "pci shutdown failed for domid %d", domid);
- rc = xc_domain_pause(ctx->xch, domid);
- if (rc < 0) {
- LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, "xc_domain_pause failed for %d", domid);
- }
- if (dm_present) {
- if (libxl__destroy_device_model(gc, domid) < 0)
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl__destroy_device_model failed for %d", domid);
+static void domain_destroy_cb(libxl__egc *, libxl__domain_destroy_state *, int);
- libxl__qmp_cleanup(gc, domid);
- }
- if (libxl__devices_destroy(gc, domid) < 0)
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "libxl__devices_destroy failed for %d", domid);
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
+ const libxl_asyncop_how *ao_how)
+{
+ AO_CREATE(ctx, domid, ao_how);
+ libxl__domain_destroy_state *dds;
- vm_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/vm", dom_path));
- if (vm_path)
- if (!xs_rm(ctx->xsh, XBT_NULL, vm_path))
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs_rm failed for %s", vm_path);
+ GCNEW(dds);
+ dds->ao = ao;
+ dds->domid = domid;
+ dds->callback = domain_destroy_cb;
+ libxl__domain_destroy(egc, dds);
- if (!xs_rm(ctx->xsh, XBT_NULL, dom_path))
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs_rm failed for %s", dom_path);
+ return AO_INPROGRESS;
+}
- xs_rm(ctx->xsh, XBT_NULL, libxl__xs_libxl_path(gc, domid));
+static void domain_destroy_cb(libxl__egc *egc, libxl__domain_destroy_state *dds,
+ int rc)
+{
+ STATE_AO_GC(dds->ao);
- libxl__userdata_destroyall(gc, domid);
+ if (rc)
+ LOG(ERROR, "destruction of domain %u failed", dds->domid);
- rc = xc_domain_destroy(ctx->xch, domid);
- if (rc < 0) {
- LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, "xc_domain_destroy failed for %d", domid);
- rc = ERROR_FAIL;
- goto out;
- }
- rc = 0;
-out:
- GC_FREE;
- return rc;
+ libxl__ao_complete(egc, ao, rc);
}
int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type)
@@ -1271,6 +1225,26 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass)
/******************************************************************************/
+/* generic callback for devices that only need to set ao_complete */
+static void libxl__device_cb(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+
+ if (aorm->rc) {
+ LOGE(ERROR, "unable to %s %s with id %u",
+ aorm->action == DEVICE_CONNECT ? "add" : "remove",
+ libxl__device_kind_to_string(aorm->dev->kind),
+ aorm->dev->devid);
+ goto out;
+ }
+
+out:
+ libxl__ao_complete(egc, ao, aorm->rc);
+ return;
+}
+
+/******************************************************************************/
+
int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
{
int rc;
@@ -1297,19 +1271,25 @@ int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
const libxl_asyncop_how *ao_how)
{
AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
+ libxl__device *device;
+ libxl__ao_device *aorm = 0;
int rc;
- rc = libxl__device_from_disk(gc, domid, disk, &device);
+ GCNEW(device);
+ rc = libxl__device_from_disk(gc, domid, disk, device);
if (rc != 0) goto out;
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ GCNEW(aorm);
+ libxl__init_ao_device(aorm, ao, NULL);
+ aorm->action = DEVICE_DISCONNECT;
+ aorm->dev = device;
+ aorm->callback = libxl__device_cb;
- return AO_INPROGRESS;
+ libxl__initiate_device_remove(egc, aorm);
out:
- return AO_ABORT(rc);
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
@@ -1671,19 +1651,25 @@ int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
const libxl_asyncop_how *ao_how)
{
AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
+ libxl__device *device;
+ libxl__ao_device *aorm = 0;
int rc;
- rc = libxl__device_from_nic(gc, domid, nic, &device);
+ GCNEW(device);
+ rc = libxl__device_from_nic(gc, domid, nic, device);
if (rc != 0) goto out;
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ GCNEW(aorm);
+ libxl__init_ao_device(aorm, ao, NULL);
+ aorm->action = DEVICE_DISCONNECT;
+ aorm->dev = device;
+ aorm->callback = libxl__device_cb;
- return AO_INPROGRESS;
+ libxl__initiate_device_remove(egc, aorm);
out:
- return AO_ABORT(rc);
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid,
@@ -2033,19 +2019,25 @@ int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
const libxl_asyncop_how *ao_how)
{
AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
+ libxl__device *device;
+ libxl__ao_device *aorm = 0;
int rc;
- rc = libxl__device_from_vkb(gc, domid, vkb, &device);
+ GCNEW(device);
+ rc = libxl__device_from_vkb(gc, domid, vkb, device);
if (rc != 0) goto out;
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ GCNEW(aorm);
+ libxl__init_ao_device(aorm, ao, NULL);
+ aorm->action = DEVICE_DISCONNECT;
+ aorm->dev = device;
+ aorm->callback = libxl__device_cb;
- return AO_INPROGRESS;
+ libxl__initiate_device_remove(egc, aorm);
out:
- return AO_ABORT(rc);
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid,
@@ -2166,19 +2158,25 @@ int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
const libxl_asyncop_how *ao_how)
{
AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
+ libxl__device *device;
+ libxl__ao_device *aorm = 0;
int rc;
- rc = libxl__device_from_vfb(gc, domid, vfb, &device);
+ GCNEW(device);
+ rc = libxl__device_from_vfb(gc, domid, vfb, device);
if (rc != 0) goto out;
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ GCNEW(aorm);
+ libxl__init_ao_device(aorm, ao, NULL);
+ aorm->action = DEVICE_DISCONNECT;
+ aorm->dev = device;
+ aorm->callback = libxl__device_cb;
- return AO_INPROGRESS;
+ libxl__initiate_device_remove(egc, aorm);
out:
- return AO_ABORT(rc);
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 979940a..2f4694e 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -530,7 +530,8 @@ int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
+ const libxl_asyncop_how *ao_how);
int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_info *info, const char *name_suffix, libxl_uuid new_uuid);
/* get max. number of cpus supported by hypervisor */
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 14721eb..bd8e6a6 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -584,6 +584,13 @@ static void domcreate_complete(libxl__egc *egc,
libxl__domain_create_state *dcs,
int rc);
+/* If creation is not successful, this callback will be executed
+ * when domain destruction is finished */
+
+static void domcreate_destruction_cb(libxl__egc *egc,
+ libxl__domain_destroy_state *dds,
+ int rc);
+
static void initiate_domain_create(libxl__egc *egc,
libxl__domain_create_state *dcs)
{
@@ -848,16 +855,31 @@ static void domcreate_complete(libxl__egc *egc,
if (rc) {
if (dcs->guest_domid) {
- int rc2 = libxl_domain_destroy(CTX, dcs->guest_domid);
- if (rc2)
- LOG(ERROR, "unable to destroy domain %d following"
- " failed creation", dcs->guest_domid);
+ dcs->dds.ao = ao;
+ dcs->dds.domid = dcs->guest_domid;
+ dcs->dds.callback = domcreate_destruction_cb;
+ libxl__domain_destroy(egc, &dcs->dds);
+ return;
}
dcs->guest_domid = -1;
}
dcs->callback(egc, dcs, rc, dcs->guest_domid);
}
+static void domcreate_destruction_cb(libxl__egc *egc,
+ libxl__domain_destroy_state *dds,
+ int rc)
+{
+ STATE_AO_GC(dds->ao);
+ libxl__domain_create_state *dcs = CONTAINER_OF(dds, *dcs, dds);
+
+ if (rc)
+ LOG(ERROR, "unable to destroy domain %u following failed creation",
+ dds->domid);
+
+ dcs->callback(egc, dcs, rc, dcs->guest_domid);
+}
+
/*----- application-facing domain creation interface -----*/
typedef struct {
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 1e34135..577324c 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -58,6 +58,48 @@ int libxl__parse_backend_path(libxl__gc *gc,
return libxl__device_kind_from_string(strkind, &dev->backend_kind);
}
+static int libxl__num_devices(libxl__gc *gc, uint32_t domid)
+{
+ char *path;
+ unsigned int num_kinds, num_devs;
+ char **kinds = NULL, **devs = NULL;
+ int i, j, rc = 0;
+ libxl__device dev;
+ libxl__device_kind kind;
+ int numdevs = 0;
+
+ path = GCSPRINTF("/local/domain/%d/device", domid);
+ kinds = libxl__xs_directory(gc, XBT_NULL, path, &num_kinds);
+ if (!kinds) {
+ if (errno != ENOENT) {
+ LOGE(ERROR, "unable to get xenstore device listing %s", path);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ num_kinds = 0;
+ }
+ for (i = 0; i < num_kinds; i++) {
+ if (libxl__device_kind_from_string(kinds[i], &kind))
+ continue;
+
+ path = GCSPRINTF("/local/domain/%d/device/%s", domid, kinds[i]);
+ devs = libxl__xs_directory(gc, XBT_NULL, path, &num_devs);
+ if (!devs)
+ continue;
+ for (j = 0; j < num_devs; j++) {
+ path = GCSPRINTF("/local/domain/%d/device/%s/%s/backend",
+ domid, kinds[i], devs[j]);
+ path = libxl__xs_read(gc, XBT_NULL, path);
+ if (path && libxl__parse_backend_path(gc, path, &dev) == 0) {
+ numdevs++;
+ }
+ }
+ }
+out:
+ if (rc) return rc;
+ return numdevs;
+}
+
int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
char **bents, char **fents)
{
@@ -624,49 +666,70 @@ int libxl__device_disk_dev_number(const char *virtpath, int *pdisk,
return -1;
}
+/* Device destruction */
-typedef struct {
- libxl__ao *ao;
- libxl__ev_devstate ds;
-} libxl__ao_device_remove;
-
-static void device_remove_cleanup(libxl__gc *gc,
- libxl__ao_device_remove *aorm) {
+static void device_backend_cleanup(libxl__gc *gc, libxl__ao_device *aorm)
+{
if (!aorm) return;
libxl__ev_devstate_cancel(gc, &aorm->ds);
}
-static void device_remove_callback(libxl__egc *egc, libxl__ev_devstate *ds,
- int rc) {
- libxl__ao_device_remove *aorm = CONTAINER_OF(ds, *aorm, ds);
- libxl__gc *gc = &aorm->ao->gc;
- libxl__ao_complete(egc, aorm->ao, rc);
- device_remove_cleanup(gc, aorm);
+void libxl__init_ao_device(libxl__ao_device *aorm, libxl__ao *ao,
+ libxl__ao_device **base)
+{
+ aorm->ao = ao;
+ aorm->state = DEVICE_ACTIVE;
+ aorm->rc = 0;
+ aorm->base = base;
+}
+
+int libxl__ao_device_check_last(libxl__gc *gc, libxl__ao_device *device,
+ libxl__ao_device *list, int num, int *last)
+{
+ int i, ret = 0;
+
+ device->state = DEVICE_FINISHED;
+ *last = 1;
+ for (i = 0; i < num; i++) {
+ if (list[i].state != DEVICE_FINISHED && *last) {
+ *last = 0;
+ }
+ if (list[i].rc)
+ ret = list[i].rc;
+ }
+ return ret;
}
-int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
- libxl__device *dev)
+/* Common callbacks for device add/remove */
+
+static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
+ int rc);
+
+void libxl__initiate_device_remove(libxl__egc *egc, libxl__ao_device *aorm)
{
- AO_GC;
+ STATE_AO_GC(aorm->ao);
libxl_ctx *ctx = libxl__gc_owner(gc);
- xs_transaction_t t;
- char *be_path = libxl__device_backend_path(gc, dev);
+ xs_transaction_t t = 0;
+ char *be_path = libxl__device_backend_path(gc, aorm->dev);
char *state_path = libxl__sprintf(gc, "%s/state", be_path);
- char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+ char *state;
int rc = 0;
- libxl__ao_device_remove *aorm = 0;
+retry_transaction:
+ t = xs_transaction_start(ctx->xsh);
+ if (aorm->force)
+ libxl__xs_path_cleanup(gc, t,
+ libxl__device_frontend_path(gc, aorm->dev));
+ /* Remove frontend xs paths to force backend disconnection */
+ state = libxl__xs_read(gc, t, state_path);
if (!state)
goto out_ok;
- if (atoi(state) != 4) {
+ if (atoi(state) == XenbusStateClosed) {
libxl__device_destroy_tapdisk(gc, be_path);
- xs_rm(ctx->xsh, XBT_NULL, be_path);
goto out_ok;
}
-retry_transaction:
- t = xs_transaction_start(ctx->xsh);
- xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", strlen("0"));
+ xs_write(ctx->xsh, t, GCSPRINTF("%s/online", be_path), "0", strlen("0"));
xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
if (!xs_transaction_end(ctx->xsh, t, 0)) {
if (errno == EAGAIN)
@@ -676,60 +739,67 @@ retry_transaction:
goto out_fail;
}
}
+ /* mark transaction as ended, to prevent double closing it on out_ok */
+ t = 0;
libxl__device_destroy_tapdisk(gc, be_path);
- aorm = libxl__zalloc(gc, sizeof(*aorm));
- aorm->ao = ao;
libxl__ev_devstate_init(&aorm->ds);
-
- rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_remove_callback,
+ rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_backend_callback,
state_path, XenbusStateClosed,
LIBXL_DESTROY_TIMEOUT * 1000);
- if (rc) goto out_fail;
+ if (rc) {
+ LOGE(ERROR, "unable to remove device %s", be_path);
+ goto out_fail;
+ }
- return 0;
+ return;
out_fail:
assert(rc);
- device_remove_cleanup(gc, aorm);
- return rc;
+ aorm->rc = rc;
+ aorm->callback(egc, aorm);
+ return;
out_ok:
- libxl__ao_complete(egc, ao, 0);
- return 0;
+ if (t) xs_transaction_end(ctx->xsh, t, 0);
+ aorm->callback(egc, aorm);
+ return;
}
-int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
-{
- libxl_ctx *ctx = libxl__gc_owner(gc);
- char *be_path = libxl__device_backend_path(gc, dev);
- char *fe_path = libxl__device_frontend_path(gc, dev);
-
- xs_rm(ctx->xsh, XBT_NULL, be_path);
- xs_rm(ctx->xsh, XBT_NULL, fe_path);
+/* Callback for device destruction */
- libxl__device_destroy_tapdisk(gc, be_path);
-
- return 0;
-}
+static void device_remove_callback(libxl__egc *egc, libxl__ao_device *aorm);
-int libxl__devices_destroy(libxl__gc *gc, uint32_t domid)
+void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
{
- libxl_ctx *ctx = libxl__gc_owner(gc);
+ STATE_AO_GC(drs->ao);
char *path;
unsigned int num_kinds, num_devs;
char **kinds = NULL, **devs = NULL;
- int i, j;
- libxl__device dev;
+ int i, j, rc = 0, numdev = 0;
+ libxl__device *dev;
libxl__device_kind kind;
- path = libxl__sprintf(gc, "/local/domain/%d/device", domid);
+ drs->num_devices = libxl__num_devices(gc, drs->domid);
+ if (drs->num_devices < 0) {
+ LOGE(ERROR, "unable to get number of devices for domain %u",
+ drs->domid);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ GCNEW_ARRAY(drs->aorm, drs->num_devices);
+ for (i = 0; i < drs->num_devices; i++) {
+ libxl__init_ao_device(&drs->aorm[i], drs->ao, &drs->aorm);
+ }
+
+ path = GCSPRINTF("/local/domain/%d/device", drs->domid);
kinds = libxl__xs_directory(gc, XBT_NULL, path, &num_kinds);
if (!kinds) {
if (errno != ENOENT) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get xenstore"
- " device listing %s", path);
+ LOGE(ERROR, "unable to get xenstore device listing %s", path);
+ rc = ERROR_FAIL;
goto out;
}
num_kinds = 0;
@@ -738,38 +808,136 @@ int libxl__devices_destroy(libxl__gc *gc, uint32_t domid)
if (libxl__device_kind_from_string(kinds[i], &kind))
continue;
- path = libxl__sprintf(gc, "/local/domain/%d/device/%s", domid, kinds[i]);
+ path = GCSPRINTF("/local/domain/%d/device/%s", drs->domid, kinds[i]);
devs = libxl__xs_directory(gc, XBT_NULL, path, &num_devs);
if (!devs)
continue;
for (j = 0; j < num_devs; j++) {
- path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s/backend",
- domid, kinds[i], devs[j]);
+ path = GCSPRINTF("/local/domain/%d/device/%s/%s/backend",
+ drs->domid, kinds[i], devs[j]);
path = libxl__xs_read(gc, XBT_NULL, path);
- if (path && libxl__parse_backend_path(gc, path, &dev) == 0) {
- dev.domid = domid;
- dev.kind = kind;
- dev.devid = atoi(devs[j]);
-
- libxl__device_destroy(gc, &dev);
+ GCNEW(dev);
+ if (path && libxl__parse_backend_path(gc, path, dev) == 0) {
+ dev->domid = drs->domid;
+ dev->kind = kind;
+ dev->devid = atoi(devs[j]);
+ drs->aorm[numdev].action = DEVICE_DISCONNECT;
+ drs->aorm[numdev].dev = dev;
+ drs->aorm[numdev].callback = device_remove_callback;
+ libxl__initiate_device_remove(egc, &drs->aorm[numdev]);
+ numdev++;
}
}
}
/* console 0 frontend directory is not under /local/domain/<domid>/device */
- path = libxl__sprintf(gc, "/local/domain/%d/console/backend", domid);
+ path = GCSPRINTF("/local/domain/%d/console/backend", drs->domid);
path = libxl__xs_read(gc, XBT_NULL, path);
+ GCNEW(dev);
if (path && strcmp(path, "") &&
- libxl__parse_backend_path(gc, path, &dev) == 0) {
- dev.domid = domid;
- dev.kind = LIBXL__DEVICE_KIND_CONSOLE;
- dev.devid = 0;
+ libxl__parse_backend_path(gc, path, dev) == 0) {
+ dev->domid = drs->domid;
+ dev->kind = LIBXL__DEVICE_KIND_CONSOLE;
+ dev->devid = 0;
- libxl__device_destroy(gc, &dev);
+ libxl__device_destroy(gc, dev);
}
out:
- return 0;
+ if (!numdev) drs->callback(egc, drs, rc);
+ return;
+}
+
+static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
+ int rc)
+{
+ libxl__ao_device *aorm = CONTAINER_OF(ds, *aorm, ds);
+ STATE_AO_GC(aorm->ao);
+
+ device_backend_cleanup(gc, aorm);
+
+ if (rc == ERROR_TIMEDOUT && aorm->action == DEVICE_DISCONNECT &&
+ !aorm->force) {
+ aorm->force = 1;
+ libxl__initiate_device_remove(egc, aorm);
+ return;
+ }
+
+ /* Some devices (vkbd) fail to disconnect properly,
+ * but we shouldn't alarm the user if it's during
+ * domain destruction.
+ */
+ if (rc && aorm->action == DEVICE_CONNECT) {
+ LOG(ERROR, "unable to connect device with path %s",
+ libxl__device_backend_path(gc, aorm->dev));
+ goto out;
+ } else if(rc) {
+ LOG(DEBUG, "unable to disconnect device with path %s",
+ libxl__device_backend_path(gc, aorm->dev));
+ goto out;
+ }
+
+out:
+ aorm->rc = rc;
+ aorm->callback(egc, aorm);
+ return;
+}
+
+static void device_remove_callback(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl__devices_remove_state *drs = CONTAINER_OF(aorm->base, *drs, aorm);
+ char *be_path = libxl__device_backend_path(gc, aorm->dev);
+ char *fe_path = libxl__device_frontend_path(gc, aorm->dev);
+ xs_transaction_t t = 0;
+ int rc = 0, last = 1;
+
+retry_transaction:
+ t = xs_transaction_start(CTX->xsh);
+ if (aorm->action == DEVICE_DISCONNECT) {
+ libxl__xs_path_cleanup(gc, t, fe_path);
+ libxl__xs_path_cleanup(gc, t, be_path);
+ }
+ if (!xs_transaction_end(CTX->xsh, t, 0)) {
+ if (errno == EAGAIN)
+ goto retry_transaction;
+ else {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ }
+
+out:
+ rc = libxl__ao_device_check_last(gc, aorm, drs->aorm,
+ drs->num_devices, &last);
+ if (last)
+ drs->callback(egc, drs, rc);
+ return;
+}
+
+int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
+{
+ char *be_path = libxl__device_backend_path(gc, dev);
+ char *fe_path = libxl__device_frontend_path(gc, dev);
+ xs_transaction_t t = 0;
+ int rc = 0;
+
+retry_transaction:
+ t = xs_transaction_start(CTX->xsh);
+ libxl__xs_path_cleanup(gc, t, be_path);
+ libxl__xs_path_cleanup(gc, t, fe_path);
+ if (!xs_transaction_end(CTX->xsh, t, 0)) {
+ if (errno == EAGAIN)
+ goto retry_transaction;
+ else {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ }
+
+out:
+ libxl__device_destroy_tapdisk(gc, be_path);
+ return rc;
}
int libxl__wait_for_device_model(libxl__gc *gc,
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 95fd0ac..4019309 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -673,6 +673,10 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
libxl__dm_spawn_state *stubdom_dmss,
int rc);
+static void spaw_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
+ libxl__destroy_domid_state *dis,
+ int rc);
+
void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
{
STATE_AO_GC(sdss->dm.spawn.ao);
@@ -891,12 +895,30 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
out:
if (rc) {
- if (dm_domid)
- libxl_domain_destroy(CTX, dm_domid);
+ if (dm_domid) {
+ sdss->dis.ao = ao;
+ sdss->dis.domid = dm_domid;
+ sdss->dis.callback = spaw_stubdom_pvqemu_destroy_cb;
+ libxl__destroy_domid(egc, &sdss->dis);
+ }
}
sdss->callback(egc, &sdss->dm, rc);
}
+static void spaw_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
+ libxl__destroy_domid_state *dis,
+ int rc)
+{
+ libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(dis, *sdss, dis);
+ STATE_AO_GC(sdss->dis.ao);
+
+ if (rc)
+ LOG(ERROR, "destruction of domain %u after failed creation failed",
+ sdss->pvqemu.guest_domid);
+
+ sdss->callback(egc, &sdss->dm, rc);
+}
+
/* callbacks passed to libxl__spawn_spawn */
static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
const char *xsdata);
@@ -1089,48 +1111,25 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
int ret;
pid = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/%d/image/device-model-pid", domid));
- if (!pid) {
- int stubdomid = libxl_get_stubdom_id(ctx, domid);
- const char *savefile;
-
- if (!stubdomid) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't find device model's pid");
- ret = ERROR_INVAL;
- goto out;
- }
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device model is a stubdom, domid=%d", stubdomid);
- ret = libxl_domain_destroy(ctx, stubdomid);
- if (ret)
- goto out;
-
- savefile = libxl__device_model_savefile(gc, domid);
- ret = unlink(savefile);
- /*
- * On suspend libxl__domain_save_device_model will have already
- * unlinked the save file.
- */
- if (ret && errno == ENOENT) ret = 0;
- if (ret) {
- LIBXL__LOG_ERRNO(ctx, XTL_ERROR,
- "failed to remove device-model savefile %s\n",
- savefile);
- goto out;
- }
+ if (!pid || !atoi(pid)) {
+ LOGE(ERROR, "Couldn't find device model's pid");
+ ret = ERROR_INVAL;
+ goto out;
+ }
+ ret = kill(atoi(pid), SIGHUP);
+ if (ret < 0 && errno == ESRCH) {
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device Model already exited");
+ ret = 0;
+ } else if (ret == 0) {
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device Model signaled");
+ ret = 0;
} else {
- ret = kill(atoi(pid), SIGHUP);
- if (ret < 0 && errno == ESRCH) {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device Model already exited");
- ret = 0;
- } else if (ret == 0) {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device Model signaled");
- ret = 0;
- } else {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to kill Device Model [%d]",
- atoi(pid));
- ret = ERROR_FAIL;
- goto out;
- }
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to kill Device Model [%d]",
+ atoi(pid));
+ ret = ERROR_FAIL;
+ goto out;
}
+
xs_rm(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "/local/domain/0/device-model/%d", domid));
xs_rm(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "/local/domain/%d/hvmloader", domid));
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 6064d44..1e73f64 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1125,6 +1125,176 @@ out:
return rc;
}
+/* Callbacks for libxl__domain_destroy */
+static void stubdom_callback(libxl__egc *egc, libxl__destroy_domid_state *dis,
+ int rc);
+static void domain_callback(libxl__egc *egc, libxl__destroy_domid_state *dis,
+ int rc);
+
+void libxl__domain_destroy(libxl__egc *egc, libxl__domain_destroy_state *dds)
+{
+ STATE_AO_GC(dds->ao);
+ uint32_t stubdomid = libxl_get_stubdom_id(CTX, dds->domid);
+
+ if (stubdomid) {
+ dds->stubdom.ao = ao;
+ dds->stubdom.domid = stubdomid;
+ dds->stubdom.callback = stubdom_callback;
+ libxl__destroy_domid(egc, &dds->stubdom);
+ } else {
+ dds->stubdom_finished = 1;
+ }
+
+ dds->domain.ao = ao;
+ dds->domain.domid = dds->domid;
+ dds->domain.callback = domain_callback;
+ libxl__destroy_domid(egc, &dds->domain);
+}
+
+static void stubdom_callback(libxl__egc *egc, libxl__destroy_domid_state *dis,
+ int rc)
+{
+ STATE_AO_GC(dis->ao);
+ libxl__domain_destroy_state *dds = CONTAINER_OF(dis, *dds, stubdom);
+ const char *savefile;
+
+ if (rc)
+ LOG(ERROR, "unable to destroy stubdom with domid %u", dis->domid);
+
+ dds->stubdom_finished = 1;
+ savefile = libxl__device_model_savefile(gc, dis->domid);
+ rc = unlink(savefile);
+ /*
+ * On suspend libxl__domain_save_device_model will have already
+ * unlinked the save file.
+ */
+ if (rc && errno == ENOENT) rc = 0;
+ if (rc) {
+ LOGEV(ERROR, errno, "failed to remove device-model savefile %s",
+ savefile);
+ }
+
+ if (dds->domain_finished)
+ dds->callback(egc, dds, rc);
+}
+
+static void domain_callback(libxl__egc *egc, libxl__destroy_domid_state *dis,
+ int rc)
+{
+ STATE_AO_GC(dis->ao);
+ libxl__domain_destroy_state *dds = CONTAINER_OF(dis, *dds, domain);
+
+ if (rc)
+ LOG(ERROR, "unable to destroy guest with domid %u", dis->domid);
+
+ dds->domain_finished = 1;
+ if (dds->stubdom_finished)
+ dds->callback(egc, dds, rc);
+}
+
+/* Callbacks for libxl__domain_clean */
+
+/* Device destruction callback */
+static void devices_destroy_cb(libxl__egc *egc,
+ libxl__devices_remove_state *drs,
+ int rc);
+
+void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
+{
+ STATE_AO_GC(dis->ao);
+ int rc;
+
+ rc = libxl_domain_info(CTX, NULL, dis->domid);
+ switch (rc) {
+ case 0:
+ break;
+ case ERROR_INVAL:
+ LOGEV(ERROR, rc, "non-existant domain %d", dis->domid);
+ default:
+ goto out;
+ }
+
+ switch (libxl__domain_type(gc, dis->domid)) {
+ case LIBXL_DOMAIN_TYPE_HVM:
+ dis->dm_present = 1;
+ break;
+ case LIBXL_DOMAIN_TYPE_PV:
+ dis->pid = libxl__xs_read(gc, XBT_NULL,
+ GCSPRINTF("/local/domain/%d/image/device-model-pid",
+ dis->domid));
+ dis->dm_present = (dis->pid != NULL);
+ break;
+ default:
+ abort();
+ }
+
+ dis->dom_path = libxl__xs_get_dompath(gc, dis->domid);
+ if (!dis->dom_path) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ if (libxl__device_pci_destroy_all(gc, dis->domid) < 0)
+ LOG(ERROR, "pci shutdown failed for domid %d", dis->domid);
+ rc = xc_domain_pause(CTX->xch, dis->domid);
+ if (rc < 0) {
+ LOGEV(ERROR, rc, "xc_domain_pause failed for %d", dis->domid);
+ }
+ if (dis->dm_present) {
+ rc = libxl__destroy_device_model(gc, dis->domid);
+ if (rc < 0)
+ LOG(ERROR, "libxl__destroy_device_model failed for %d",
+ dis->domid);
+ libxl__qmp_cleanup(gc, dis->domid);
+ }
+ dis->drs.ao = ao;
+ dis->drs.domid = dis->domid;
+ dis->drs.callback = devices_destroy_cb;
+ libxl__devices_destroy(egc, &dis->drs);
+ return;
+
+out:
+ assert(rc);
+ dis->callback(egc, dis, rc);
+ return;
+}
+
+static void devices_destroy_cb(libxl__egc *egc,
+ libxl__devices_remove_state *drs,
+ int rc)
+{
+ STATE_AO_GC(drs->ao);
+ libxl__destroy_domid_state *dis = CONTAINER_OF(drs, *dis, drs);
+
+ if (rc < 0)
+ LOG(DEBUG, "libxl__devices_destroy failed for %d", dis->domid);
+
+ dis->vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/vm",
+ dis->dom_path));
+ if (dis->vm_path)
+ if (!xs_rm(CTX->xsh, XBT_NULL, dis->vm_path))
+ LOG(ERROR, "xs_rm failed for %s", dis->vm_path);
+
+ if (!xs_rm(CTX->xsh, XBT_NULL, dis->dom_path))
+ LOG(ERROR, "xs_rm failed for %s", dis->dom_path);
+
+ xs_rm(CTX->xsh, XBT_NULL, libxl__xs_libxl_path(gc, dis->domid));
+
+ libxl__userdata_destroyall(gc, dis->domid);
+
+ rc = xc_domain_destroy(CTX->xch, dis->domid);
+ if (rc < 0) {
+ LOGEV(ERROR, rc, "xc_domain_destroy failed for %d", dis->domid);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ rc = 0;
+
+out:
+ dis->callback(egc, dis, rc);
+ return;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 0ddfe72..e324da2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -807,7 +807,6 @@ _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
_hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
libxl__device *dev);
_hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
-_hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid);
_hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
/*
@@ -838,13 +837,6 @@ _hidden const char *libxl__device_nic_devname(libxl__gc *gc,
uint32_t devid,
libxl_nic_type type);
-/* Arranges that dev will be removed from its guest. When
- * this is done, the ao will be completed. An error
- * return from libxl__initiate_device_remove means that the ao
- * will _not_ be completed and the caller must do so. */
-_hidden int libxl__initiate_device_remove(libxl__egc*, libxl__ao*,
- libxl__device *dev);
-
/*
* libxl__ev_devstate - waits a given time for a device to
* reach a given state. Follows the libxl_ev_* conventions.
@@ -1085,43 +1077,6 @@ static inline int libxl__spawn_inuse(libxl__spawn_state *ss)
_hidden int libxl__spawn_record_pid(libxl__gc*, libxl__spawn_state*,
pid_t innerchild);
-/*----- device model creation -----*/
-
-/* First layer; wraps libxl__spawn_spawn. */
-
-typedef struct libxl__dm_spawn_state libxl__dm_spawn_state;
-
-typedef void libxl__dm_spawn_cb(libxl__egc *egc, libxl__dm_spawn_state*,
- int rc /* if !0, error was logged */);
-
-struct libxl__dm_spawn_state {
- /* mixed - spawn.ao must be initialised by user; rest is private: */
- libxl__spawn_state spawn;
- /* filled in by user, must remain valid: */
- uint32_t guest_domid; /* domain being served */
- libxl_domain_config *guest_config;
- libxl__domain_build_state *build_state; /* relates to guest_domid */
- libxl__dm_spawn_cb *callback;
-};
-
-_hidden void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state*);
-
-/* Stubdom device models. */
-
-typedef struct {
- /* Mixed - user must fill in public parts EXCEPT callback,
- * which may be undefined on entry. (See above for details) */
- libxl__dm_spawn_state dm; /* the stub domain device model */
- /* filled in by user, must remain valid: */
- libxl__dm_spawn_cb *callback; /* called as callback(,&sdss->dm,) */
- /* private to libxl__spawn_stub_dm: */
- libxl_domain_config dm_config;
- libxl__domain_build_state dm_state;
- libxl__dm_spawn_state pvqemu;
-} libxl__stub_dm_spawn_state;
-
-_hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
-
/*
* libxl__wait_for_offspring - Wait for child state
@@ -1813,6 +1768,183 @@ _hidden void libxl__bootloader_init(libxl__bootloader_state *bl);
* If callback is passed rc==0, will have updated st->info appropriately */
_hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
+/*----- device addition/removal -----*/
+
+/* During the init/destruction process, the device can be in several states:
+ *
+ * DEVICE_UNKNOWN: device in unknown state (default value when struct is
+ * initialized).
+ *
+ * DEVICE_WAIT_BACKEND: waiting for the backend to switch to XenbusStateInit or
+ * XenbusStateClosed.
+ *
+ * DEVICE_WAIT_HOTPLUG: waiting for hotplug script to finish execution.
+ *
+ * DEVICE_FINISHED: device is connected/disconnected.
+ */
+typedef enum {
+ DEVICE_ACTIVE,
+ DEVICE_FINISHED
+} libxl__device_state;
+
+/* Action to perform (either connect or disconnect) */
+typedef enum {
+ DEVICE_CONNECT,
+ DEVICE_DISCONNECT
+} libxl__device_action;
+
+typedef struct libxl__ao_device libxl__ao_device;
+typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*);
+
+_hidden void libxl__init_ao_device(libxl__ao_device *aorm, libxl__ao *ao,
+ libxl__ao_device **base);
+_hidden int libxl__ao_device_check_last(libxl__gc *gc, libxl__ao_device *device,
+ libxl__ao_device *list,
+ int num, int *last);
+
+struct libxl__ao_device {
+ libxl__ao *ao;
+ /* State in which the device is */
+ libxl__device_state state;
+ /* action being performed */
+ libxl__device_action action;
+ libxl__device *dev;
+ int force;
+ libxl__device_callback *callback;
+ /* private for implementation */
+ int rc;
+ libxl__ev_devstate ds;
+ void *base;
+};
+
+/* Arranges that dev will be removed to the guest, and the
+ * hotplug scripts will be executed (if necessary). When
+ * this is done (or an error happens), the callback in
+ * aorm->callback will be called.
+ */
+_hidden void libxl__initiate_device_remove(libxl__egc *egc,
+ libxl__ao_device *aorm);
+
+/*----- Domain destruction -----*/
+
+/* Domain destruction has been splitted in two functions:
+ *
+ * libxl__domain_destroy is the main destroy function, it detects
+ * stubdoms and calls libxl__destroy_domid on the domain and it's
+ * stubdom if present, creating a different libxl__destroy_domid_state
+ * for each one of them.
+ *
+ * libxl__destroy_domid actually destroys the domain, but it
+ * doesn't check for stubdomains, since that would involve
+ * recursion, which we want to avoid.
+ */
+
+typedef struct libxl__domain_destroy_state libxl__domain_destroy_state;
+typedef struct libxl__destroy_domid_state libxl__destroy_domid_state;
+typedef struct libxl__devices_remove_state libxl__devices_remove_state;
+
+typedef void libxl__domain_destroy_cb(libxl__egc *egc,
+ libxl__domain_destroy_state *dds,
+ int rc);
+
+typedef void libxl__domid_destroy_cb(libxl__egc *egc,
+ libxl__destroy_domid_state *dis,
+ int rc);
+
+typedef void libxl__devices_remove_callback(libxl__egc *egc,
+ libxl__devices_remove_state *drs,
+ int rc);
+
+struct libxl__devices_remove_state {
+ /* filled in by user */
+ libxl__ao *ao;
+ uint32_t domid;
+ libxl__devices_remove_callback *callback;
+ /* private */
+ libxl__ao_device *aorm;
+ int num_devices;
+};
+
+struct libxl__destroy_domid_state {
+ /* filled in by user */
+ libxl__ao *ao;
+ uint32_t domid;
+ libxl__domid_destroy_cb *callback;
+ /* private to implementation */
+ libxl__devices_remove_state drs;
+ char *dom_path;
+ char *vm_path;
+ char *pid;
+ int dm_present;
+};
+
+struct libxl__domain_destroy_state {
+ /* filled by the user */
+ libxl__ao *ao;
+ uint32_t domid;
+ libxl__domain_destroy_cb *callback;
+ /* Private */
+ uint32_t stubdomid;
+ libxl__destroy_domid_state stubdom;
+ int stubdom_finished;
+ libxl__destroy_domid_state domain;
+ int domain_finished;
+};
+
+/*
+ * Entry point for domain destruction
+ * This function checks for stubdom presence and then calls
+ * libxl__destroy_domid on the passed domain and it's stubdom if found.
+ */
+_hidden void libxl__domain_destroy(libxl__egc *egc,
+ libxl__domain_destroy_state *dds);
+
+/* Used to destroy a domain with the passed id (it doesn't check for stubs) */
+_hidden void libxl__destroy_domid(libxl__egc *egc,
+ libxl__destroy_domid_state *dis);
+
+/* Entry point for devices destruction */
+_hidden void libxl__devices_destroy(libxl__egc *egc,
+ libxl__devices_remove_state *drs);
+
+/*----- device model creation -----*/
+
+/* First layer; wraps libxl__spawn_spawn. */
+
+typedef struct libxl__dm_spawn_state libxl__dm_spawn_state;
+
+typedef void libxl__dm_spawn_cb(libxl__egc *egc, libxl__dm_spawn_state*,
+ int rc /* if !0, error was logged */);
+
+struct libxl__dm_spawn_state {
+ /* mixed - spawn.ao must be initialised by user; rest is private: */
+ libxl__spawn_state spawn;
+ /* filled in by user, must remain valid: */
+ uint32_t guest_domid; /* domain being served */
+ libxl_domain_config *guest_config;
+ libxl__domain_build_state *build_state; /* relates to guest_domid */
+ libxl__dm_spawn_cb *callback;
+};
+
+_hidden void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state*);
+
+/* Stubdom device models. */
+
+typedef struct {
+ /* Mixed - user must fill in public parts EXCEPT callback,
+ * which may be undefined on entry. (See above for details) */
+ libxl__dm_spawn_state dm; /* the stub domain device model */
+ /* filled in by user, must remain valid: */
+ libxl__dm_spawn_cb *callback; /* called as callback(,&sdss->dm,) */
+ /* private to libxl__spawn_stub_dm: */
+ libxl_domain_config dm_config;
+ libxl__domain_build_state dm_state;
+ libxl__dm_spawn_state pvqemu;
+ libxl__destroy_domid_state dis;
+} libxl__stub_dm_spawn_state;
+
+_hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
+
/*----- Domain creation -----*/
typedef struct libxl__domain_create_state libxl__domain_create_state;
@@ -1835,6 +1967,8 @@ struct libxl__domain_create_state {
libxl__stub_dm_spawn_state dmss;
/* If we're not doing stubdom, we use only dmss.dm,
* for the non-stubdom device model. */
+ /* necessary if the domain creation failed and we have to destroy it */
+ libxl__domain_destroy_state dds;
};
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index efaf3de..3c02b69 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1333,7 +1333,7 @@ static int handle_domain_death(libxl_ctx *ctx, uint32_t domid,
/* fall-through */
case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
LOG("Domain %d needs to be cleaned up: destroying the domain", domid);
- libxl_domain_destroy(ctx, domid);
+ libxl_domain_destroy(ctx, domid, 0);
break;
case LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY:
@@ -1899,7 +1899,7 @@ start:
error_out:
release_lock();
if (libxl_domid_valid_guest(domid))
- libxl_domain_destroy(ctx, domid);
+ libxl_domain_destroy(ctx, domid, 0);
out:
if (logfile != 2)
@@ -2390,7 +2390,7 @@ static void destroy_domain(const char *p)
fprintf(stderr, "Cannot destroy privileged domain 0.\n\n");
exit(-1);
}
- rc = libxl_domain_destroy(ctx, domid);
+ rc = libxl_domain_destroy(ctx, domid, 0);
if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc); exit(-1); }
}
@@ -2664,7 +2664,7 @@ static int save_domain(const char *p, const char *filename, int checkpoint,
if (checkpoint)
libxl_domain_unpause(ctx, domid);
else
- libxl_domain_destroy(ctx, domid);
+ libxl_domain_destroy(ctx, domid, 0);
exit(0);
}
@@ -2896,7 +2896,7 @@ static void migrate_domain(const char *domain_spec, const char *rune,
}
fprintf(stderr, "migration sender: Target reports successful startup.\n");
- libxl_domain_destroy(ctx, domid); /* bang! */
+ libxl_domain_destroy(ctx, domid, 0); /* bang! */
fprintf(stderr, "Migration successful.\n");
exit(0);
@@ -3014,7 +3014,7 @@ static void migrate_receive(int debug, int daemonize, int monitor)
if (rc) {
fprintf(stderr, "migration target: Failure, destroying our copy.\n");
- rc2 = libxl_domain_destroy(ctx, domid);
+ rc2 = libxl_domain_destroy(ctx, domid, 0);
if (rc2) {
fprintf(stderr, "migration target: Failed to destroy our copy"
" (code %d).\n", rc2);
diff --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowlevel/xl/xl.c
index c4f7c52..e144670 100644
--- a/tools/python/xen/lowlevel/xl/xl.c
+++ b/tools/python/xen/lowlevel/xl/xl.c
@@ -447,7 +447,7 @@ static PyObject *pyxl_domain_destroy(XlObject *self, PyObject *args)
int domid;
if ( !PyArg_ParseTuple(args, "i", &domid) )
return NULL;
- if ( libxl_domain_destroy(self->ctx, domid) ) {
+ if ( libxl_domain_destroy(self->ctx, domid, 0) ) {
PyErr_SetString(xl_error_obj, "cannot destroy domain");
return NULL;
}
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 08/13] libxl: convert libxl_device_disk_add to an async operation
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (6 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:33 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 09/13] libxl: convert libxl_device_nic_add " Roger Pau Monne
` (4 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
This patch converts libxl_device_disk_add to an ao operation that
waits for device backend to reach state XenbusStateInitWait and then
marks the operation as completed. This is not really useful now, but
will be used by latter patches that will launch hotplug scripts after
we reached the desired xenbus state.
As usual, libxl_device_disk_add callers have been modified, and the
internal function libxl__device_disk_add has been used if the call was
inside an already running ao.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl.c | 22 +++++++++-----
tools/libxl/libxl.h | 4 ++-
tools/libxl/libxl_create.c | 51 ++++++++++++++++++++++++++++----
tools/libxl/libxl_device.c | 66 ++++++++++++++++++++++++++++++++++++------
tools/libxl/libxl_dm.c | 62 +++++++++++++++++++++++++++++++--------
tools/libxl/libxl_internal.h | 21 ++++++++++++-
tools/libxl/xl_cmdimpl.c | 2 +-
7 files changed, 187 insertions(+), 41 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index dabe92b..5edfbdc 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1255,15 +1255,19 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
return rc;
}
-int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
+int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_disk *disk,
+ const libxl_asyncop_how *ao_how)
{
- GC_INIT(ctx);
- int rc;
+ AO_CREATE(ctx, domid, ao_how);
+ libxl__ao_device *device;
- rc = libxl__device_disk_add(gc, domid, disk);
+ GCNEW(device);
+ libxl__init_ao_device(device, ao, NULL);
+ device->callback = libxl__device_cb;
+ libxl__device_disk_add(egc, domid, disk, device);
- GC_FREE;
- return rc;
+ return AO_INPROGRESS;
}
int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
@@ -1508,11 +1512,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
ret = 0;
libxl_device_disk_remove(ctx, domid, disks + i, 0);
- libxl_device_disk_add(ctx, domid, disk);
+ /* fixme-ao */
+ libxl_device_disk_add(ctx, domid, disk, 0);
stubdomid = libxl_get_stubdom_id(ctx, domid);
if (stubdomid) {
libxl_device_disk_remove(ctx, stubdomid, disks + i, 0);
- libxl_device_disk_add(ctx, stubdomid, disk);
+ /* fixme-ao */
+ libxl_device_disk_add(ctx, stubdomid, disk, 0);
}
out:
for (i = 0; i < num; i++)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 2f4694e..f76b2e6 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -663,7 +663,9 @@ void libxl_vminfo_list_free(libxl_vminfo *list, int nr);
*/
/* Disks */
-int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
+int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_disk *disk,
+ const libxl_asyncop_how *ao_how);
int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
const libxl_asyncop_how *ao_how);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bd8e6a6..9a65c45 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -575,6 +575,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
libxl__bootloader_state *bl,
int rc);
+static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
static void domcreate_console_available(libxl__egc *egc,
libxl__domain_create_state *dcs);
@@ -705,15 +707,50 @@ static void domcreate_bootloader_done(libxl__egc *egc,
store_libxl_entry(gc, domid, &d_config->b_info);
+ GCNEW_ARRAY(dcs->devices, d_config->num_disks);
+ dcs->num_devices = d_config->num_disks;
for (i = 0; i < d_config->num_disks; i++) {
- ret = libxl_device_disk_add(ctx, domid, &d_config->disks[i]);
- if (ret) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "cannot add disk %d to domain: %d", i, ret);
- ret = ERROR_FAIL;
- goto error_out;
- }
+ libxl__init_ao_device(&dcs->devices[i], ao, &dcs->devices);
+ dcs->devices[i].callback = domcreate_disk_connected;
+ libxl__device_disk_add(egc, domid, &d_config->disks[i],
+ &dcs->devices[i]);
}
+ return;
+
+ error_out:
+ assert(ret);
+ domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl__domain_create_state *dcs = CONTAINER_OF(aorm->base, *dcs, devices);
+ int i, last, ret = 0;
+
+ /* convenience aliases */
+ const uint32_t domid = dcs->guest_domid;
+ libxl_domain_config *const d_config = dcs->guest_config;
+ libxl__domain_build_state *const state = &dcs->build_state;
+ libxl_ctx *const ctx = CTX;
+
+ ret = libxl__ao_device_check_last(gc, aorm, dcs->devices,
+ dcs->num_devices, &last);
+ if (!last) return;
+ if (last && ret) {
+ LOGE(ERROR, "error connecting disk devices");
+ goto error_out;
+ }
+
+ /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm.
+ * Fill in any field required by either, including both relevant
+ * callbacks (_spawn_stub_dm will overwrite our trespass if needed). */
+ dcs->dmss.dm.spawn.ao = ao;
+ dcs->dmss.dm.guest_config = dcs->guest_config;
+ dcs->dmss.dm.build_state = &dcs->build_state;
+ dcs->dmss.dm.callback = domcreate_devmodel_started;
+ dcs->dmss.callback = domcreate_devmodel_started;
+
for (i = 0; i < d_config->num_vifs; i++) {
ret = libxl_device_nic_add(ctx, domid, &d_config->vifs[i]);
if (ret) {
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 577324c..96e6ec4 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -329,13 +329,15 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
return 0;
}
-int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
- libxl_device_disk *disk)
+void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_disk *disk,
+ libxl__ao_device *aorm)
{
+ STATE_AO_GC(aorm->ao);
flexarray_t *front;
flexarray_t *back;
char *dev, *format;
- libxl__device device;
+ libxl__device *device;
int major, minor, rc;
rc = libxl__device_disk_setdefault(gc, disk);
@@ -358,7 +360,8 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
goto out_free;
}
- rc = libxl__device_from_disk(gc, domid, disk, &device);
+ GCNEW(device);
+ rc = libxl__device_from_disk(gc, domid, disk, device);
if (rc != 0) {
LOG(ERROR, "Invalid or unsupported virtual disk identifier %s",
disk->vdev);
@@ -377,7 +380,7 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
flexarray_append(back, "params");
flexarray_append(back, dev);
- assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
+ assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
break;
case LIBXL_DISK_BACKEND_TAP:
dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
@@ -395,7 +398,7 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
case LIBXL_DISK_BACKEND_QDISK:
flexarray_append(back, "params");
flexarray_append(back, GCSPRINTF("%s:%s", format, disk->pdev_path));
- assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
+ assert(device->backend_kind == LIBXL__DEVICE_KIND_QDISK);
break;
default:
LOG(ERROR, "unrecognized disk backend type: %d\n", disk->backend);
@@ -427,21 +430,27 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
flexarray_append(front, "state");
flexarray_append(front, GCSPRINTF("%d", 1));
flexarray_append(front, "virtual-device");
- flexarray_append(front, GCSPRINTF("%d", device.devid));
+ flexarray_append(front, GCSPRINTF("%d", device->devid));
flexarray_append(front, "device-type");
flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
- libxl__device_generic_add(gc, &device,
+ libxl__device_generic_add(gc, device,
libxl__xs_kvs_of_flexarray(gc, back, back->count),
libxl__xs_kvs_of_flexarray(gc, front, front->count));
+ aorm->dev = device;
+ aorm->action = DEVICE_CONNECT;
+ libxl__initiate_device_add(egc, aorm);
+
rc = 0;
out_free:
flexarray_free(back);
flexarray_free(front);
out:
- return rc;
+ aorm->rc = rc;
+ if (aorm->rc) aorm->callback(egc,aorm);
+ return;
}
int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
@@ -705,6 +714,45 @@ int libxl__ao_device_check_last(libxl__gc *gc, libxl__ao_device *device,
static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
int rc);
+void libxl__initiate_device_add(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ char *be_path = libxl__device_backend_path(gc, aorm->dev);
+ char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+ char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+ int rc = 0;
+
+ if (aorm->dev->backend_kind == LIBXL__DEVICE_KIND_QDISK) {
+ aorm->callback(egc, aorm);
+ return;
+ }
+
+ if (atoi(state) == XenbusStateInitWait)
+ goto out_ok;
+
+ libxl__ev_devstate_init(&aorm->ds);
+ rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_backend_callback,
+ state_path, XenbusStateInitWait,
+ LIBXL_INIT_TIMEOUT * 1000);
+ if (rc) {
+ LOGE(ERROR, "unable to initialize device %s", be_path);
+ goto out_fail;
+ }
+
+ return;
+
+out_fail:
+ assert(rc);
+ aorm->rc = rc;
+ aorm->callback(egc, aorm);
+ return;
+
+out_ok:
+ assert(!rc);
+ aorm->callback(egc, aorm);
+ return;
+}
+
void libxl__initiate_device_remove(libxl__egc *egc, libxl__ao_device *aorm)
{
STATE_AO_GC(aorm->ao);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4019309..2b63a15 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -673,6 +673,8 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
libxl__dm_spawn_state *stubdom_dmss,
int rc);
+static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
static void spaw_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
libxl__destroy_domid_state *dis,
int rc);
@@ -681,8 +683,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
{
STATE_AO_GC(sdss->dm.spawn.ao);
libxl_ctx *ctx = libxl__gc_owner(gc);
- int i, num_console = STUBDOM_SPECIAL_CONSOLES, ret;
- libxl__device_console *console;
+ int i, ret;
libxl_device_vfb vfb;
libxl_device_vkb vkb;
char **args;
@@ -800,22 +801,60 @@ retry_transaction:
if (errno == EAGAIN)
goto retry_transaction;
+ GCNEW_ARRAY(sdss->devices, dm_config->num_disks);
+ sdss->num_devices = dm_config->num_disks;
for (i = 0; i < dm_config->num_disks; i++) {
- ret = libxl_device_disk_add(ctx, dm_domid, &dm_config->disks[i]);
- if (ret)
- goto out_free;
+ libxl__init_ao_device(&sdss->devices[i], ao, &sdss->devices);
+ sdss->devices[i].callback = spawn_stub_disk_connected;
+ libxl__device_disk_add(egc, dm_domid, &dm_config->disks[i],
+ &sdss->devices[i]);
+ }
+
+ free(args);
+ return;
+
+out_free:
+ free(args);
+out:
+ assert(ret);
+ spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
+}
+
+static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(aorm->base, *sdss, devices);
+ int i, num_console = STUBDOM_SPECIAL_CONSOLES, ret = 0, last;
+ libxl__device_console *console;
+
+ /* convenience aliases */
+ libxl_domain_config *const dm_config = &sdss->dm_config;
+ libxl_domain_config *const guest_config = sdss->dm.guest_config;
+ const int guest_domid = sdss->dm.guest_domid;
+ libxl__domain_build_state *const d_state = sdss->dm.build_state;
+ libxl__domain_build_state *const stubdom_state = &sdss->dm_state;
+ uint32_t dm_domid = sdss->pvqemu.guest_domid;
+
+ ret = libxl__ao_device_check_last(gc, aorm, sdss->devices,
+ sdss->num_devices, &last);
+ if (!last) return;
+ if (last && ret) {
+ LOGE(ERROR, "error connecting disk devices");
+ goto out;
}
+
for (i = 0; i < dm_config->num_vifs; i++) {
ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->vifs[i]);
if (ret)
- goto out_free;
+ goto out;
}
ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
if (ret)
- goto out_free;
+ goto out;
ret = libxl_device_vkb_add(ctx, dm_domid, &dm_config->vkbs[0]);
if (ret)
- goto out_free;
+ goto out;
if (guest_config->b_info.u.hvm.serial)
num_console++;
@@ -823,7 +862,7 @@ retry_transaction:
console = libxl__calloc(gc, num_console, sizeof(libxl__device_console));
if (!console) {
ret = ERROR_NOMEM;
- goto out_free;
+ goto out;
}
for (i = 0; i < num_console; i++) {
@@ -859,7 +898,7 @@ retry_transaction:
ret = libxl__device_console_add(gc, dm_domid, &console[i],
i == STUBDOM_CONSOLE_LOGGING ? stubdom_state : NULL);
if (ret)
- goto out_free;
+ goto out;
}
sdss->pvqemu.guest_domid = dm_domid;
@@ -869,11 +908,8 @@ retry_transaction:
libxl__spawn_local_dm(egc, &sdss->pvqemu);
- free(args);
return;
-out_free:
- free(args);
out:
assert(ret);
spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e324da2..2500b86 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -70,6 +70,7 @@
#include "_libxl_types_internal.h"
#include "_libxl_types_internal_json.h"
+#define LIBXL_INIT_TIMEOUT 10
#define LIBXL_DESTROY_TIMEOUT 10
#define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
#define LIBXL_XENCONSOLE_LIMIT 1048576
@@ -783,8 +784,6 @@ _hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*);
_hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
libxl_device_disk *disk,
libxl__device *device);
-_hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
- libxl_device_disk *disk);
_hidden int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
libxl_device_nic *nic,
@@ -1817,6 +1816,18 @@ struct libxl__ao_device {
void *base;
};
+/* Internal AO operation to connect a disk device */
+_hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_disk *disk,
+ libxl__ao_device *aorm);
+
+/* Arranges that dev will be added to the guest, and the
+ * hotplug scripts will be executed (if necessary). When
+ * this is done (or an error happens), the callback in
+ * aorm->callback will be called.
+ */
+_hidden void libxl__initiate_device_add(libxl__egc*, libxl__ao_device *aorm);
+
/* Arranges that dev will be removed to the guest, and the
* hotplug scripts will be executed (if necessary). When
* this is done (or an error happens), the callback in
@@ -1941,6 +1952,9 @@ typedef struct {
libxl__domain_build_state dm_state;
libxl__dm_spawn_state pvqemu;
libxl__destroy_domid_state dis;
+ /* used to store the state of devices being connected */
+ libxl__ao_device *devices;
+ int num_devices;
} libxl__stub_dm_spawn_state;
_hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
@@ -1969,6 +1983,9 @@ struct libxl__domain_create_state {
* for the non-stubdom device model. */
/* necessary if the domain creation failed and we have to destroy it */
libxl__domain_destroy_state dds;
+ /* used to store the state of devices being connected */
+ libxl__ao_device *devices;
+ int num_devices;
};
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3c02b69..69ce45a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5081,7 +5081,7 @@ int main_blockattach(int argc, char **argv)
return 0;
}
- if (libxl_device_disk_add(ctx, fe_domid, &disk)) {
+ if (libxl_device_disk_add(ctx, fe_domid, &disk, 0)) {
fprintf(stderr, "libxl_device_disk_add failed.\n");
}
return 0;
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 09/13] libxl: convert libxl_device_nic_add to an async operation
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (7 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 08/13] libxl: convert libxl_device_disk_add to an async operation Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:38 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 10/13] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
` (3 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
This patch converts libxl_device_nic_add to an ao operation that
waits for device backend to reach state XenbusStateInitWait and then
marks the operation as completed. This is not really useful now, but
will be used by latter patches that will launch hotplug scripts after
we reached the desired xenbus state.
Calls to libxl_device_nic_add have also been moved to occur after the
device model has been launched, so when hotplug scripts are called
from this functions the interfaces already exists.
As usual, libxl_device_nic_add callers have been modified, and the
internal function libxl__device_disk_add has been used if the call was
inside an already running ao.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl.c | 15 +++++---
tools/libxl/libxl.h | 3 +-
tools/libxl/libxl_create.c | 73 ++++++++++++++++++++++++++++++++++++++----
tools/libxl/libxl_device.c | 23 ++++++++++---
tools/libxl/libxl_dm.c | 58 +++++++++++++++++++++++++++++++--
tools/libxl/libxl_internal.h | 7 +++-
tools/libxl/xl_cmdimpl.c | 2 +-
7 files changed, 155 insertions(+), 26 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 5edfbdc..2490138 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1641,15 +1641,18 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
return 0;
}
-int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
+int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic,
+ const libxl_asyncop_how *ao_how)
{
- GC_INIT(ctx);
- int rc;
+ AO_CREATE(ctx, domid, ao_how);
+ libxl__ao_device *device;
- rc = libxl__device_nic_add(gc, domid, nic);
+ GCNEW(device);
+ libxl__init_ao_device(device, ao, NULL);
+ device->callback = libxl__device_cb;
+ libxl__device_nic_add(egc, domid, nic, device);
- GC_FREE;
- return rc;
+ return AO_INPROGRESS;
}
int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index f76b2e6..fc8a1a1 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -690,7 +690,8 @@ char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk);
int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
/* Network Interfaces */
-int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
+int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic,
+ const libxl_asyncop_how *ao_how);
int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_nic *nic,
const libxl_asyncop_how *ao_how);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9a65c45..2134645 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -577,6 +577,11 @@ static void domcreate_bootloader_done(libxl__egc *egc,
static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm);
+static void domcreate_nics_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
+static void domcreate_attach_pci(libxl__egc *egc,
+ libxl__domain_create_state *dcs);
+
static void domcreate_console_available(libxl__egc *egc,
libxl__domain_create_state *dcs);
@@ -752,13 +757,11 @@ static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
dcs->dmss.callback = domcreate_devmodel_started;
for (i = 0; i < d_config->num_vifs; i++) {
- ret = libxl_device_nic_add(ctx, domid, &d_config->vifs[i]);
- if (ret) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "cannot add nic %d to domain: %d", i, ret);
- ret = ERROR_FAIL;
- goto error_out;
- }
+ /* We have to init the nic here, because we still haven't
+ * called libxl_device_nic_add at this point, but qemu needs
+ * the nic information to be complete.
+ */
+ libxl__device_nic_setdefault(gc, &d_config->vifs[i]);
}
switch (d_config->c_info.type) {
case LIBXL_DOMAIN_TYPE_HVM:
@@ -851,6 +854,62 @@ static void domcreate_devmodel_started(libxl__egc *egc,
}
}
+ /* Plug nic interfaces */
+ if (!ret && d_config->num_vifs > 0) {
+ /* Attach nics */
+ GCNEW_ARRAY(dcs->devices, d_config->num_vifs);
+ dcs->num_devices = d_config->num_vifs;
+ for (i = 0; i < d_config->num_vifs; i++) {
+ libxl__init_ao_device(&dcs->devices[i], ao, &dcs->devices);
+ dcs->devices[i].callback = domcreate_nics_connected;
+ libxl__device_nic_add(egc, domid, &d_config->vifs[i],
+ &dcs->devices[i]);
+ }
+ return;
+ }
+
+ domcreate_attach_pci(egc, dcs);
+ return;
+
+error_out:
+ assert(ret);
+ domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_nics_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl__domain_create_state *dcs = CONTAINER_OF(aorm->base, *dcs, devices);
+ int last, ret = 0;
+
+ ret = libxl__ao_device_check_last(gc, aorm, dcs->devices,
+ dcs->num_devices, &last);
+
+ if (!last) return;
+ if (last && ret) {
+ LOGE(ERROR, "error connecting nics devices");
+ goto error_out;
+ }
+
+ domcreate_attach_pci(egc, dcs);
+ return;
+
+error_out:
+ assert(ret);
+ domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_attach_pci(libxl__egc *egc,
+ libxl__domain_create_state *dcs)
+{
+ STATE_AO_GC(dcs->ao);
+ int i, ret = 0;
+ libxl_ctx *ctx = CTX;
+ int domid = dcs->guest_domid;
+
+ /* convenience aliases */
+ libxl_domain_config *const d_config = dcs->guest_config;
+
for (i = 0; i < d_config->num_pcidevs; i++)
libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1);
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 96e6ec4..6b0ce95 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -467,11 +467,13 @@ int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
return 0;
}
-int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
+void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_nic *nic, libxl__ao_device *aorm)
{
+ STATE_AO_GC(aorm->ao);
flexarray_t *front;
flexarray_t *back;
- libxl__device device;
+ libxl__device *device;
char *dompath, **l;
unsigned int nb, rc;
@@ -504,7 +506,8 @@ int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
}
}
- rc = libxl__device_from_nic(gc, domid, nic, &device);
+ GCNEW(device);
+ rc = libxl__device_from_nic(gc, domid, nic, device);
if ( rc != 0 ) goto out_free;
flexarray_append(back, "frontend-id");
@@ -545,6 +548,9 @@ int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
flexarray_append(back, libxl__strdup(gc, nic->bridge));
flexarray_append(back, "handle");
flexarray_append(back, GCSPRINTF("%d", nic->devid));
+ flexarray_append(back, "type");
+ flexarray_append(back, GCSPRINTF("%s",
+ libxl_nic_type_to_string(nic->nictype)));
flexarray_append(front, "backend-id");
flexarray_append(front, GCSPRINTF("%d", nic->backend_domid));
@@ -555,17 +561,22 @@ int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
flexarray_append(front, "mac");
flexarray_append(front, GCSPRINTF(LIBXL_MAC_FMT,
LIBXL_MAC_BYTES(nic->mac)));
- libxl__device_generic_add(gc, &device,
+ libxl__device_generic_add(gc, device,
libxl__xs_kvs_of_flexarray(gc, back, back->count),
libxl__xs_kvs_of_flexarray(gc, front, front->count));
- /* FIXME: wait for plug */
+ aorm->dev = device;
+ aorm->action = DEVICE_CONNECT;
+ libxl__initiate_device_add(egc, aorm);
+
rc = 0;
out_free:
flexarray_free(back);
flexarray_free(front);
out:
- return rc;
+ aorm->rc = rc;
+ if (aorm->rc) aorm->callback(egc, aorm);
+ return;
}
int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 2b63a15..f63cf31 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -675,6 +675,12 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm);
+static void stubdom_nics_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
+static void stubdom_pvqemu_cb(libxl__egc *egc,
+ libxl__dm_spawn_state *stubdom_dmss,
+ int rc);
+
static void spaw_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
libxl__destroy_domid_state *dis,
int rc);
@@ -845,9 +851,11 @@ static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
}
for (i = 0; i < dm_config->num_vifs; i++) {
- ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->vifs[i]);
- if (ret)
- goto out;
+ /* We have to init the nic here, because we still haven't
+ * called libxl_device_nic_add at this point, but qemu needs
+ * the nic information to be complete.
+ */
+ libxl__device_nic_setdefault(gc, &dm_config->vifs[i]);
}
ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
if (ret)
@@ -923,9 +931,53 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
CONTAINER_OF(stubdom_dmss, *sdss, pvqemu);
STATE_AO_GC(sdss->dm.spawn.ao);
uint32_t dm_domid = sdss->pvqemu.guest_domid;
+ libxl_domain_config *d_config = stubdom_dmss->guest_config;
+ int i;
if (rc) goto out;
+ if (!rc && d_config->num_vifs > 0) {
+ GCNEW_ARRAY(sdss->devices, d_config->num_vifs);
+ sdss->num_devices = d_config->num_vifs;
+ for (i = 0; i < d_config->num_vifs; i++) {
+ libxl__init_ao_device(&sdss->devices[i], ao, &sdss->devices);
+ sdss->devices[i].callback = stubdom_nics_connected;
+ libxl__device_nic_add(egc, dm_domid, &d_config->vifs[i],
+ &sdss->devices[i]);
+ }
+ return;
+ }
+
+out:
+ stubdom_pvqemu_cb(egc, stubdom_dmss, rc);
+}
+
+static void stubdom_nics_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(aorm->base, *sdss, devices);
+ int last, ret = 0;
+
+ ret = libxl__ao_device_check_last(gc, aorm, sdss->devices,
+ sdss->num_devices, &last);
+
+ if (!last) return;
+ if (last && ret)
+ LOGE(ERROR, "error connecting nics devices");
+
+ stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
+ return;
+}
+
+static void stubdom_pvqemu_cb(libxl__egc *egc,
+ libxl__dm_spawn_state *stubdom_dmss,
+ int rc)
+{
+ libxl__stub_dm_spawn_state *sdss =
+ CONTAINER_OF(stubdom_dmss, *sdss, pvqemu);
+ STATE_AO_GC(sdss->dm.spawn.ao);
+ uint32_t dm_domid = sdss->pvqemu.guest_domid;
+
rc = libxl_domain_unpause(CTX, dm_domid);
if (rc) goto out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2500b86..725975d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -788,8 +788,6 @@ _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
_hidden int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
libxl_device_nic *nic,
libxl__device *device);
-_hidden int libxl__device_nic_add(libxl__gc *gc, uint32_t domid,
- libxl_device_nic *nic);
_hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
_hidden int libxl__device_disk_dev_number(const char *virtpath,
@@ -1821,6 +1819,11 @@ _hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
libxl_device_disk *disk,
libxl__ao_device *aorm);
+/* Internal AO operation to connect a nic device */
+_hidden void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_nic *nic,
+ libxl__ao_device *aorm);
+
/* Arranges that dev will be added to the guest, and the
* hotplug scripts will be executed (if necessary). When
* this is done (or an error happens), the callback in
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 69ce45a..46af9fb 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4970,7 +4970,7 @@ int main_networkattach(int argc, char **argv)
return 0;
}
- if (libxl_device_nic_add(ctx, domid, &nic)) {
+ if (libxl_device_nic_add(ctx, domid, &nic, 0)) {
fprintf(stderr, "libxl_device_nic_add failed.\n");
return 1;
}
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 10/13] libxl: add option to choose who executes hotplug scripts
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (8 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 09/13] libxl: convert libxl_device_nic_add " Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:40 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 11/13] libxl: set nic type to VIF by default Roger Pau Monne
` (2 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Add and option to xl.conf file to decide if hotplug scripts are
executed from the toolstack (xl) or from udev as it used to be in the
past.
This option is only introduced in this patch, but it has no effect
since the code to call hotplug scripts from libxl is introduced in a
latter patch.
This choice will be saved in "libxl/disable_udev", as specified in the
DISABLE_UDEV_PATH constant.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
docs/man/xl.conf.pod.5 | 8 ++++++++
tools/examples/xl.conf | 5 +++++
tools/libxl/libxl_create.c | 25 ++++++++++++++++++++++++-
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl.c | 4 ++++
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 1 +
8 files changed, 45 insertions(+), 1 deletions(-)
diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 8bd45ea..72825a0 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -55,6 +55,14 @@ default.
Default: C<1>
+=item B<run_hotplug_scripts=BOOLEAN>
+
+If disabled hotplug scripts will be called from udev, as it used to
+be in the previous releases. With the default option, hotplug scripts
+will be launched by xl directly.
+
+Default: C<1>
+
=item B<lockfile="PATH">
Sets the path to the lock file used by xl to serialise certain
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index 56d3b3b..75b00e0 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -12,3 +12,8 @@
# default output format used by "xl list -l"
#output_format="json"
+
+# default option to run hotplug scripts from xl
+# if disabled the old behaviour will be used, and hotplug scripts will be
+# launched by udev.
+#run_hotplug_scripts=1
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 2134645..4520bc7 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -398,7 +398,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info,
uint32_t *domid)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
- int flags, ret, rc;
+ int flags, ret, rc, nb_vm;
char *uuid_string;
char *dom_path, *vm_path, *libxl_path;
struct xs_permissions roperm[2];
@@ -519,6 +519,29 @@ retry_transaction:
libxl__sprintf(gc, "%s/hvmloader/generation-id-address", dom_path),
rwperm, ARRAY_SIZE(rwperm));
+ if (libxl_list_vm(ctx, &nb_vm) < 0) {
+ LOG(ERROR, "cannot get number of running guests");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ if (libxl_defbool_val(info->run_hotplug_scripts)) {
+ if (!libxl__xs_read(gc, t, DISABLE_UDEV_PATH) && (nb_vm - 1)) {
+ LOG(ERROR, "cannot change hotplug execution option once set, "
+ "please shutdown all guests before changing it");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ libxl__xs_write(gc, t, DISABLE_UDEV_PATH, "1");
+ } else {
+ if (libxl__xs_read(gc, t, DISABLE_UDEV_PATH) && (nb_vm - 1)) {
+ LOG(ERROR, "cannot change hotplug execution option once set, "
+ "please shutdown all guests before changing it");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ xs_rm(ctx->xsh, t, DISABLE_UDEV_PATH);
+ }
+
xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/uuid", vm_path), uuid_string, strlen(uuid_string));
xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/name", vm_path), info->name, strlen(info->name));
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 725975d..7969a43 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -86,6 +86,7 @@
#define STUBDOM_CONSOLE_SERIAL 3
#define STUBDOM_SPECIAL_CONSOLES 3
#define TAP_DEVICE_SUFFIX "-emu"
+#define DISABLE_UDEV_PATH "libxl/disable_udev"
#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 551e367..b1351de 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -220,6 +220,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
("xsdata", libxl_key_value_list),
("platformdata", libxl_key_value_list),
("poolid", uint32),
+ ("run_hotplug_scripts",libxl_defbool),
], dir=DIR_IN)
MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index d4db1f8..d992c32 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -35,6 +35,7 @@
xentoollog_logger_stdiostream *logger;
int dryrun_only;
int autoballoon = 1;
+libxl_defbool run_hotplug_scripts;
char *lockfile;
char *default_vifscript = NULL;
char *default_bridge = NULL;
@@ -66,6 +67,9 @@ static void parse_global_config(const char *configfile,
if (!xlu_cfg_get_long (config, "autoballoon", &l, 0))
autoballoon = l;
+ libxl_defbool_setdefault(&run_hotplug_scripts, true);
+ xlu_cfg_get_defbool(config, "run_hotplug_scripts", &run_hotplug_scripts, 0);
+
if (!xlu_cfg_get_string (config, "lockfile", &buf, 0))
lockfile = strdup(buf);
else {
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 2b6714a..43d727a 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -109,6 +109,7 @@ void postfork(void);
/* global options */
extern int autoballoon;
+extern libxl_defbool run_hotplug_scripts;
extern int dryrun_only;
extern char *lockfile;
extern char *default_vifscript;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 46af9fb..c13c48b 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -562,6 +562,7 @@ static void parse_config_data(const char *configfile_filename_report,
}
}
+ c_info->run_hotplug_scripts = run_hotplug_scripts;
c_info->type = LIBXL_DOMAIN_TYPE_PV;
if (!xlu_cfg_get_string (config, "builder", &buf, 0) &&
!strncmp(buf, "hvm", strlen(buf)))
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (9 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 10/13] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:41 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 12/13] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
2012-05-16 16:11 ` [PATCH 13/13] libxl: call hotplug scripts for nic " Roger Pau Monne
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Set the default value for nic interfaces to VIF, since it used to be
IOEMU, even for PV guests.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2490138..631de15 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1637,7 +1637,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
libxl__xen_script_dir_path()) < 0 )
return ERROR_FAIL;
if (!nic->nictype)
- nic->nictype = LIBXL_NIC_TYPE_IOEMU;
+ nic->nictype = LIBXL_NIC_TYPE_VIF;
return 0;
}
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 12/13] libxl: call hotplug scripts for disk devices from libxl
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (10 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 11/13] libxl: set nic type to VIF by default Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
2012-05-18 16:51 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 13/13] libxl: call hotplug scripts for nic " Roger Pau Monne
12 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Since most of the needed work is already done in previous patches,
this patch only contains the necessary code to call hotplug scripts
for disk devices, that should be called when the device is added or
removed from a guest.
We will chain the launch of the disk hotplug scripts after the
device_backend_callback callback, or directly from
libxl__initiate_device_{add,remove} if the device is already in the
desired state.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/hotplug/Linux/xen-backend.rules | 6 +-
tools/hotplug/Linux/xen-hotplug-common.sh | 6 ++
tools/libxl/Makefile | 3 +-
tools/libxl/libxl_device.c | 23 ++++-
tools/libxl/libxl_hotplug.c | 84 +++++++++++++++++++
tools/libxl/libxl_internal.h | 17 ++++
tools/libxl/libxl_linux.c | 126 +++++++++++++++++++++++++++++
7 files changed, 256 insertions(+), 9 deletions(-)
create mode 100644 tools/libxl/libxl_hotplug.c
diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
index 405387f..d55ff11 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -1,11 +1,11 @@
-SUBSYSTEM=="xen-backend", KERNEL=="tap*", RUN+="/etc/xen/scripts/blktap $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vbd*", RUN+="/etc/xen/scripts/block $env{ACTION}"
+SUBSYSTEM=="xen-backend", KERNEL=="tap*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/blktap $env{ACTION}"
+SUBSYSTEM=="xen-backend", KERNEL=="vbd*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/block $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vtpm*", RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
-SUBSYSTEM=="xen-backend", ACTION=="remove", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
+SUBSYSTEM=="xen-backend", ACTION=="remove", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
KERNEL=="evtchn", NAME="xen/%k"
SUBSYSTEM=="xen", KERNEL=="blktap[0-9]*", NAME="xen/%k", MODE="0600"
SUBSYSTEM=="blktap2", KERNEL=="blktap[0-9]*", NAME="xen/blktap-2/%k", MODE="0600"
diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh b/tools/hotplug/Linux/xen-hotplug-common.sh
index 8f6557d..4a7bc73 100644
--- a/tools/hotplug/Linux/xen-hotplug-common.sh
+++ b/tools/hotplug/Linux/xen-hotplug-common.sh
@@ -15,6 +15,12 @@
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
#
+# Hack to prevent the execution of hotplug scripts from udev if the domain
+# has been launched from libxl
+if [ -n "${UDEV_CALL}" ] && \
+ xenstore-read "libxl/disable_udev" >/dev/null 2>&1; then
+ exit 0
+fi
dir=$(dirname "$0")
. "$dir/hotplugpath.sh"
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 5d9227e..9abadff 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -66,7 +66,8 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
libxl_internal.o libxl_utils.o libxl_uuid.o \
libxl_json.o libxl_aoutils.o \
- libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
+ libxl_qmp.o libxl_event.o libxl_fork.o libxl_hotplug.o \
+ $(LIBXL_OBJS-y)
LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
$(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl) -include $(XEN_ROOT)/tools/config.h
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 6b0ce95..0b7beb7 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -380,6 +380,11 @@ void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
flexarray_append(back, "params");
flexarray_append(back, dev);
+ flexarray_append(back, "script");
+ flexarray_append(back, GCSPRINTF("%s/%s",
+ libxl__xen_script_dir_path(),
+ "block"));
+
assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
break;
case LIBXL_DISK_BACKEND_TAP:
@@ -393,6 +398,11 @@ void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
flexarray_append(back, "tapdisk-params");
flexarray_append(back, GCSPRINTF("%s:%s", format, disk->pdev_path));
+ flexarray_append(back, "script");
+ flexarray_append(back, GCSPRINTF("%s/%s",
+ libxl__xen_script_dir_path(),
+ "blktap"));
+
/* now create a phy device to export the device to the guest */
goto do_backend_phy;
case LIBXL_DISK_BACKEND_QDISK:
@@ -760,7 +770,7 @@ out_fail:
out_ok:
assert(!rc);
- aorm->callback(egc, aorm);
+ libxl__device_hotplug(egc, aorm);
return;
}
@@ -822,7 +832,7 @@ retry_transaction:
out_ok:
if (t) xs_transaction_end(ctx->xsh, t, 0);
- aorm->callback(egc, aorm);
+ libxl__device_hotplug(egc, aorm);
return;
}
@@ -929,14 +939,17 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
if (rc && aorm->action == DEVICE_CONNECT) {
LOG(ERROR, "unable to connect device with path %s",
libxl__device_backend_path(gc, aorm->dev));
- goto out;
+ goto error;
} else if(rc) {
LOG(DEBUG, "unable to disconnect device with path %s",
libxl__device_backend_path(gc, aorm->dev));
- goto out;
+ goto error;
}
-out:
+ libxl__device_hotplug(egc, aorm);
+ return;
+
+error:
aorm->rc = rc;
aorm->callback(egc, aorm);
return;
diff --git a/tools/libxl/libxl_hotplug.c b/tools/libxl/libxl_hotplug.c
new file mode 100644
index 0000000..686a38d
--- /dev/null
+++ b/tools/libxl/libxl_hotplug.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2012
+ * Author Roger Pau Monne <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.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+/*
+ * Generic hotplug helpers
+ *
+ * This helpers are both used by Linux and NetBSD hotplug script
+ * dispatcher functions.
+ */
+
+static void device_hotplug_timeout(libxl__egc *egc, libxl__ev_time *ev,
+ const struct timeval *requested_abs)
+{
+ libxl__ao_device *aorm = CONTAINER_OF(ev, *aorm, ev);
+ STATE_AO_GC(aorm->ao);
+
+ if (!aorm) return;
+ if (libxl__ev_child_inuse(&aorm->child)) {
+ if (kill(aorm->pid, SIGKILL)) {
+ LOGEV(ERROR, errno, "unable to kill hotplug script %s [%ld]",
+ aorm->what, (unsigned long)aorm->pid);
+ goto out;
+ }
+ }
+
+out:
+ libxl__ev_time_deregister(gc, &aorm->ev);
+ return;
+}
+
+int libxl__hotplug_launch(libxl__gc *gc, libxl__ao_device *aorm,
+ const char *arg0, char *const args[],
+ char *const env[], libxl__ev_child_callback *death)
+{
+ int rc = 0;
+ pid_t pid = -1;
+
+ libxl__ev_time_init(&aorm->ev);
+ libxl__ev_child_init(&aorm->child);
+
+ rc = libxl__ev_time_register_rel(gc, &aorm->ev, device_hotplug_timeout,
+ LIBXL_HOTPLUG_TIMEOUT * 1000);
+ if (rc) {
+ LOGE(ERROR, "unable to register timeout for hotplug script %s", arg0);
+ goto out;
+ }
+
+ pid = libxl__ev_child_fork(gc, &aorm->child, death);
+ if (pid == -1) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ if (!pid) {
+ /* child */
+ libxl__exec(gc, -1, -1, -1, arg0, args, env);
+ LOGE(ERROR, "unable execute hotplug scripts for device %"PRIu32 " on "
+ "dom %"PRIu32, aorm->dev->devid, aorm->dev->backend_domid);
+ abort();
+ }
+out:
+ if (libxl__ev_child_inuse(&aorm->child)) {
+ aorm->pid = pid;
+ /* we will get a callback when the child dies */
+ return 0;
+ }
+ libxl__ev_time_deregister(gc, &aorm->ev);
+ return rc;
+}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 7969a43..5e2bac5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -72,6 +72,7 @@
#define LIBXL_INIT_TIMEOUT 10
#define LIBXL_DESTROY_TIMEOUT 10
+#define LIBXL_HOTPLUG_TIMEOUT 10
#define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
#define LIBXL_XENCONSOLE_LIMIT 1048576
#define LIBXL_XENCONSOLE_PROTOCOL "vt100"
@@ -1813,6 +1814,11 @@ struct libxl__ao_device {
int rc;
libxl__ev_devstate ds;
void *base;
+ /* device hotplug execution */
+ pid_t pid;
+ char *what;
+ libxl__ev_time ev;
+ libxl__ev_child child;
};
/* Internal AO operation to connect a disk device */
@@ -1840,6 +1846,17 @@ _hidden void libxl__initiate_device_add(libxl__egc*, libxl__ao_device *aorm);
_hidden void libxl__initiate_device_remove(libxl__egc *egc,
libxl__ao_device *aorm);
+/*
+ * libxl__hotplug_launch is an internal function that should not be used
+ * directly, all hotplug script calls have to implement and use
+ * libxl__device_hotplug.
+ */
+_hidden void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aorm);
+_hidden int libxl__hotplug_launch(libxl__gc *gc, libxl__ao_device *aorm,
+ const char *arg0, char *const args[],
+ char *const env[],
+ libxl__ev_child_callback *death);
+
/*----- Domain destruction -----*/
/* Domain destruction has been splitted in two functions:
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 925248b..315ce15 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -25,3 +25,129 @@ int libxl__try_phy_backend(mode_t st_mode)
return 1;
}
+
+/* Hotplug scripts helpers */
+
+static void cleanup(libxl__gc *gc, libxl__ao_device *aorm)
+{
+ if (!aorm) return;
+ libxl__ev_time_deregister(gc, &aorm->ev);
+}
+
+static void callback(libxl__egc *egc, libxl__ev_child *child,
+ pid_t pid, int status)
+{
+ libxl__ao_device *aorm = CONTAINER_OF(child, *aorm, child);
+ STATE_AO_GC(aorm->ao);
+
+ cleanup(gc, aorm);
+
+ if (status) {
+ libxl_report_child_exitstatus(CTX, aorm->rc ? LIBXL__LOG_ERROR
+ : LIBXL__LOG_WARNING,
+ aorm->what, pid, status);
+ aorm->rc = ERROR_FAIL;
+ }
+ aorm->callback(egc, aorm);
+}
+
+static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
+{
+ char *be_path = libxl__device_backend_path(gc, dev);
+ char *script;
+ const char *type = libxl__device_kind_to_string(dev->backend_kind);
+ char **env;
+ int nr = 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);
+ return NULL;
+ }
+
+ GCNEW_ARRAY(env, 9);
+ env[nr++] = "script";
+ env[nr++] = script;
+ env[nr++] = "XENBUS_TYPE";
+ env[nr++] = libxl__strdup(gc, type);
+ env[nr++] = "XENBUS_PATH";
+ env[nr++] = GCSPRINTF("backend/%s/%u/%d", type, dev->domid, dev->devid);
+ env[nr++] = "XENBUS_BASE_PATH";
+ env[nr++] = "backend";
+ env[nr++] = NULL;
+
+ return env;
+}
+
+/* Hotplug scripts caller functions */
+
+static int libxl__hotplug_disk(libxl__gc *gc, libxl__ao_device *aorm)
+{
+ char *be_path = libxl__device_backend_path(gc, aorm->dev);
+ char *script;
+ char **args, **env;
+ 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);
+ return ERROR_FAIL;
+ }
+
+ env = get_hotplug_env(gc, aorm->dev);
+ if (!env)
+ return ERROR_FAIL;
+
+ GCNEW_ARRAY(args, 3);
+
+ args[nr++] = script;
+ args[nr++] = aorm->action == DEVICE_CONNECT ? "add" : "remove";
+ args[nr++] = NULL;
+
+ aorm->what = GCSPRINTF("%s %s", args[0], args[1]);
+ LOG(DEBUG, "calling hotplug script: %s %s", args[0], args[1]);
+ rc = libxl__hotplug_launch(gc, aorm, args[0], args, env, callback);
+ if (rc) {
+ goto out_free;
+ }
+
+ rc = 0;
+
+out_free:
+ return rc;
+}
+
+void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH);
+
+ /* Check if we have to run hotplug scripts */
+ if (!disable_udev) goto out;
+
+ switch (aorm->dev->backend_kind) {
+ case LIBXL__DEVICE_KIND_VBD:
+ aorm->rc = libxl__hotplug_disk(gc, aorm);
+ if (aorm->rc) goto error;
+ break;
+ default:
+ /* If no need to execute any hotplug scripts,
+ * call the callback manually
+ */
+ aorm->rc = 0;
+ aorm->callback(egc, aorm);
+ break;
+ }
+
+ return;
+
+error:
+ assert(aorm->rc);
+ LOGE(ERROR, "unable to queue execution of hotplug script for device "
+ "with path %s", libxl__device_backend_path(gc, aorm->dev));
+out:
+ aorm->callback(egc, aorm);
+ return;
+}
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 13/13] libxl: call hotplug scripts for nic devices from libxl
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
` (11 preceding siblings ...)
2012-05-16 16:11 ` [PATCH 12/13] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
@ 2012-05-16 16:11 ` Roger Pau Monne
12 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-16 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Since most of the needed work is already done in previous patches,
this patch only contains the necessary code to call hotplug scripts
for nic devices, that should be called when the device is added or
removed from a guest.
Since the call to libxl__device_hotplug is already there, this patch
only adds the necessary code to this function to handle the vif case,
and the necessary modification in the udev rules.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/hotplug/Linux/xen-backend.rules | 6 +-
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_linux.c | 130 ++++++++++++++++++++++++++++++++-
3 files changed, 133 insertions(+), 4 deletions(-)
diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
index d55ff11..c591a3f 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -2,8 +2,8 @@ SUBSYSTEM=="xen-backend", KERNEL=="tap*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scr
SUBSYSTEM=="xen-backend", KERNEL=="vbd*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/block $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vtpm*", RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ENV{UDEV_CALL}="1", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ENV{UDEV_CALL}="1", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
SUBSYSTEM=="xen-backend", ACTION=="remove", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
KERNEL=="evtchn", NAME="xen/%k"
@@ -13,4 +13,4 @@ KERNEL=="blktap-control", NAME="xen/blktap-2/control", MODE="0600"
KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5e2bac5..b5002db 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1817,6 +1817,7 @@ struct libxl__ao_device {
/* device hotplug execution */
pid_t pid;
char *what;
+ int vif_executed;
libxl__ev_time ev;
libxl__ev_child child;
};
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 315ce15..6e752bd 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -28,6 +28,26 @@ int libxl__try_phy_backend(mode_t st_mode)
/* Hotplug scripts helpers */
+static libxl_nic_type get_nic_type(libxl__gc *gc, libxl__device *dev)
+{
+ char *snictype, *be_path;
+ libxl_nic_type nictype;
+
+ be_path = libxl__device_backend_path(gc, dev);
+ snictype = libxl__xs_read(gc, XBT_NULL,
+ GCSPRINTF("%s/%s", be_path, "type"));
+ if (!snictype) {
+ LOGE(ERROR, "unable to read nictype from %s", be_path);
+ return -1;
+ }
+ if (libxl_nic_type_from_string(snictype, &nictype) < 0) {
+ LOGE(ERROR, "unable to parse nictype from %s", be_path);
+ return -1;
+ }
+
+ return nictype;
+}
+
static void cleanup(libxl__gc *gc, libxl__ao_device *aorm)
{
if (!aorm) return;
@@ -47,7 +67,18 @@ static void callback(libxl__egc *egc, libxl__ev_child *child,
: LIBXL__LOG_WARNING,
aorm->what, pid, status);
aorm->rc = ERROR_FAIL;
+ goto out;
+ }
+
+ if (aorm->dev->backend_kind == LIBXL__DEVICE_KIND_VIF &&
+ get_nic_type(gc, aorm->dev) == LIBXL_NIC_TYPE_IOEMU &&
+ !aorm->vif_executed) {
+ aorm->vif_executed = 1;
+ libxl__device_hotplug(egc, aorm);
+ return;
}
+
+out:
aorm->callback(egc, aorm);
}
@@ -66,7 +97,7 @@ static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
return NULL;
}
- GCNEW_ARRAY(env, 9);
+ GCNEW_ARRAY(env, 13);
env[nr++] = "script";
env[nr++] = script;
env[nr++] = "XENBUS_TYPE";
@@ -75,6 +106,24 @@ static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
env[nr++] = GCSPRINTF("backend/%s/%u/%d", type, dev->domid, dev->devid);
env[nr++] = "XENBUS_BASE_PATH";
env[nr++] = "backend";
+ if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
+ switch (get_nic_type(gc, dev)) {
+ case LIBXL_NIC_TYPE_IOEMU:
+ env[nr++] = "INTERFACE";
+ env[nr++] = libxl__strdup(gc, libxl__device_nic_devname(gc,
+ dev->domid, dev->devid,
+ LIBXL_NIC_TYPE_IOEMU));
+ case LIBXL_NIC_TYPE_VIF:
+ env[nr++] = "vif";
+ env[nr++] = libxl__strdup(gc, libxl__device_nic_devname(gc,
+ dev->domid, dev->devid,
+ LIBXL_NIC_TYPE_VIF));
+ break;
+ default:
+ return NULL;
+ }
+ }
+
env[nr++] = NULL;
return env;
@@ -119,6 +168,81 @@ out_free:
return rc;
}
+static int libxl__hotplug_nic(libxl__gc *gc, libxl__ao_device *aorm)
+{
+ char *be_path = libxl__device_backend_path(gc, aorm->dev);
+ char *what, *script;
+ char **args, **env;
+ int nr = 0, rc = 0;
+ libxl_nic_type nictype;
+
+ script = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/%s", be_path,
+ "script"));
+ if (!script) {
+ LOGE(ERROR, "unable to read script from %s", be_path);
+ return -1;
+ }
+
+ nictype = get_nic_type(gc, aorm->dev);
+ if (nictype < 0) {
+ LOGE(ERROR, "error when fetching nic type");
+ return -1;
+ }
+
+ env = get_hotplug_env(gc, aorm->dev);
+ if (!env)
+ return -1;
+
+ GCNEW_ARRAY(args, 4);
+
+ switch (nictype) {
+ case LIBXL_NIC_TYPE_IOEMU:
+ if (!aorm->vif_executed) goto execute_vif;
+ args[nr++] = script;
+ args[nr++] = aorm->action == DEVICE_CONNECT ? "add" : "remove";
+ args[nr++] = libxl__strdup(gc, "type_if=tap");
+ args[nr++] = NULL;
+ what = GCSPRINTF("%s %s", args[0], aorm->action == DEVICE_CONNECT ?
+ "connect" : "disconnect");
+ LOG(DEBUG, "Calling hotplug script: %s %s %s",
+ args[0], args[1], args[2]);
+ rc = libxl__hotplug_launch(gc, aorm, args[0], args, env, callback);
+ if (rc) {
+ LOGE(ERROR, "unable execute hotplug scripts for vif device %"PRIu32,
+ aorm->dev->devid);
+ goto out;
+ }
+ break;
+ case LIBXL_NIC_TYPE_VIF:
+execute_vif:
+ args[nr++] = script;
+ args[nr++] = aorm->action == DEVICE_CONNECT ? "online" : "offline";
+ args[nr++] = libxl__strdup(gc, "type_if=vif");
+ args[nr++] = NULL;
+ what = GCSPRINTF("%s %s", args[0], aorm->action == DEVICE_CONNECT ?
+ "connect" : "disconnect");
+ LOG(DEBUG, "Calling hotplug script: %s %s %s",
+ args[0], args[1], args[2]);
+ rc = libxl__hotplug_launch(gc, aorm, args[0], args, env, callback);
+ if (rc) {
+ LOGE(ERROR, "unable execute hotplug scripts for vif device %"PRIu32,
+ aorm->dev->devid);
+ goto out;
+ }
+ break;
+ default:
+ /* Unknown network type */
+ LOG(DEBUG, "unknown network card type with id %"PRIu32,
+ aorm->dev->devid);
+ return 0;
+ }
+
+ rc = 0;
+
+out:
+ return rc;
+}
+
void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aorm)
{
STATE_AO_GC(aorm->ao);
@@ -132,6 +256,10 @@ void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aorm)
aorm->rc = libxl__hotplug_disk(gc, aorm);
if (aorm->rc) goto error;
break;
+ case LIBXL__DEVICE_KIND_VIF:
+ aorm->rc = libxl__hotplug_nic(gc, aorm);
+ if (aorm->rc) goto error;
+ break;
default:
/* If no need to execute any hotplug scripts,
* call the callback manually
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 01/13] libxl: pass env vars to libxl__exec
2012-05-16 16:11 ` [PATCH 01/13] libxl: pass env vars to libxl__exec Roger Pau Monne
@ 2012-05-18 16:02 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:02 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 01/13] libxl: pass env vars to libxl__exec"):
> Add another parameter to libxl__exec call that contains the
> environment variables to use when performing the execvp call.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 02/13] libxl: fix libxl__xs_directory usage of transaction
2012-05-16 16:11 ` [PATCH 02/13] libxl: fix libxl__xs_directory usage of transaction Roger Pau Monne
@ 2012-05-18 16:03 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:03 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 02/13] libxl: fix libxl__xs_directory usage of transaction"):
> libxl__xs_directory takes a transaction parameter, but completely
> ignores it, passing XBT_NULL unconditionally to xs_directory.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 03/13] libxl: add libxl__xs_path_cleanup
2012-05-16 16:11 ` [PATCH 03/13] libxl: add libxl__xs_path_cleanup Roger Pau Monne
@ 2012-05-18 16:06 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:06 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 03/13] libxl: add libxl__xs_path_cleanup"):
> Add a function which behaves like "xenstore-rm -t", and which will be
> used to clean xenstore after unplug since we will be no longer
> executing xen-hotplug-cleanup script, that used to do that for us.
...
> + if (!user_path) {
> + LOGE(ERROR, "null path provided");
> + return ERROR_INVAL;
> + }
What is this for ? Why not just crash ?
> + if (!t) {
> + LOGE(ERROR, "null transaction provided");
> + return ERROR_INVAL;
> + }
Likewise why not
assert(t);
?
> + path = libxl__strdup(gc, user_path);
> + if (!xs_rm(CTX->xsh, t, path)) {
> + rc = ERROR_FAIL;
> + goto out;
...
> +out:
> + return rc;
This has the effect of discarding the errno value if anything fails.
Perhaps this function should log on all errors ?
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 06/13] libxl: cleanup libxl__device_{disk, nic}_add
2012-05-16 16:11 ` [PATCH 06/13] libxl: cleanup libxl__device_{disk, nic}_add Roger Pau Monne
@ 2012-05-18 16:07 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:07 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 06/13] libxl: cleanup libxl__device_{disk,nic}_add"):
> Change libxl__sprintf for GCSPRINTF, LIBXL__LOG* for LOG and remove
> use of ctx variable in favour of the CTX constant.
Is this necessary to make things fit in subsequent patches ?
In general I approve of this kind of cleanup but I don't think now is
the right time to be doing it...
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op
2012-05-16 16:11 ` [PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op Roger Pau Monne
@ 2012-05-18 16:19 ` Ian Jackson
2012-05-18 16:24 ` Ian Jackson
1 sibling, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:19 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op"):
> This change introduces some new structures, and breaks the mutual
> dependency that libxl_domain_destroy and libxl__destroy_device_model
> had. This is done by checking if the domid passed to
> libxl_domain_destroy has a stubdom, and then having the bulk of the
> destroy machinery in a separate function (libxl__destroy_domid) that
> doesn't check for stubdom presence, since we check for it in the upper
> level function. The reason behind this change is the need to use
> structures for ao operations, and it was impossible to have two
> different self-referencing structs.
>
> All uses of libxl_domain_destroy have been changed, and either
> replaced by the new libxl_domain_destroy ao function or by the
> internal libxl__domain_destroy that can be used inside an already
> running ao.
Can you separate out the code motion into a separate patch ? As it is
it's very difficult to review.
Also can you try to eliminate any code motion that's not strictly
necessaray.
Thanks,
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op
2012-05-16 16:11 ` [PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op Roger Pau Monne
2012-05-18 16:19 ` Ian Jackson
@ 2012-05-18 16:24 ` Ian Jackson
1 sibling, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:24 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op"):
> +struct libxl__ao_device {
> + libxl__ao *ao;
> + /* State in which the device is */
> + libxl__device_state state;
> + /* action being performed */
> + libxl__device_action action;
> + libxl__device *dev;
> + int force;
> + libxl__device_callback *callback;
> + /* private for implementation */
> + int rc;
> + libxl__ev_devstate ds;
> + void *base;
> +};
Perhaps this new functionality could be in a separate patch too ? I
guess it won't share a great deal of code with the existing machinery.
> +/*----- device addition/removal -----*/
> +
> +/* During the init/destruction process, the device can be in several states:
> + *
> + * DEVICE_UNKNOWN: device in unknown state (default value when struct is
> + * initialized).
> + *
> + * DEVICE_WAIT_BACKEND: waiting for the backend to switch to XenbusStateInit or
> + * XenbusStateClosed.
> + *
> + * DEVICE_WAIT_HOTPLUG: waiting for hotplug script to finish execution.
> + *
> + * DEVICE_FINISHED: device is connected/disconnected.
> + */
> +typedef enum {
> + DEVICE_ACTIVE,
> + DEVICE_FINISHED
> +} libxl__device_state;
This enum doesn't seem to correspond to the comment. Also if it
really is a two-state enum why not just have a boolean called
"finished" ?
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 08/13] libxl: convert libxl_device_disk_add to an async operation
2012-05-16 16:11 ` [PATCH 08/13] libxl: convert libxl_device_disk_add to an async operation Roger Pau Monne
@ 2012-05-18 16:33 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:33 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 08/13] libxl: convert libxl_device_disk_add to an async operation"):
> +static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
> +{
> + STATE_AO_GC(aorm->ao);
> + libxl__domain_create_state *dcs = CONTAINER_OF(aorm->base, *dcs, devices)...
> + ret = libxl__ao_device_check_last(gc, aorm, dcs->devices,
> + dcs->num_devices, &last);
> + if (!last) return;
> + if (last && ret) {
> + LOGE(ERROR, "error connecting disk devices");
> + goto error_out;
> + }
This is a slightly odd way of putting it. The `if (last' part is
always going to be true because we always return if !last.
Also, please can you use "rc" for libxl error codes rather than "ret".
We should standardise on one name for this and all the recent new code
uses "rc".
> + /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm.
> + * Fill in any field required by either, including both relevant
> + * callbacks (_spawn_stub_dm will overwrite our trespass if needed). */
> + dcs->dmss.dm.spawn.ao = ao;
> + dcs->dmss.dm.guest_config = dcs->guest_config;
> + dcs->dmss.dm.build_state = &dcs->build_state;
> + dcs->dmss.dm.callback = domcreate_devmodel_started;
> + dcs->dmss.callback = domcreate_devmodel_started;
> +
I don't understand why this hunk appears here in this diff. Surely
this patch should not need to change the initialisation of dcs->dmss ?
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 4019309..2b63a15 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
... int rc);
> @@ -800,22 +801,60 @@ retry_transaction:
> if (errno == EAGAIN)
> goto retry_transaction;
>
> + GCNEW_ARRAY(sdss->devices, dm_config->num_disks);
> + sdss->num_devices = dm_config->num_disks;
> for (i = 0; i < dm_config->num_disks; i++) {
> - ret = libxl_device_disk_add(ctx, dm_domid, &dm_config->disks[i]);
> - if (ret)
> - goto out_free;
> + libxl__init_ao_device(&sdss->devices[i], ao, &sdss->devices);
> + sdss->devices[i].callback = spawn_stub_disk_connected;
> + libxl__device_disk_add(egc, dm_domid, &dm_config->disks[i],
> + &sdss->devices[i]);
> + }
We seem to have a couple of instances of this pattern. You have a
helper function libxl__ao_device_check_last to check they're all done
and fish out the rc value.
Perhaps there should be a small helper function to populate devices[]
and call libxl__device_disk_add ?
Thanks,
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 09/13] libxl: convert libxl_device_nic_add to an async operation
2012-05-16 16:11 ` [PATCH 09/13] libxl: convert libxl_device_nic_add " Roger Pau Monne
@ 2012-05-18 16:38 ` Ian Jackson
2012-05-22 13:49 ` Roger Pau Monne
0 siblings, 1 reply; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:38 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 09/13] libxl: convert libxl_device_nic_add to an async operation"):
> This patch converts libxl_device_nic_add to an ao operation that
> waits for device backend to reach state XenbusStateInitWait and then
> marks the operation as completed. This is not really useful now, but
> will be used by latter patches that will launch hotplug scripts after
> we reached the desired xenbus state.
Why do you serialise vif and vbd initialisation ? Would anything go
wrong if you did them both at once ?
> + /* Plug nic interfaces */
> + if (!ret && d_config->num_vifs > 0) {
> + /* Attach nics */
> + GCNEW_ARRAY(dcs->devices, d_config->num_vifs);
> + dcs->num_devices = d_config->num_vifs;
> + for (i = 0; i < d_config->num_vifs; i++) {
> + libxl__init_ao_device(&dcs->devices[i], ao, &dcs->devices);
> + dcs->devices[i].callback = domcreate_nics_connected;
> + libxl__device_nic_add(egc, domid, &d_config->vifs[i],
> + &dcs->devices[i]);
Ah this pattern again....
> -int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
> +void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
> + libxl_device_nic *nic, libxl__ao_device *aorm)
> {
> + STATE_AO_GC(aorm->ao);
> flexarray_t *front;
> flexarray_t *back;
> - libxl__device device;
> + libxl__device *device;
You might want to consider a pre-patch which changes this, and
libxl__device_disk_add's, as follows:
- libxl__device device;
+ libxl__device device[1];
That would get rid of the noise from this patch. Up to you, though;
because it's separated out textually doesn't make the diff unreadable
like it is.
> @@ -845,9 +851,11 @@ static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
> }
>
> for (i = 0; i < dm_config->num_vifs; i++) {
> - ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->vifs[i]);
> - if (ret)
> - goto out;
> + /* We have to init the nic here, because we still haven't
> + * called libxl_device_nic_add at this point, but qemu needs
> + * the nic information to be complete.
> + */
> + libxl__device_nic_setdefault(gc, &dm_config->vifs[i]);
This is really too much repetition now. Can we not have a common "add
devices" function which is called once for the main domain and once
for the stubdom ?
In the future we'll need that if we have more service domains, too.
Thanks,
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 10/13] libxl: add option to choose who executes hotplug scripts
2012-05-16 16:11 ` [PATCH 10/13] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
@ 2012-05-18 16:40 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:40 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 10/13] libxl: add option to choose who executes hotplug scripts"):
> Add and option to xl.conf file to decide if hotplug scripts are
> executed from the toolstack (xl) or from udev as it used to be in the
> past.
...
> + if (libxl_defbool_val(info->run_hotplug_scripts)) {
> + if (!libxl__xs_read(gc, t, DISABLE_UDEV_PATH) && (nb_vm - 1)) {
> + LOG(ERROR, "cannot change hotplug execution option once set, "
> + "please shutdown all guests before changing it");
> + rc = ERROR_FAIL;
> + goto out;
> + }
> + libxl__xs_write(gc, t, DISABLE_UDEV_PATH, "1");
> + } else {
> + if (libxl__xs_read(gc, t, DISABLE_UDEV_PATH) && (nb_vm - 1)) {
> + LOG(ERROR, "cannot change hotplug execution option once set, "
> + "please shutdown all guests before changing it");
> + rc = ERROR_FAIL;
> + goto out;
> + }
> + xs_rm(ctx->xsh, t, DISABLE_UDEV_PATH);
How about
int disable_wanted_now = something involving libxl__xs_read(....);
if (disable_wanted_now !=
libxl_defbool_val(info->run_hotplug_scripts) {
LOG(ERROR,
etc.
}
?
Also you should check for the errno value from libxl__xs_read. ENOENT
is fine but anything else should be a fatal error.
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-16 16:11 ` [PATCH 11/13] libxl: set nic type to VIF by default Roger Pau Monne
@ 2012-05-18 16:41 ` Ian Jackson
2012-05-21 16:29 ` Roger Pau Monne
0 siblings, 1 reply; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:41 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 11/13] libxl: set nic type to VIF by default"):
> Set the default value for nic interfaces to VIF, since it used to be
> IOEMU, even for PV guests.
How odd.
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 2490138..631de15 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1637,7 +1637,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
> libxl__xen_script_dir_path()) < 0 )
> return ERROR_FAIL;
> if (!nic->nictype)
> - nic->nictype = LIBXL_NIC_TYPE_IOEMU;
> + nic->nictype = LIBXL_NIC_TYPE_VIF;
But doesn't this set the default type to VIF even for HVM guests ?
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 12/13] libxl: call hotplug scripts for disk devices from libxl
2012-05-16 16:11 ` [PATCH 12/13] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
@ 2012-05-18 16:51 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-18 16:51 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[PATCH 12/13] libxl: call hotplug scripts for disk devices from libxl"):
> Since most of the needed work is already done in previous patches,
> this patch only contains the necessary code to call hotplug scripts
> for disk devices, that should be called when the device is added or
> removed from a guest.
>
> We will chain the launch of the disk hotplug scripts after the
> device_backend_callback callback, or directly from
> libxl__initiate_device_{add,remove} if the device is already in the
> desired state.
>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> ---
> tools/hotplug/Linux/xen-backend.rules | 6 +-
> tools/hotplug/Linux/xen-hotplug-common.sh | 6 ++
> tools/libxl/Makefile | 3 +-
> tools/libxl/libxl_device.c | 23 ++++-
> tools/libxl/libxl_hotplug.c | 84 +++++++++++++++++++
> tools/libxl/libxl_internal.h | 17 ++++
> tools/libxl/libxl_linux.c | 126 +++++++++++++++++++++++++++++
> 7 files changed, 256 insertions(+), 9 deletions(-)
> create mode 100644 tools/libxl/libxl_hotplug.c
>
> diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
> index 405387f..d55ff11 100644
> --- a/tools/hotplug/Linux/xen-backend.rules
> +++ b/tools/hotplug/Linux/xen-backend.rules
> @@ -1,11 +1,11 @@
> -SUBSYSTEM=="xen-backend", KERNEL=="tap*", RUN+="/etc/xen/scripts/blktap $env{ACTION}"
> -SUBSYSTEM=="xen-backend", KERNEL=="vbd*", RUN+="/etc/xen/scripts/block $env{ACTION}"
> +SUBSYSTEM=="xen-backend", KERNEL=="tap*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/blktap $env{ACTION}"
> +SUBSYSTEM=="xen-backend", KERNEL=="vbd*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/block $env{ACTION}"
> SUBSYSTEM=="xen-backend", KERNEL=="vtpm*", RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
> SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
> SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
> SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
> SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
> -SUBSYSTEM=="xen-backend", ACTION=="remove", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
> +SUBSYSTEM=="xen-backend", ACTION=="remove", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
> KERNEL=="evtchn", NAME="xen/%k"
> SUBSYSTEM=="xen", KERNEL=="blktap[0-9]*", NAME="xen/%k", MODE="0600"
> SUBSYSTEM=="blktap2", KERNEL=="blktap[0-9]*", NAME="xen/blktap-2/%k", MODE="0600"
> diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh b/tools/hotplug/Linux/xen-hotplug-common.sh
> index 8f6557d..4a7bc73 100644
> --- a/tools/hotplug/Linux/xen-hotplug-common.sh
> +++ b/tools/hotplug/Linux/xen-hotplug-common.sh
> @@ -15,6 +15,12 @@
> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> #
>
> +# Hack to prevent the execution of hotplug scripts from udev if the domain
> +# has been launched from libxl
> +if [ -n "${UDEV_CALL}" ] && \
> + xenstore-read "libxl/disable_udev" >/dev/null 2>&1; then
> + exit 0
> +fi
>
> dir=$(dirname "$0")
> . "$dir/hotplugpath.sh"
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 5d9227e..9abadff 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -66,7 +66,8 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
> libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
> libxl_internal.o libxl_utils.o libxl_uuid.o \
> libxl_json.o libxl_aoutils.o \
> - libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
> + libxl_qmp.o libxl_event.o libxl_fork.o libxl_hotplug.o \
> + $(LIBXL_OBJS-y)
> LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
>
> $(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl) -include $(XEN_ROOT)/tools/config.h
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 6b0ce95..0b7beb7 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -393,6 +398,11 @@ void libxl__device_disk_add(libxl__egc *egc, uint32_t dom> @@ -760,7 +770,7 @@ out_fail:
>
> out_ok:
> assert(!rc);
> - aorm->callback(egc, aorm);
> + libxl__device_hotplug(egc, aorm);
Where does the name "aorm" come from here ? It doesn't seem to
involve removal.
> +int libxl__hotplug_launch(libxl__gc *gc, libxl__ao_device *aorm,
Ie shouldn't that be "aodev" or something ?
> +/* Hotplug scripts helpers */
> +
> +static void cleanup(libxl__gc *gc, libxl__ao_device *aorm)
> +{
> + if (!aorm) return;
> + libxl__ev_time_deregister(gc, &aorm->ev);
> +}
> +
> +static void callback(libxl__egc *egc, libxl__ev_child *child,
> + pid_t pid, int status)
> +{
> + libxl__ao_device *aorm = CONTAINER_OF(child, *aorm, child);
> + STATE_AO_GC(aorm->ao);
> +
> + cleanup(gc, aorm);
> +
> + if (status) {
> + libxl_report_child_exitstatus(CTX, aorm->rc ? LIBXL__LOG_ERROR
> + : LIBXL__LOG_WARNING,
> + aorm->what, pid, status);
> + aorm->rc = ERROR_FAIL;
> + }
> + aorm->callback(egc, aorm);
> +}
This structure where half of the event handling is in the common code
and half of it is in libxl_linux.c is not really appropriate. It
means that the hotplug event machinery is not all in the same place.
Can you make an interface to libxl_linux.c which is fully synchronous,
so that libxl_linux.c simply says whether to run the script or not ?
Eg
libxl__get_hotplug_script_info(libxl__egc *, libxl__device *dev,
char ***args, char ***env);
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-18 16:41 ` Ian Jackson
@ 2012-05-21 16:29 ` Roger Pau Monne
2012-05-29 14:40 ` Ian Jackson
0 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-21 16:29 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 11/13] libxl: set nic type to VIF by default"):
>> Set the default value for nic interfaces to VIF, since it used to be
>> IOEMU, even for PV guests.
>
> How odd.
>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 2490138..631de15 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1637,7 +1637,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
>> libxl__xen_script_dir_path())< 0 )
>> return ERROR_FAIL;
>> if (!nic->nictype)
>> - nic->nictype = LIBXL_NIC_TYPE_IOEMU;
>> + nic->nictype = LIBXL_NIC_TYPE_VIF;
>
> But doesn't this set the default type to VIF even for HVM guests ?
Shouldn't HVM guests use the "ioemu" parameter if they want an emulated
card? If no parameter is provided then the default type should be VIF,
because there's no other way to specifically set the type to VIF.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 09/13] libxl: convert libxl_device_nic_add to an async operation
2012-05-18 16:38 ` Ian Jackson
@ 2012-05-22 13:49 ` Roger Pau Monne
2012-05-22 14:04 ` Ian Jackson
0 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-22 13:49 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 09/13] libxl: convert libxl_device_nic_add to an async operation"):
>> This patch converts libxl_device_nic_add to an ao operation that
>> waits for device backend to reach state XenbusStateInitWait and then
>> marks the operation as completed. This is not really useful now, but
>> will be used by latter patches that will launch hotplug scripts after
>> we reached the desired xenbus state.
>
> Why do you serialise vif and vbd initialisation ? Would anything go
> wrong if you did them both at once ?
disk needs to be added before Qemu is launched, on the other hand nics
need to be added after qemu is launched, so we need to add them at
different moments during the domain creation, it's a PITA.
>> + /* Plug nic interfaces */
>> + if (!ret&& d_config->num_vifs> 0) {
>> + /* Attach nics */
>> + GCNEW_ARRAY(dcs->devices, d_config->num_vifs);
>> + dcs->num_devices = d_config->num_vifs;
>> + for (i = 0; i< d_config->num_vifs; i++) {
>> + libxl__init_ao_device(&dcs->devices[i], ao,&dcs->devices);
>> + dcs->devices[i].callback = domcreate_nics_connected;
>> + libxl__device_nic_add(egc, domid,&d_config->vifs[i],
>> +&dcs->devices[i]);
>
> Ah this pattern again....
>
>> -int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
>> +void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
>> + libxl_device_nic *nic, libxl__ao_device *aorm)
>> {
>> + STATE_AO_GC(aorm->ao);
>> flexarray_t *front;
>> flexarray_t *back;
>> - libxl__device device;
>> + libxl__device *device;
>
> You might want to consider a pre-patch which changes this, and
> libxl__device_disk_add's, as follows:
> - libxl__device device;
> + libxl__device device[1];
>
> That would get rid of the noise from this patch. Up to you, though;
> because it's separated out textually doesn't make the diff unreadable
> like it is.
>
>> @@ -845,9 +851,11 @@ static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
>> }
>>
>> for (i = 0; i< dm_config->num_vifs; i++) {
>> - ret = libxl_device_nic_add(ctx, dm_domid,&dm_config->vifs[i]);
>> - if (ret)
>> - goto out;
>> + /* We have to init the nic here, because we still haven't
>> + * called libxl_device_nic_add at this point, but qemu needs
>> + * the nic information to be complete.
>> + */
>> + libxl__device_nic_setdefault(gc,&dm_config->vifs[i]);
>
> This is really too much repetition now. Can we not have a common "add
> devices" function which is called once for the main domain and once
> for the stubdom ?
I've added a functions that does the libxl_device_disk_add loop, and
another one for the nics. Although this line you are quoting is not
adding a nic, it is just filling the needed values so we can launch Qemu
correctly.
> In the future we'll need that if we have more service domains, too.
>
> Thanks,
> Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 09/13] libxl: convert libxl_device_nic_add to an async operation
2012-05-22 13:49 ` Roger Pau Monne
@ 2012-05-22 14:04 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2012-05-22 14:04 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("Re: [PATCH 09/13] libxl: convert libxl_device_nic_add to an async operation"):
> Ian Jackson wrote:
> > Why do you serialise vif and vbd initialisation ? Would anything go
> > wrong if you did them both at once ?
>
> disk needs to be added before Qemu is launched, on the other hand nics
> need to be added after qemu is launched, so we need to add them at
> different moments during the domain creation, it's a PITA.
Ah I didn't spot that. Right.
> >> for (i = 0; i< dm_config->num_vifs; i++) {
> >> - ret = libxl_device_nic_add(ctx, dm_domid,&dm_config->vifs[i]);
> >> - if (ret)
> >> - goto out;
> >> + /* We have to init the nic here, because we still haven't
> >> + * called libxl_device_nic_add at this point, but qemu needs
> >> + * the nic information to be complete.
> >> + */
> >> + libxl__device_nic_setdefault(gc,&dm_config->vifs[i]);
> >
> > This is really too much repetition now. Can we not have a common "add
> > devices" function which is called once for the main domain and once
> > for the stubdom ?
>
> I've added a functions that does the libxl_device_disk_add loop, and
> another one for the nics. Although this line you are quoting is not
> adding a nic, it is just filling the needed values so we can launch Qemu
> correctly.
Right, OK. I guess making these two functions common would involve
macros or a lot of void*'s, which you would rather avoid.
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-21 16:29 ` Roger Pau Monne
@ 2012-05-29 14:40 ` Ian Jackson
2012-05-29 14:46 ` Ian Campbell
0 siblings, 1 reply; 35+ messages in thread
From: Ian Jackson @ 2012-05-29 14:40 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("Re: [PATCH 11/13] libxl: set nic type to VIF by default"):
> Ian Jackson wrote:
> > Roger Pau Monne writes ("[PATCH 11/13] libxl: set nic type to VIF by default"):
> >> - nic->nictype = LIBXL_NIC_TYPE_IOEMU;
> >> + nic->nictype = LIBXL_NIC_TYPE_VIF;
> >
> > But doesn't this set the default type to VIF even for HVM guests ?
>
> Shouldn't HVM guests use the "ioemu" parameter if they want an emulated
> card? If no parameter is provided then the default type should be VIF,
> because there's no other way to specifically set the type to VIF.
Sorry. I seem to have failed to reply to this.
I'm a bit confused about all this, I confess. What does xend do ?
ATM it seems that this change would mean that a guest config file
specified in the most obvious way wouldn't get an emulated nic at
all. That can't be right, can it ?
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-29 14:40 ` Ian Jackson
@ 2012-05-29 14:46 ` Ian Campbell
2012-05-29 15:02 ` Ian Jackson
2012-05-30 12:03 ` Roger Pau Monne
0 siblings, 2 replies; 35+ messages in thread
From: Ian Campbell @ 2012-05-29 14:46 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Roger Pau Monne
On Tue, 2012-05-29 at 15:40 +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [PATCH 11/13] libxl: set nic type to VIF by default"):
> > Ian Jackson wrote:
> > > Roger Pau Monne writes ("[PATCH 11/13] libxl: set nic type to VIF by default"):
> > >> - nic->nictype = LIBXL_NIC_TYPE_IOEMU;
> > >> + nic->nictype = LIBXL_NIC_TYPE_VIF;
> > >
> > > But doesn't this set the default type to VIF even for HVM guests ?
> >
> > Shouldn't HVM guests use the "ioemu" parameter if they want an emulated
> > card? If no parameter is provided then the default type should be VIF,
> > because there's no other way to specifically set the type to VIF.
>
> Sorry. I seem to have failed to reply to this.
>
> I'm a bit confused about all this, I confess. What does xend do ?
>
> ATM it seems that this change would mean that a guest config file
> specified in the most obvious way wouldn't get an emulated nic at
> all. That can't be right, can it ?
It's confusingly named.
IOEMU -> emulated only on HVM, meaningless on PV.
VIF -> both emulated and PV (on HVM) or just PV (on PV)
So a default of "VIF" is what you normally want.
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-29 14:46 ` Ian Campbell
@ 2012-05-29 15:02 ` Ian Jackson
2012-05-29 15:06 ` Ian Campbell
2012-05-30 12:03 ` Roger Pau Monne
1 sibling, 1 reply; 35+ messages in thread
From: Ian Jackson @ 2012-05-29 15:02 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Roger Pau Monne
Ian Campbell writes ("Re: [Xen-devel] [PATCH 11/13] libxl: set nic type to VIF by default"):
> On Tue, 2012-05-29 at 15:40 +0100, Ian Jackson wrote:
...
> > I'm a bit confused about all this, I confess. What does xend do ?
> >
> > ATM it seems that this change would mean that a guest config file
> > specified in the most obvious way wouldn't get an emulated nic at
> > all. That can't be right, can it ?
>
> It's confusingly named.
>
> IOEMU -> emulated only on HVM, meaningless on PV.
> VIF -> both emulated and PV (on HVM) or just PV (on PV)
>
> So a default of "VIF" is what you normally want.
Oh!
Perhaps we could rename the enum values (and disrupt everyone's
patches) to fix this before we call the API stable.
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-29 15:02 ` Ian Jackson
@ 2012-05-29 15:06 ` Ian Campbell
0 siblings, 0 replies; 35+ messages in thread
From: Ian Campbell @ 2012-05-29 15:06 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Roger Pau Monne
On Tue, 2012-05-29 at 16:02 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 11/13] libxl: set nic type to VIF by default"):
> > On Tue, 2012-05-29 at 15:40 +0100, Ian Jackson wrote:
> ...
> > > I'm a bit confused about all this, I confess. What does xend do ?
> > >
> > > ATM it seems that this change would mean that a guest config file
> > > specified in the most obvious way wouldn't get an emulated nic at
> > > all. That can't be right, can it ?
> >
> > It's confusingly named.
> >
> > IOEMU -> emulated only on HVM, meaningless on PV.
> > VIF -> both emulated and PV (on HVM) or just PV (on PV)
> >
> > So a default of "VIF" is what you normally want.
>
> Oh!
>
> Perhaps we could rename the enum values (and disrupt everyone's
> patches) to fix this before we call the API stable.
As a "Nice To Have", perhaps. The stable API rules we have set don't
preclude us changing them later, we just have to provide a suitable
compat define if LIBXL_API_VERSION is set to a relevant value.
What are some better names? IOEMU, VIF and HYBRID? Or DUAL?
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-29 14:46 ` Ian Campbell
2012-05-29 15:02 ` Ian Jackson
@ 2012-05-30 12:03 ` Roger Pau Monne
2012-06-07 14:30 ` Ian Jackson
1 sibling, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2012-05-30 12:03 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian Jackson, xen-devel
Ian Campbell wrote:
> On Tue, 2012-05-29 at 15:40 +0100, Ian Jackson wrote:
>> Roger Pau Monne writes ("Re: [PATCH 11/13] libxl: set nic type to VIF by default"):
>>> Ian Jackson wrote:
>>>> Roger Pau Monne writes ("[PATCH 11/13] libxl: set nic type to VIF by default"):
>>>>> - nic->nictype = LIBXL_NIC_TYPE_IOEMU;
>>>>> + nic->nictype = LIBXL_NIC_TYPE_VIF;
>>>> But doesn't this set the default type to VIF even for HVM guests ?
>>> Shouldn't HVM guests use the "ioemu" parameter if they want an emulated
>>> card? If no parameter is provided then the default type should be VIF,
>>> because there's no other way to specifically set the type to VIF.
>> Sorry. I seem to have failed to reply to this.
>>
>> I'm a bit confused about all this, I confess. What does xend do ?
>>
>> ATM it seems that this change would mean that a guest config file
>> specified in the most obvious way wouldn't get an emulated nic at
>> all. That can't be right, can it ?
>
> It's confusingly named.
>
> IOEMU -> emulated only on HVM, meaningless on PV.
My understanding was that IOEMU meant both TAP and PV, and was only
valid for HVM guests, and VIF meant PV only and was valid for both
domain types?
I don't think this is right, this will mean VIF has a different meaning
depending on the type of domain, which is quite annoying.
If this is right, I have to change my hotplug patches, but this will be
a mess, because I have no way to tell if a domain is PV or HVM when
plugging in the devices, so if VIF is passed for both PV or PV+TAP I
will have no way of knowing that.
Should we create three different nic types? VIF (PV), IOEMU (TAP),
HYBRID (PV+TAP)?
> VIF -> both emulated and PV (on HVM) or just PV (on PV)
>
> So a default of "VIF" is what you normally want.
>
> Ian.
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-05-30 12:03 ` Roger Pau Monne
@ 2012-06-07 14:30 ` Ian Jackson
2012-06-11 14:05 ` Roger Pau Monne
0 siblings, 1 reply; 35+ messages in thread
From: Ian Jackson @ 2012-06-07 14:30 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: Ian Campbell, xen-devel
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 11/13] libxl: set nic type to VIF by default"):
> If this is right, I have to change my hotplug patches, but this will be
> a mess, because I have no way to tell if a domain is PV or HVM when
> plugging in the devices, so if VIF is passed for both PV or PV+TAP I
> will have no way of knowing that.
Hmmm.
> Should we create three different nic types? VIF (PV), IOEMU (TAP),
> HYBRID (PV+TAP)?
I think this might be better. Or alternatively, always provide PV,
and have a flag which says `do provide ioemu'.
I think it's very confusing if setting the `type' to `ioemu' rather
than `pv' means `do provide pv but provide ioemu as well'.
Another way of looking at it is: if we wanted to provide some other
interfaces (besides Xen PV and emulated PCI) in the future, would it
be an alternative to PV or to PCI or something additional ?
Ian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/13] libxl: set nic type to VIF by default
2012-06-07 14:30 ` Ian Jackson
@ 2012-06-11 14:05 ` Roger Pau Monne
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monne @ 2012-06-11 14:05 UTC (permalink / raw)
To: Ian Jackson; +Cc: Ian Campbell, xen-devel
Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 11/13] libxl: set nic type to VIF by default"):
>> If this is right, I have to change my hotplug patches, but this will be
>> a mess, because I have no way to tell if a domain is PV or HVM when
>> plugging in the devices, so if VIF is passed for both PV or PV+TAP I
>> will have no way of knowing that.
>
> Hmmm.
>
>> Should we create three different nic types? VIF (PV), IOEMU (TAP),
>> HYBRID (PV+TAP)?
>
> I think this might be better. Or alternatively, always provide PV,
> and have a flag which says `do provide ioemu'.
This looks reasonable given the current types of nic interfaces.
>
> I think it's very confusing if setting the `type' to `ioemu' rather
> than `pv' means `do provide pv but provide ioemu as well'.
>
> Another way of looking at it is: if we wanted to provide some other
> interfaces (besides Xen PV and emulated PCI) in the future, would it
> be an alternative to PV or to PCI or something additional ?
I have no idea, could someone shed some light on this?
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2012-06-11 14:05 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-16 16:11 [PATCH 0/13] execute hotplug scripts from libxl Roger Pau Monne
2012-05-16 16:11 ` [PATCH 01/13] libxl: pass env vars to libxl__exec Roger Pau Monne
2012-05-18 16:02 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 02/13] libxl: fix libxl__xs_directory usage of transaction Roger Pau Monne
2012-05-18 16:03 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 03/13] libxl: add libxl__xs_path_cleanup Roger Pau Monne
2012-05-18 16:06 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 04/13] libxl: move libxl_device_disk_add to libxl_device Roger Pau Monne
2012-05-16 16:11 ` [PATCH 05/13] libxl: move libxl_device_nic_add " Roger Pau Monne
2012-05-16 16:11 ` [PATCH 06/13] libxl: cleanup libxl__device_{disk, nic}_add Roger Pau Monne
2012-05-18 16:07 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 07/13] libxl: convert libxl_domain_destroy to an AO op Roger Pau Monne
2012-05-18 16:19 ` Ian Jackson
2012-05-18 16:24 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 08/13] libxl: convert libxl_device_disk_add to an async operation Roger Pau Monne
2012-05-18 16:33 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 09/13] libxl: convert libxl_device_nic_add " Roger Pau Monne
2012-05-18 16:38 ` Ian Jackson
2012-05-22 13:49 ` Roger Pau Monne
2012-05-22 14:04 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 10/13] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
2012-05-18 16:40 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 11/13] libxl: set nic type to VIF by default Roger Pau Monne
2012-05-18 16:41 ` Ian Jackson
2012-05-21 16:29 ` Roger Pau Monne
2012-05-29 14:40 ` Ian Jackson
2012-05-29 14:46 ` Ian Campbell
2012-05-29 15:02 ` Ian Jackson
2012-05-29 15:06 ` Ian Campbell
2012-05-30 12:03 ` Roger Pau Monne
2012-06-07 14:30 ` Ian Jackson
2012-06-11 14:05 ` Roger Pau Monne
2012-05-16 16:11 ` [PATCH 12/13] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
2012-05-18 16:51 ` Ian Jackson
2012-05-16 16:11 ` [PATCH 13/13] libxl: call hotplug scripts for nic " Roger Pau Monne
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.