All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v12 00/17] execute hotplug scripts from libxl
@ 2012-07-25 11:19 Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 01/17] libxl: fix removal of secondary consoles Roger Pau Monne
                   ` (18 more replies)
  0 siblings, 19 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel

Reworked patch 1 and 9, and fixed the local attach problems. Not 
having an Ack on patch 1 should not prevent the rest from going in.

Patches 4, 8, 11 and 16, that are already Acked can be commited, since 
they don't require the previous patches, and should apply cleanly 
since they touch different parts of the code.

Changes are available in the git repository at:
  git://xenbits.xen.org/people/royger/xen.git hotplug.v12

Roger Pau Monne (17):
      libxl: fix removal of secondary consoles
      libxl: refactor disk addition to take a helper
      libxl: convert libxl__device_disk_local_attach to an async op
     *libxl: rename vifs to nics
     *libxl: convert libxl_device_disk_add to an async op
     *libxl: convert libxl_device_nic_add to an async operation
     *libxl: add option to choose who executes hotplug scripts
     *libxl: rename _IOEMU nic type to VIF_IOEMU
      libxl: set nic type of stub to PV instead of copying from the 
      libxl: set correct nic type depending on the guest
     *libxl: use libxl__xs_path_cleanup on device_destroy
     *libxl: call hotplug scripts for disk devices from libxl
     *libxl: call hotplug scripts for nic devices from libxl
      libxl: convert libxl_device_vkb_add to an async operation
      libxl: convert libxl_device_vfb_add to an async operation
     *xl: main_blockdetach don't call destroy if remove succeeds
      libxl: libxl__xs_path_cleanup don't print error if ENOENT

* Acked

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v12 01/17] libxl: fix removal of secondary consoles
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-26 10:39   ` Ian Campbell
  2012-07-26 10:43   ` Ian Campbell
  2012-07-25 11:19 ` [PATCH v12 02/17] libxl: refactor disk addition to take a helper Roger Pau Monne
                   ` (17 subsequent siblings)
  18 siblings, 2 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

Secondary consoles are processed by libxl with the rest of the
devices by calling libxl__initiate_device_remove that waits for the
device to reach state 6 before procceeding with the removal.

When libxl is destroying the console devices, Qemu is already dead or
dying, and xenconsoled completely ignores the state backend entry for
console devices, since it performs the cleanup based on the result of
reads/writes to the tty.

Since we don't want to execute hotplug scripts for consoles, leave the
behaviour as it was previously, and just nuke the backend/frontend
xenstore entries by calling libxl__device_destroy.

Report: http://markmail.org/message/yqgppcsdip6tnmh6

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Reported-by: Ian Campbell <ian.campbell@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl_device.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index a94beab..2120cfb 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -81,6 +81,8 @@ static int libxl__num_devices(libxl__gc *gc, uint32_t domid)
     for (i = 0; i < num_kinds; i++) {
         if (libxl__device_kind_from_string(kinds[i], &kind))
             continue;
+        if (kind == LIBXL__DEVICE_KIND_CONSOLE)
+            continue;
 
         path = GCSPRINTF("/local/domain/%d/device/%s", domid, kinds[i]);
         devs = libxl__xs_directory(gc, XBT_NULL, path, &num_devs);
@@ -512,10 +514,18 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
             path = libxl__xs_read(gc, XBT_NULL, path);
             GCNEW(dev);
             if (path && libxl__parse_backend_path(gc, path, dev) == 0) {
-                aodev = &aodevs->array[numdev];
                 dev->domid = domid;
                 dev->kind = kind;
                 dev->devid = atoi(devs[j]);
+                if (dev->backend_kind == LIBXL__DEVICE_KIND_CONSOLE) {
+                    /* Currently console devices can be destroyed
+                     * synchronously by just removing xenstore entries,
+                     * this is what libxl__device_destroy does.
+                     */
+                    libxl__device_destroy(gc, dev);
+                    continue;
+                }
+                aodev = &aodevs->array[numdev];
                 aodev->action = DEVICE_DISCONNECT;
                 aodev->dev = dev;
                 aodev->callback = libxl__ao_devices_callback;
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 02/17] libxl: refactor disk addition to take a helper
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 01/17] libxl: fix removal of secondary consoles Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-26 11:06   ` Ian Jackson
  2012-07-25 11:19 ` [PATCH v12 03/17] libxl: convert libxl__device_disk_local_attach to an async op Roger Pau Monne
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

Change libxl__device_disk_add to no longer take a xs transaction and
instead pass a helper for the local attach case that's used to get the
free vdev.

This function contains some non-functional changes due to an
indentation change.

Changes since v10:

 * Changed name/type of params of device_disk_add.

Changes since v9:

 * Added better comment for device_disk_add parameters.

 * Changed char *fn to void *get_vdev_user

 * Removed error log message.

Changes since v7:

 * Fixed the use of the transaction inside device_disk_add (which in
   v7 was fixed in the next patch).

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c          |  267 +++++++++++++++++++++++-------------------
 tools/libxl/libxl_internal.h |    2 +-
 2 files changed, 146 insertions(+), 123 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 7e8bf45..c8ce40b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1757,115 +1757,152 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
-int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
-        xs_transaction_t t, libxl_device_disk *disk)
-{
-    flexarray_t *front;
-    flexarray_t *back;
+/* Specific function called directly only by local disk attach,
+ * all other users should instead use the regular
+ * libxl__device_disk_add wrapper
+ *
+ * The (optionally) passed function get_vdev will be used to
+ * set the vdev the disk should be attached to. When it is set the caller
+ * must also pass get_vdev_user, which will be passed to get_vdev.
+ *
+ * The passed get_vdev function is also in charge of printing
+ * the corresponding error message when appropiate.
+ */
+static int device_disk_add(libxl__gc *gc, uint32_t domid,
+                           libxl_device_disk *disk,
+                           void *get_vdev(libxl__gc *, void *,
+                                          xs_transaction_t),
+                           void *get_vdev_user)
+{
+    flexarray_t *front = NULL;
+    flexarray_t *back = NULL;
     char *dev;
     libxl__device device;
     int major, minor, rc;
     libxl_ctx *ctx = gc->owner;
+    xs_transaction_t t = XBT_NULL;
 
-    rc = libxl__device_disk_setdefault(gc, disk);
-    if (rc) goto out;
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        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 (get_vdev) {
+            assert(get_vdev_user);
+            disk->vdev = (char *) get_vdev(gc, get_vdev_user, t);
+            if (disk->vdev == NULL) {
+                rc = ERROR_FAIL;
+                goto out;
+            }
+        }
 
-    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_disk_setdefault(gc, disk);
+        if (rc) goto out;
 
-    rc = libxl__device_from_disk(gc, domid, disk, &device);
-    if (rc != 0) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->vdev);
-        goto out_free;
-    }
+        if (front)
+            flexarray_free(front);
+        front = flexarray_make(16, 1);
+        if (!front) {
+            rc = ERROR_NOMEM;
+            goto out;
+        }
+        if (back)
+            flexarray_free(back);
+        back = flexarray_make(16, 1);
+        if (!back) {
+            rc = ERROR_NOMEM;
+            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));
+        if (disk->script) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
+                       " not yet supported, sorry");
+            rc = ERROR_INVAL;
+            goto out_free;
+        }
 
-            flexarray_append(back, "params");
-            flexarray_append(back, dev);
+        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;
+        }
 
-            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));
+        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));
 
-            /* 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, "params");
+                flexarray_append(back, dev);
 
-    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");
+                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(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");
+        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, t, &device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+        libxl__device_generic_add(gc, t, &device,
+                            libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                            libxl__xs_kvs_of_flexarray(gc, front, front->count));
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out_free;
+    }
 
     rc = 0;
 
@@ -1873,13 +1910,20 @@ out_free:
     flexarray_free(back);
     flexarray_free(front);
 out:
+    libxl__xs_transaction_abort(gc, &t);
     return rc;
 }
 
+int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+                           libxl_device_disk *disk)
+{
+    return device_disk_add(gc, domid, disk, NULL, NULL);
+}
+
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
 {
     GC_INIT(ctx);
-    int rc = libxl__device_disk_add(gc, domid, XBT_NULL, disk);
+    int rc = libxl__device_disk_add(gc, domid, disk);
     GC_FREE;
     return rc;
 }
@@ -2137,9 +2181,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
     libxl_ctx *ctx = gc->owner;
     char *dev = NULL, *be_path = NULL;
     char *ret = NULL;
-    int rc, xs_ret;
+    int rc;
     libxl__device device;
-    xs_transaction_t t = XBT_NULL;
 
     if (in_disk->pdev_path == NULL)
         return NULL;
@@ -2183,27 +2226,10 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             break;
         case LIBXL_DISK_BACKEND_QDISK:
             if (disk->format != LIBXL_DISK_FORMAT_RAW) {
-                do {
-                    t = xs_transaction_start(ctx->xsh);
-                    if (t == XBT_NULL) {
-                        LOG(ERROR, "failed to start a xenstore transaction");
-                        goto out;
-                    }
-                    disk->vdev = libxl__alloc_vdev(gc, blkdev_start, t);
-                    if (disk->vdev == NULL) {
-                        LOG(ERROR, "libxl__alloc_vdev failed");
-                        goto out;
-                    }
-                    if (libxl__device_disk_add(gc, LIBXL_TOOLSTACK_DOMID,
-                                t, disk)) {
-                        LOG(ERROR, "libxl_device_disk_add failed");
-                        goto out;
-                    }
-                    xs_ret = xs_transaction_end(ctx->xsh, t, 0);
-                } while (xs_ret == 0 && errno == EAGAIN);
-                t = XBT_NULL;
-                if (xs_ret == 0) {
-                    LOGE(ERROR, "xenstore transaction failed");
+                if (device_disk_add(gc, LIBXL_TOOLSTACK_DOMID, disk,
+                                    (void *) libxl__alloc_vdev,
+                                    (void *) blkdev_start)) {
+                    LOG(ERROR, "libxl_device_disk_add failed");
                     goto out;
                 }
                 dev = GCSPRINTF("/dev/%s", disk->vdev);
@@ -2232,10 +2258,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
     return ret;
 
  out:
-    if (t != XBT_NULL)
-        xs_transaction_end(ctx->xsh, t, 1);
-    else
-        libxl__device_disk_local_detach(gc, disk);
+    libxl__device_disk_local_detach(gc, disk);
     return NULL;
 }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f832fbd..35e889f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1298,7 +1298,7 @@ _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,
-        xs_transaction_t t, libxl_device_disk *disk);
+                                   libxl_device_disk *disk);
 
 /*
  * Make a disk available in this (the control) domain. Returns path to
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 03/17] libxl: convert libxl__device_disk_local_attach to an async op
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 01/17] libxl: fix removal of secondary consoles Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 02/17] libxl: refactor disk addition to take a helper Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-26 11:07   ` Ian Jackson
  2012-07-25 11:19 ` [PATCH v12 04/17] libxl: rename vifs to nics Roger Pau Monne
                   ` (15 subsequent siblings)
  18 siblings, 1 reply; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Ian Jackson, Roger Pau Monne

This will be needed in future patches, when libxl__device_disk_add
becomes async also. Create a new status structure that defines the
local attach of a disk device and use it in
libxl__device_disk_local_attach.

This is done in this patch to split the changes introduced when
libxl__device_disk_add becomes async.

Changes since v11:

 * Correctly pass error in libxl__device_disk_local_initiate_detach.

Changes since v10:

 * Convert breaks to gotos in error paths of
   libxl__device_disk_local_initiate_attach.

 * Unify exit path of libxl__device_disk_local_detach.

 * Redo some comments.

Changes since v9:

 * Fixed state changes comments on header of functions.

 * Rework libxl__device_disk_local_init to just set rc to 0, and
   perform the initialization in the callers context.

 * If attach fails, perform a detach, so the state of the local_device
   on the callback is Attached if rc == 0, or Idle if rc != 0.

 * Free diskpath inside the detach callback.

Changes since v6:

 * Added better comments.

 * Instead of passing the xs_transaction arround several functions,
   put all the xs related operations inside device_disk_add.

 * Make sure libxl__device_disk_local_initiate_detach either calls the
   async device destroy function, or the callback.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c            |   98 ++++++++++++++++++++++++++++++---------
 tools/libxl/libxl_bootloader.c |   73 ++++++++++++++++++++++++------
 tools/libxl/libxl_internal.h   |   72 +++++++++++++++++++++++------
 3 files changed, 192 insertions(+), 51 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index c8ce40b..e8ef581 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2173,19 +2173,19 @@ static char * libxl__alloc_vdev(libxl__gc *gc, const char *blkdev_start,
     return NULL;
 }
 
-char * libxl__device_disk_local_attach(libxl__gc *gc,
-        const libxl_device_disk *in_disk,
-        libxl_device_disk *disk,
-        const char *blkdev_start)
+void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
+                                     libxl__disk_local_state *dls)
 {
-    libxl_ctx *ctx = gc->owner;
+    STATE_AO_GC(dls->ao);
+    libxl_ctx *ctx = CTX;
     char *dev = NULL, *be_path = NULL;
-    char *ret = NULL;
     int rc;
     libxl__device device;
+    const libxl_device_disk *in_disk = dls->in_disk;
+    libxl_device_disk *disk = &dls->disk;
+    const char *blkdev_start = dls->blkdev_start;
 
-    if (in_disk->pdev_path == NULL)
-        return NULL;
+    assert(in_disk->pdev_path);
 
     memcpy(disk, in_disk, sizeof(libxl_device_disk));
     disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
@@ -2221,7 +2221,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             default:
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                            "unrecognized disk format: %d", disk->format);
-                break;
+                rc = ERROR_FAIL;
+                goto out;
             }
             break;
         case LIBXL_DISK_BACKEND_QDISK:
@@ -2230,6 +2231,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
                                     (void *) libxl__alloc_vdev,
                                     (void *) blkdev_start)) {
                     LOG(ERROR, "libxl_device_disk_add failed");
+                    rc = ERROR_FAIL;
                     goto out;
                 }
                 dev = GCSPRINTF("/dev/%s", disk->vdev);
@@ -2241,7 +2243,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
                 "type: %d", disk->backend);
-            break;
+            rc = ERROR_FAIL;
+            goto out;
     }
 
     if (disk->vdev != NULL) {
@@ -2254,39 +2257,88 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             goto out;
     }
     if (dev != NULL)
-        ret = strdup(dev);
-    return ret;
+        dls->diskpath = libxl__strdup(gc, dev);
+
+    dls->callback(egc, dls, 0);
+    return;
 
  out:
-    libxl__device_disk_local_detach(gc, disk);
-    return NULL;
+    assert(rc);
+    dls->rc = rc;
+    libxl__device_disk_local_initiate_detach(egc, dls);
+    return;
 }
 
-int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk *disk)
+/* Callbacks for local detach */
+
+static void local_device_detach_cb(libxl__egc *egc, libxl__ao_device *aodev);
+
+void libxl__device_disk_local_initiate_detach(libxl__egc *egc,
+                                     libxl__disk_local_state *dls)
 {
+    STATE_AO_GC(dls->ao);
     int rc = 0;
+    libxl_device_disk *disk = &dls->disk;
+    libxl__device *device;
+    libxl__ao_device *aodev = &dls->aodev;
+    libxl__prepare_ao_device(ao, aodev);
+
+    if (!dls->diskpath) goto out;
 
     switch (disk->backend) {
         case LIBXL_DISK_BACKEND_QDISK:
             if (disk->vdev != NULL) {
-                libxl_device_disk_remove(gc->owner, LIBXL_TOOLSTACK_DOMID,
-                        disk, 0);
-                /* fixme-ao */
-                rc = libxl_device_disk_destroy(gc->owner,
-                        LIBXL_TOOLSTACK_DOMID, disk, 0);
+                GCNEW(device);
+                rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID,
+                                             disk, device);
+                if (rc != 0) goto out;
+
+                aodev->action = DEVICE_DISCONNECT;
+                aodev->dev = device;
+                aodev->callback = local_device_detach_cb;
+                aodev->force = 0;
+                libxl__initiate_device_remove(egc, aodev);
+                return;
             }
-            break;
+            /* disk->vdev == NULL; fall through */
         default:
             /*
              * Nothing to do for PHYSTYPE_PHY.
              * For other device types assume that the blktap2 process is
              * needed by the soon to be started domain and do nothing.
              */
-            break;
+            goto out;
     }
 
+out:
+    aodev->rc = rc;
+    local_device_detach_cb(egc, aodev);
+    return;
+}
 
-    return rc;
+static void local_device_detach_cb(libxl__egc *egc, libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__disk_local_state *dls = CONTAINER_OF(aodev, *dls, aodev);
+    int rc;
+
+    if (aodev->rc) {
+        LOGE(ERROR, "unable to %s %s with id %u",
+                    aodev->action == DEVICE_CONNECT ? "add" : "remove",
+                    libxl__device_kind_to_string(aodev->dev->kind),
+                    aodev->dev->devid);
+        goto out;
+    }
+
+out:
+    /*
+     * If there was an error in dls->rc, it means we have been called from
+     * a failed execution of libxl__device_disk_local_initiate_attach,
+     * so return the original error.
+     */
+    rc = dls->rc ? dls->rc : aodev->rc;
+    dls->callback(egc, dls, rc);
+    return;
 }
 
 /******************************************************************************/
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 7ebc0df..ef5a91b 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -75,7 +75,7 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
         }
     }
 
-    ARG(bl->diskpath);
+    ARG(bl->dls.diskpath);
 
     /* Sentinel for execv */
     ARG(NULL);
@@ -206,8 +206,9 @@ static int parse_bootloader_result(libxl__egc *egc,
 void libxl__bootloader_init(libxl__bootloader_state *bl)
 {
     assert(bl->ao);
-    bl->diskpath = NULL;
+    bl->dls.diskpath = NULL;
     bl->openpty.ao = bl->ao;
+    bl->dls.ao = bl->ao;
     bl->ptys[0].master = bl->ptys[0].slave = 0;
     bl->ptys[1].master = bl->ptys[1].slave = 0;
     libxl__ev_child_init(&bl->child);
@@ -224,11 +225,6 @@ static void bootloader_cleanup(libxl__egc *egc, libxl__bootloader_state *bl)
     if (bl->outputpath) libxl__remove_file(gc, bl->outputpath);
     if (bl->outputdir) libxl__remove_directory(gc, bl->outputdir);
 
-    if (bl->diskpath) {
-        libxl__device_disk_local_detach(gc, &bl->localdisk);
-        free(bl->diskpath);
-        bl->diskpath = 0;
-    }
     libxl__domaindeathcheck_stop(gc,&bl->deathcheck);
     libxl__datacopier_kill(&bl->keystrokes);
     libxl__datacopier_kill(&bl->display);
@@ -249,10 +245,32 @@ static void bootloader_setpaths(libxl__gc *gc, libxl__bootloader_state *bl)
     bl->outputpath = GCSPRINTF(XEN_RUN_DIR "/bootloader.%"PRIu32".out", domid);
 }
 
+/* Callbacks */
+
+static void bootloader_local_detached_cb(libxl__egc *egc,
+                                         libxl__disk_local_state *dls,
+                                         int rc);
+
 static void bootloader_callback(libxl__egc *egc, libxl__bootloader_state *bl,
                                 int rc)
 {
     bootloader_cleanup(egc, bl);
+
+    bl->dls.callback = bootloader_local_detached_cb;
+    libxl__device_disk_local_initiate_detach(egc, &bl->dls);
+}
+
+static void bootloader_local_detached_cb(libxl__egc *egc,
+                                         libxl__disk_local_state *dls,
+                                         int rc)
+{
+    STATE_AO_GC(dls->ao);
+    libxl__bootloader_state *bl = CONTAINER_OF(dls, *bl, dls);
+
+    if (rc) {
+        LOG(ERROR, "unable to detach locally attached disk");
+    }
+
     bl->callback(egc, bl, rc);
 }
 
@@ -275,6 +293,12 @@ static void bootloader_abort(libxl__egc *egc,
 
 /*----- main flow of control -----*/
 
+/* Callbacks */
+
+static void bootloader_disk_attached_cb(libxl__egc *egc,
+                                        libxl__disk_local_state *dls,
+                                        int rc);
+
 void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
 {
     STATE_AO_GC(bl->ao);
@@ -282,7 +306,6 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
     uint32_t domid = bl->domid;
     char *logfile_tmp = NULL;
     int rc, r;
-    const char *bootloader;
 
     libxl__bootloader_init(bl);
 
@@ -344,10 +367,34 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
         goto out;
     }
 
-    bl->diskpath = libxl__device_disk_local_attach(gc, bl->disk, &bl->localdisk,
-            info->blkdev_start);
-    if (!bl->diskpath) {
-        rc = ERROR_FAIL;
+
+    /* This sets the state of the dls struct from Undefined to Idle */
+    libxl__device_disk_local_init(&bl->dls);
+    bl->dls.ao = ao;
+    bl->dls.in_disk = bl->disk;
+    bl->dls.blkdev_start = info->blkdev_start;
+    bl->dls.callback = bootloader_disk_attached_cb;
+    libxl__device_disk_local_initiate_attach(egc, &bl->dls);
+    return;
+
+ out:
+    assert(rc);
+ out_ok:
+    free(logfile_tmp);
+    bootloader_callback(egc, bl, rc);
+}
+
+static void bootloader_disk_attached_cb(libxl__egc *egc,
+                                        libxl__disk_local_state *dls,
+                                        int rc)
+{
+    STATE_AO_GC(dls->ao);
+    libxl__bootloader_state *bl = CONTAINER_OF(dls, *bl, dls);
+    const libxl_domain_build_info *info = bl->info;
+    const char *bootloader;
+
+    if (rc) {
+        LOG(ERROR, "failed to attach local disk for bootloader execution");
         goto out;
     }
 
@@ -389,8 +436,6 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
 
  out:
     assert(rc);
- out_ok:
-    free(logfile_tmp);
     bootloader_callback(egc, bl, rc);
 }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 35e889f..cb2975a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1300,17 +1300,6 @@ _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);
 
-/*
- * Make a disk available in this (the control) domain. Returns path to
- * a device.
- */
-_hidden char * libxl__device_disk_local_attach(libxl__gc *gc,
-        const libxl_device_disk *in_disk,
-        libxl_device_disk *new_disk,
-        const char *blkdev_start);
-_hidden int libxl__device_disk_local_detach(libxl__gc *gc,
-        libxl_device_disk *disk);
-
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);
 
 struct libxl__xen_console_reader {
@@ -1908,6 +1897,62 @@ struct libxl__ao_devices {
 _hidden void libxl__initiate_device_remove(libxl__egc *egc,
                                            libxl__ao_device *aodev);
 
+/*----- local disk attach: attach a disk locally to run the bootloader -----*/
+
+typedef struct libxl__disk_local_state libxl__disk_local_state;
+typedef void libxl__disk_local_state_callback(libxl__egc*,
+                                              libxl__disk_local_state*,
+                                              int rc);
+
+/* A libxl__disk_local_state may be in the following states:
+ * Undefined, Idle, Attaching, Attached, Detaching.
+ */
+struct libxl__disk_local_state {
+    /* filled by the user */
+    libxl__ao *ao;
+    const libxl_device_disk *in_disk;
+    libxl_device_disk disk;
+    const char *blkdev_start;
+    libxl__disk_local_state_callback *callback;
+    /* filled by libxl__device_disk_local_initiate_attach */
+    char *diskpath;
+    /* private for implementation of local detach */
+    libxl__ao_device aodev;
+    int rc;
+};
+
+/*
+ * Prepares a dls for use.
+ * State Undefined -> Idle
+ */
+static inline void libxl__device_disk_local_init(libxl__disk_local_state *dls)
+{
+    dls->rc = 0;
+}
+
+/* Make a disk available in this (the control) domain. Always calls
+ * dls->callback when finished.
+ * State Idle -> Attaching
+ *
+ * The state of dls on entry to the callback depends on the value
+ * of rc passed to the callback:
+ *     rc == 0: Attached if rc == 0
+ *     rc != 0: Idle
+ */
+_hidden void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
+                                                libxl__disk_local_state *dls);
+
+/* Disconnects a disk device form the control domain. If the passed
+ * dls is not attached (or has already been detached),
+ * libxl__device_disk_local_initiate_detach will just call the callback
+ * directly.
+ * State Idle/Attached -> Detaching
+ *
+ * The state of dls on entry to the callback is Idle.
+ */
+_hidden void libxl__device_disk_local_initiate_detach(libxl__egc *egc,
+                                                libxl__disk_local_state *dls);
+
 /*----- datacopier: copies data from one fd to another -----*/
 
 typedef struct libxl__datacopier_state libxl__datacopier_state;
@@ -2097,10 +2142,10 @@ struct libxl__bootloader_state {
     /* Should be zeroed by caller on entry.  Will be filled in by
      * bootloader machinery; represents the local attachment of the
      * disk for the benefit of the bootloader.  Must be detached by
-     * the caller using libxl__device_disk_local_detach, but only
+     * the caller using libxl__device_disk_local_initiate_detach, but only
      * after the domain's kernel and initramfs have been loaded into
      * memory and the file references disposed of. */
-    libxl_device_disk localdisk;
+    libxl__disk_local_state dls;
     uint32_t domid;
     /* outputs:
      *  - caller must initialise kernel and ramdisk to point to file
@@ -2112,7 +2157,6 @@ struct libxl__bootloader_state {
     const char *cmdline;
     /* private to libxl__run_bootloader */
     char *outputpath, *outputdir, *logfile;
-    char *diskpath; /* not from gc, represents actually attached disk */
     libxl__openpty_state openpty;
     libxl__openpty_result ptys[2];  /* [0] is for bootloader */
     libxl__ev_child child;
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 04/17] libxl: rename vifs to nics
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (2 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 03/17] libxl: convert libxl__device_disk_local_attach to an async op Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 05/17] libxl: convert libxl_device_disk_add to an async op Roger Pau Monne
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

This change renames functions and struct values that used to contain
vifs in their names to nics, that provides a more clear name to
define network interfaces without referring to the backend that is
behind them.

This is not a functional change.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.h        |    4 +-
 tools/libxl/libxl_create.c |   10 +++---
 tools/libxl/libxl_dm.c     |   64 ++++++++++++++++++++++----------------------
 tools/libxl/libxl_json.c   |    8 +++---
 tools/libxl/xl_cmdimpl.c   |   14 +++++-----
 tools/libxl/xl_sxp.c       |   20 +++++++-------
 6 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5c819f1..c730ac5 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -470,10 +470,10 @@ typedef struct {
     libxl_domain_create_info c_info;
     libxl_domain_build_info b_info;
 
-    int num_disks, num_vifs, num_pcidevs, num_vfbs, num_vkbs;
+    int num_disks, num_nics, num_pcidevs, num_vfbs, num_vkbs;
 
     libxl_device_disk *disks;
-    libxl_device_nic *vifs;
+    libxl_device_nic *nics;
     libxl_device_pci *pcidevs;
     libxl_device_vfb *vfbs;
     libxl_device_vkb *vkbs;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 137659f..0c81a60 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -39,9 +39,9 @@ void libxl_domain_config_dispose(libxl_domain_config *d_config)
         libxl_device_disk_dispose(&d_config->disks[i]);
     free(d_config->disks);
 
-    for (i=0; i<d_config->num_vifs; i++)
-        libxl_device_nic_dispose(&d_config->vifs[i]);
-    free(d_config->vifs);
+    for (i=0; i<d_config->num_nics; i++)
+        libxl_device_nic_dispose(&d_config->nics[i]);
+    free(d_config->nics);
 
     for (i=0; i<d_config->num_pcidevs; i++)
         libxl_device_pci_dispose(&d_config->pcidevs[i]);
@@ -873,8 +873,8 @@ static void domcreate_rebuild_done(libxl__egc *egc,
             goto error_out;
         }
     }
-    for (i = 0; i < d_config->num_vifs; i++) {
-        ret = libxl_device_nic_add(ctx, domid, &d_config->vifs[i]);
+    for (i = 0; i < d_config->num_nics; i++) {
+        ret = libxl_device_nic_add(ctx, domid, &d_config->nics[i]);
         if (ret) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                        "cannot add nic %d to domain: %d", i, ret);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 2c81d5c..0e2ec18 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -102,10 +102,10 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
 {
     const libxl_domain_create_info *c_info = &guest_config->c_info;
     const libxl_domain_build_info *b_info = &guest_config->b_info;
-    const libxl_device_nic *vifs = guest_config->vifs;
+    const libxl_device_nic *nics = guest_config->nics;
     const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config);
     const libxl_sdl_info *sdl = dm_sdl(guest_config);
-    const int num_vifs = guest_config->num_vifs;
+    const int num_nics = guest_config->num_nics;
     const char *keymap = dm_keymap(guest_config);
     int i;
     flexarray_t *dm_args;
@@ -159,7 +159,7 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
         flexarray_vappend(dm_args, "-k", keymap, NULL);
     }
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        int ioemu_vifs = 0;
+        int ioemu_nics = 0;
         int nr_set_cpus = 0;
         char *s;
 
@@ -214,31 +214,31 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
                               libxl__sprintf(gc, "%s", s), NULL);
         free(s);
 
-        for (i = 0; i < num_vifs; i++) {
-            if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
+        for (i = 0; i < num_nics; i++) {
+            if (nics[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
                 char *smac = libxl__sprintf(gc,
-                                   LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
+                                   LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nics[i].mac));
                 const char *ifname = libxl__device_nic_devname(gc,
-                                                domid, vifs[i].devid,
+                                                domid, nics[i].devid,
                                                 LIBXL_NIC_TYPE_IOEMU);
                 flexarray_vappend(dm_args,
                                   "-net",
                                   GCSPRINTF(
                                       "nic,vlan=%d,macaddr=%s,model=%s",
-                                      vifs[i].devid, smac, vifs[i].model),
+                                      nics[i].devid, smac, nics[i].model),
                                   "-net",
                                   GCSPRINTF(
                                       "tap,vlan=%d,ifname=%s,bridge=%s,"
                                       "script=%s,downscript=%s",
-                                      vifs[i].devid, ifname, vifs[i].bridge,
+                                      nics[i].devid, ifname, nics[i].bridge,
                                       libxl_tapif_script(gc),
                                       libxl_tapif_script(gc)),
                                   NULL);
-                ioemu_vifs++;
+                ioemu_nics++;
             }
         }
         /* If we have no emulated nics, tell qemu not to create any */
-        if ( ioemu_vifs == 0 ) {
+        if ( ioemu_nics == 0 ) {
             flexarray_vappend(dm_args, "-net", "none", NULL);
         }
         if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
@@ -330,9 +330,9 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     const libxl_domain_create_info *c_info = &guest_config->c_info;
     const libxl_domain_build_info *b_info = &guest_config->b_info;
     const libxl_device_disk *disks = guest_config->disks;
-    const libxl_device_nic *vifs = guest_config->vifs;
+    const libxl_device_nic *nics = guest_config->nics;
     const int num_disks = guest_config->num_disks;
-    const int num_vifs = guest_config->num_vifs;
+    const int num_nics = guest_config->num_nics;
     const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config);
     const libxl_sdl_info *sdl = dm_sdl(guest_config);
     const char *keymap = dm_keymap(guest_config);
@@ -409,7 +409,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     }
 
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        int ioemu_vifs = 0;
+        int ioemu_nics = 0;
 
         if (b_info->u.hvm.serial) {
             flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial, NULL);
@@ -468,30 +468,30 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                 flexarray_append(dm_args, libxl__sprintf(gc, "%d",
                                                          b_info->max_vcpus));
         }
-        for (i = 0; i < num_vifs; i++) {
-            if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
+        for (i = 0; i < num_nics; i++) {
+            if (nics[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
                 char *smac = libxl__sprintf(gc,
-                                LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
+                                LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nics[i].mac));
                 const char *ifname = libxl__device_nic_devname(gc,
-                                                guest_domid, vifs[i].devid,
+                                                guest_domid, nics[i].devid,
                                                 LIBXL_NIC_TYPE_IOEMU);
                 flexarray_append(dm_args, "-device");
                 flexarray_append(dm_args,
                    libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s",
-                                                vifs[i].model, vifs[i].devid,
-                                                vifs[i].devid, smac));
+                                                nics[i].model, nics[i].devid,
+                                                nics[i].devid, smac));
                 flexarray_append(dm_args, "-netdev");
                 flexarray_append(dm_args, GCSPRINTF(
                                           "type=tap,id=net%d,ifname=%s,"
                                           "script=%s,downscript=%s",
-                                          vifs[i].devid, ifname,
+                                          nics[i].devid, ifname,
                                           libxl_tapif_script(gc),
                                           libxl_tapif_script(gc)));
-                ioemu_vifs++;
+                ioemu_nics++;
             }
         }
         /* If we have no emulated nics, tell qemu not to create any */
-        if ( ioemu_vifs == 0 ) {
+        if ( ioemu_nics == 0 ) {
             flexarray_append(dm_args, "-net");
             flexarray_append(dm_args, "none");
         }
@@ -620,18 +620,18 @@ static void libxl__dm_vifs_from_hvm_guest_config(libxl__gc *gc,
                                     libxl_domain_config * const guest_config,
                                     libxl_domain_config *dm_config)
 {
-    int i, nr = guest_config->num_vifs;
+    int i, nr = guest_config->num_nics;
 
-    GCNEW_ARRAY(dm_config->vifs, nr);
+    GCNEW_ARRAY(dm_config->nics, nr);
 
     for (i=0; i<nr; i++) {
-        dm_config->vifs[i] = guest_config->vifs[i];
-        if (dm_config->vifs[i].ifname)
-            dm_config->vifs[i].ifname = GCSPRINTF("%s" TAP_DEVICE_SUFFIX,
-                                                  dm_config->vifs[i].ifname);
+        dm_config->nics[i] = guest_config->nics[i];
+        if (dm_config->nics[i].ifname)
+            dm_config->nics[i].ifname = GCSPRINTF("%s" TAP_DEVICE_SUFFIX,
+                                                  dm_config->nics[i].ifname);
     }
 
-    dm_config->num_vifs = nr;
+    dm_config->num_nics = nr;
 }
 
 static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
@@ -849,8 +849,8 @@ retry_transaction:
         if (ret)
             goto out_free;
     }
-    for (i = 0; i < dm_config->num_vifs; i++) {
-        ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->vifs[i]);
+    for (i = 0; i < dm_config->num_nics; i++) {
+        ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->nics[i]);
         if (ret)
             goto out_free;
     }
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index e870606..caa8312 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -822,15 +822,15 @@ yajl_gen_status libxl_domain_config_gen_json(yajl_gen hand,
     if (s != yajl_gen_status_ok)
         goto out;
 
-    s = yajl_gen_string(hand, (const unsigned char *)"vifs",
-                        sizeof("vifs")-1);
+    s = yajl_gen_string(hand, (const unsigned char *)"nics",
+                        sizeof("nics")-1);
     if (s != yajl_gen_status_ok)
         goto out;
     s = yajl_gen_array_open(hand);
     if (s != yajl_gen_status_ok)
         goto out;
-    for (i = 0; i < p->num_vifs; i++) {
-        s = libxl_device_nic_gen_json(hand, &p->vifs[i]);
+    for (i = 0; i < p->num_nics; i++) {
+        s = libxl_device_nic_gen_json(hand, &p->nics[i]);
         if (s != yajl_gen_status_ok)
             goto out;
     }
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index b9c9e2c..ad5849d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -919,17 +919,17 @@ static void parse_config_data(const char *config_source,
     }
 
     if (!xlu_cfg_get_list (config, "vif", &nics, 0, 0)) {
-        d_config->num_vifs = 0;
-        d_config->vifs = NULL;
-        while ((buf = xlu_cfg_get_listitem (nics, d_config->num_vifs)) != NULL) {
+        d_config->num_nics = 0;
+        d_config->nics = NULL;
+        while ((buf = xlu_cfg_get_listitem (nics, d_config->num_nics)) != NULL) {
             libxl_device_nic *nic;
             char *buf2 = strdup(buf);
             char *p, *p2;
 
-            d_config->vifs = (libxl_device_nic *) realloc(d_config->vifs, sizeof (libxl_device_nic) * (d_config->num_vifs+1));
-            nic = d_config->vifs + d_config->num_vifs;
+            d_config->nics = (libxl_device_nic *) realloc(d_config->nics, sizeof (libxl_device_nic) * (d_config->num_nics+1));
+            nic = d_config->nics + d_config->num_nics;
             libxl_device_nic_init(nic);
-            nic->devid = d_config->num_vifs;
+            nic->devid = d_config->num_nics;
 
             if (default_vifscript) {
                 free(nic->script);
@@ -1002,7 +1002,7 @@ static void parse_config_data(const char *config_source,
             } while ((p = strtok(NULL, ",")) != NULL);
 skip:
             free(buf2);
-            d_config->num_vifs++;
+            d_config->num_nics++;
         }
     }
 
diff --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
index 6e0a389..91f8f75 100644
--- a/tools/libxl/xl_sxp.c
+++ b/tools/libxl/xl_sxp.c
@@ -175,20 +175,20 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
         printf("\t)\n");
     }
 
-    for (i = 0; i < d_config->num_vifs; i++) {
+    for (i = 0; i < d_config->num_nics; i++) {
         printf("\t(device\n");
         printf("\t\t(vif\n");
-        if (d_config->vifs[i].ifname)
-            printf("\t\t\t(vifname %s)\n", d_config->vifs[i].ifname);
-        printf("\t\t\t(backend_domid %d)\n", d_config->vifs[i].backend_domid);
+        if (d_config->nics[i].ifname)
+            printf("\t\t\t(vifname %s)\n", d_config->nics[i].ifname);
+        printf("\t\t\t(backend_domid %d)\n", d_config->nics[i].backend_domid);
         printf("\t\t\t(frontend_domid %d)\n", domid);
-        printf("\t\t\t(devid %d)\n", d_config->vifs[i].devid);
-        printf("\t\t\t(mtu %d)\n", d_config->vifs[i].mtu);
-        printf("\t\t\t(model %s)\n", d_config->vifs[i].model);
+        printf("\t\t\t(devid %d)\n", d_config->nics[i].devid);
+        printf("\t\t\t(mtu %d)\n", d_config->nics[i].mtu);
+        printf("\t\t\t(model %s)\n", d_config->nics[i].model);
         printf("\t\t\t(mac %02x%02x%02x%02x%02x%02x)\n",
-               d_config->vifs[i].mac[0], d_config->vifs[i].mac[1],
-               d_config->vifs[i].mac[2], d_config->vifs[i].mac[3],
-               d_config->vifs[i].mac[4], d_config->vifs[i].mac[5]);
+               d_config->nics[i].mac[0], d_config->nics[i].mac[1],
+               d_config->nics[i].mac[2], d_config->nics[i].mac[3],
+               d_config->nics[i].mac[4], d_config->nics[i].mac[5]);
         printf("\t\t)\n");
         printf("\t)\n");
     }
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 05/17] libxl: convert libxl_device_disk_add to an async op
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (3 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 04/17] libxl: rename vifs to nics Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 06/17] libxl: convert libxl_device_nic_add to an async operation Roger Pau Monne
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 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 later 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.

Changes since v9:

 * Removed a wrong comment hunk.

 * Fixed Ocaml bindings.

Changes since v8:

 * Added a macro to define libxl_device_{type}_add functions, since
   they are all equal (just like for device removal/destroy).

Changes since v6:

 * Fixed comments.

 * Rename libxl__initiate_device_connection to
   libxl__wait_device_connection.

 * Cancel backend devstate watch if local attach of disk fails.

Changes since v5:

 * Change the disk addition to use the new ao_devices structure.

 * Change name and comment of _initiate_device_add to
   _initiate_device_connect.

Changes since v4:

 * Added more comments about libxl__device_disk_add and
   libxl__add_disks.

 * Removed stray hunk in Makefile.

 * Fixed _prepare call libxl__add_disks (separate into a different
   loop).

 * Moved some code to check if disks have finished to a macro that
   generates a custom function, which will also be used for vif
   interfaces.

Changes since v2:

 * Remove state read from libxl__initiate_device_add

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c                  |  145 +++++++++++++++++++++++++---------
 tools/libxl/libxl.h                  |    4 +-
 tools/libxl/libxl_create.c           |   42 ++++++++---
 tools/libxl/libxl_device.c           |   77 ++++++++++++++++++
 tools/libxl/libxl_dm.c               |   73 ++++++++++++-----
 tools/libxl/libxl_internal.h         |   39 +++++++++-
 tools/libxl/xl_cmdimpl.c             |    2 +-
 tools/ocaml/libs/xl/xenlight_stubs.c |    2 +-
 8 files changed, 310 insertions(+), 74 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e8ef581..bc0ad77 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1768,16 +1768,18 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
  * The passed get_vdev function is also in charge of printing
  * the corresponding error message when appropiate.
  */
-static int device_disk_add(libxl__gc *gc, uint32_t domid,
+static void device_disk_add(libxl__egc *egc, uint32_t domid,
                            libxl_device_disk *disk,
+                           libxl__ao_device *aodev,
                            void *get_vdev(libxl__gc *, void *,
                                           xs_transaction_t),
                            void *get_vdev_user)
 {
+    STATE_AO_GC(aodev->ao);
     flexarray_t *front = NULL;
     flexarray_t *back = NULL;
     char *dev;
-    libxl__device device;
+    libxl__device *device;
     int major, minor, rc;
     libxl_ctx *ctx = gc->owner;
     xs_transaction_t t = XBT_NULL;
@@ -1820,7 +1822,8 @@ static int 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) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
                    " virtual disk identifier %s", disk->vdev);
@@ -1838,7 +1841,7 @@ static int 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);
@@ -1859,7 +1862,7 @@ static int device_disk_add(libxl__gc *gc, uint32_t domid,
                 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);
+                assert(device->backend_kind == LIBXL__DEVICE_KIND_QDISK);
                 break;
             default:
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
@@ -1891,11 +1894,11 @@ static int device_disk_add(libxl__gc *gc, uint32_t 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, libxl__sprintf(gc, "%d", device->devid));
         flexarray_append(front, "device-type");
         flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
 
-        libxl__device_generic_add(gc, t, &device,
+        libxl__device_generic_add(gc, t, device,
                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
@@ -1904,6 +1907,10 @@ static int device_disk_add(libxl__gc *gc, uint32_t domid,
         if (rc < 0) goto out_free;
     }
 
+    aodev->dev = device;
+    aodev->action = DEVICE_CONNECT;
+    libxl__wait_device_connection(egc, aodev);
+
     rc = 0;
 
 out_free:
@@ -1911,21 +1918,15 @@ out_free:
     flexarray_free(front);
 out:
     libxl__xs_transaction_abort(gc, &t);
-    return rc;
-}
-
-int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
-                           libxl_device_disk *disk)
-{
-    return device_disk_add(gc, domid, disk, NULL, NULL);
+    aodev->rc = rc;
+    if (rc) aodev->callback(egc, aodev);
+    return;
 }
 
-int libxl_device_disk_add(libxl_ctx *ctx, 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 *aodev)
 {
-    GC_INIT(ctx);
-    int rc = libxl__device_disk_add(gc, domid, disk);
-    GC_FREE;
-    return rc;
+    device_disk_add(egc, domid, disk, aodev, NULL, NULL);
 }
 
 static void libxl__device_disk_from_xs_be(libxl__gc *gc,
@@ -2128,11 +2129,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++)
@@ -2173,14 +2176,17 @@ static char * libxl__alloc_vdev(libxl__gc *gc, const char *blkdev_start,
     return NULL;
 }
 
+/* Callbacks */
+
+static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev);
+
 void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
                                      libxl__disk_local_state *dls)
 {
     STATE_AO_GC(dls->ao);
     libxl_ctx *ctx = CTX;
-    char *dev = NULL, *be_path = NULL;
+    char *dev = NULL;
     int rc;
-    libxl__device device;
     const libxl_device_disk *in_disk = dls->in_disk;
     libxl_device_disk *disk = &dls->disk;
     const char *blkdev_start = dls->blkdev_start;
@@ -2227,14 +2233,12 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
             break;
         case LIBXL_DISK_BACKEND_QDISK:
             if (disk->format != LIBXL_DISK_FORMAT_RAW) {
-                if (device_disk_add(gc, LIBXL_TOOLSTACK_DOMID, disk,
-                                    (void *) libxl__alloc_vdev,
-                                    (void *) blkdev_start)) {
-                    LOG(ERROR, "libxl_device_disk_add failed");
-                    rc = ERROR_FAIL;
-                    goto out;
-                }
-                dev = GCSPRINTF("/dev/%s", disk->vdev);
+                libxl__prepare_ao_device(ao, &dls->aodev);
+                dls->aodev.callback = local_device_attach_cb;
+                device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk,
+                                &dls->aodev, (void *) libxl__alloc_vdev,
+                                (void *) blkdev_start);
+                return;
             } else {
                 dev = disk->pdev_path;
             }
@@ -2247,15 +2251,48 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
             goto out;
     }
 
-    if (disk->vdev != NULL) {
-        rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, disk, &device);
-        if (rc < 0)
-            goto out;
-        be_path = libxl__device_backend_path(gc, &device);
-        rc = libxl__wait_for_backend(gc, be_path, "4");
-        if (rc < 0)
-            goto out;
+    if (dev != NULL)
+        dls->diskpath = strdup(dev);
+
+    dls->callback(egc, dls, 0);
+    return;
+
+ out:
+    assert(rc);
+    dls->rc = rc;
+    libxl__device_disk_local_initiate_detach(egc, dls);
+    dls->callback(egc, dls, rc);
+}
+
+static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__disk_local_state *dls = CONTAINER_OF(aodev, *dls, aodev);
+    char *dev = NULL, *be_path = NULL;
+    int rc;
+    libxl__device device;
+    libxl_device_disk *disk = &dls->disk;
+
+    rc = aodev->rc;
+    if (rc) {
+        LOGE(ERROR, "unable to %s %s with id %u",
+                    aodev->action == DEVICE_CONNECT ? "add" : "remove",
+                    libxl__device_kind_to_string(aodev->dev->kind),
+                    aodev->dev->devid);
+        goto out;
     }
+
+    dev = GCSPRINTF("/dev/%s", disk->vdev);
+    LOG(DEBUG, "locally attaching qdisk %s", dev);
+
+    rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, disk, &device);
+    if (rc < 0)
+        goto out;
+    be_path = libxl__device_backend_path(gc, &device);
+    rc = libxl__wait_for_backend(gc, be_path, "4");
+    if (rc < 0)
+        goto out;
+
     if (dev != NULL)
         dls->diskpath = libxl__strdup(gc, dev);
 
@@ -2979,6 +3016,36 @@ DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
 
 /******************************************************************************/
 
+/* Macro for defining device addition functions in a compact way */
+/* The following functions are defined:
+ * libxl_device_disk_add
+ */
+
+#define DEFINE_DEVICE_ADD(type)                                         \
+    int libxl_device_##type##_add(libxl_ctx *ctx,                       \
+        uint32_t domid, libxl_device_##type *type,                      \
+        const libxl_asyncop_how *ao_how)                                \
+    {                                                                   \
+        AO_CREATE(ctx, domid, ao_how);                                  \
+        libxl__ao_device *aodev;                                        \
+                                                                        \
+        GCNEW(aodev);                                                   \
+        libxl__prepare_ao_device(ao, aodev);                            \
+        aodev->callback = device_addrm_aocomplete;                      \
+        libxl__device_##type##_add(egc, domid, type, aodev);            \
+                                                                        \
+        return AO_INPROGRESS;                                           \
+    }
+
+/* Define alladd functions and undef the macro */
+
+/* disk */
+DEFINE_DEVICE_ADD(disk)
+
+#undef DEFINE_DEVICE_ADD
+
+/******************************************************************************/
+
 int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
 {
     GC_INIT(ctx);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index c730ac5..f3161d5 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -675,7 +675,9 @@ void libxl_vcpuinfo_list_free(libxl_vcpuinfo *, int nr_vcpus);
  */
 
 /* 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 0c81a60..5c7a242 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -558,6 +558,10 @@ static void domcreate_bootloader_console_available(libxl__egc *egc,
 static void domcreate_bootloader_done(libxl__egc *egc,
                                       libxl__bootloader_state *bl,
                                       int rc);
+
+static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
+                                int ret);
+
 static void domcreate_console_available(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
 
@@ -848,12 +852,10 @@ static void domcreate_rebuild_done(libxl__egc *egc,
                                    int ret)
 {
     STATE_AO_GC(dcs->ao);
-    int i;
 
     /* 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;
 
     if (ret) {
@@ -864,14 +866,34 @@ static void domcreate_rebuild_done(libxl__egc *egc,
 
     store_libxl_entry(gc, domid, &d_config->b_info);
 
-    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;
-        }
+    dcs->aodevs.size = d_config->num_disks;
+    dcs->aodevs.callback = domcreate_launch_dm;
+    libxl__prepare_ao_devices(ao, &dcs->aodevs);
+    libxl__add_disks(egc, ao, domid, 0, d_config, &dcs->aodevs);
+
+    return;
+
+ error_out:
+    assert(ret);
+    domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
+                                int ret)
+{
+    libxl__domain_create_state *dcs = CONTAINER_OF(aodevs, *dcs, aodevs);
+    STATE_AO_GC(dcs->ao);
+    int i;
+
+    /* 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;
+
+    if (ret) {
+        LOG(ERROR, "unable to add disk devices");
+        goto error_out;
     }
     for (i = 0; i < d_config->num_nics; i++) {
         ret = libxl_device_nic_add(ctx, domid, &d_config->nics[i]);
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 2120cfb..73b8f53 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -447,6 +447,37 @@ void libxl__ao_devices_callback(libxl__egc *egc, libxl__ao_device *aodev)
     return;
 }
 
+/******************************************************************************/
+
+/* Macro for defining the functions that will add a bunch of disks when
+ * inside an async op.
+ * This macro is added to prevent repetition of code.
+ *
+ * The following functions are defined:
+ * libxl__add_disks
+ */
+
+#define DEFINE_DEVICES_ADD(type)                                               \
+    void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid,  \
+                              int start, libxl_domain_config *d_config,        \
+                              libxl__ao_devices *aodevs)                       \
+    {                                                                          \
+        AO_GC;                                                                 \
+        int i;                                                                 \
+        int end = start + d_config->num_##type##s;                             \
+        for (i = start; i < end; i++) {                                        \
+            aodevs->array[i].callback = libxl__ao_devices_callback;            \
+            libxl__device_##type##_add(egc, domid, &d_config->type##s[i-start],\
+                                       &aodevs->array[i]);                     \
+        }                                                                      \
+    }
+
+DEFINE_DEVICES_ADD(disk)
+
+#undef DEFINE_DEVICES_ADD
+
+/******************************************************************************/
+
 int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -583,6 +614,52 @@ static void device_backend_cleanup(libxl__gc *gc,
 
 static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev);
 
+void libxl__wait_device_connection(libxl__egc *egc, libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    char *be_path = libxl__device_backend_path(gc, aodev->dev);
+    char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+    libxl_dominfo info;
+    uint32_t domid = aodev->dev->domid;
+    int rc = 0;
+
+    libxl_dominfo_init(&info);
+    rc = libxl_domain_info(CTX, &info, domid);
+    if (rc) {
+        LOG(ERROR, "unable to get info for domain %d", domid);
+        goto out;
+    }
+    if (QEMU_BACKEND(aodev->dev)) {
+        /*
+         * If Qemu is not running, there's no point in waiting for
+         * it to change the state of the device.
+         *
+         * If Qemu is running, it will set the state of the device to
+         * 4 directly, without waiting in state 2 for any hotplug execution.
+         */
+        device_hotplug_done(egc, aodev);
+        return;
+    }
+
+    rc = libxl__ev_devstate_wait(gc, &aodev->backend_ds,
+                                 device_backend_callback,
+                                 state_path, XenbusStateInitWait,
+                                 LIBXL_INIT_TIMEOUT * 1000);
+    if (rc) {
+        LOG(ERROR, "unable to initialize device %s", be_path);
+        goto out;
+    }
+
+    libxl_dominfo_dispose(&info);
+    return;
+
+out:
+    aodev->rc = rc;
+    libxl_dominfo_dispose(&info);
+    device_hotplug_done(egc, aodev);
+    return;
+}
+
 void libxl__initiate_device_remove(libxl__egc *egc,
                                    libxl__ao_device *aodev)
 {
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0e2ec18..07ba94c 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -712,6 +712,9 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
                                 libxl__dm_spawn_state *stubdom_dmss,
                                 int rc);
 
+static void spawn_stub_launch_dm(libxl__egc *egc,
+                                 libxl__ao_devices *aodevs, int ret);
+
 static void spaw_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
                                            libxl__destroy_domid_state *dis,
                                            int rc);
@@ -725,10 +728,9 @@ 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;
-    libxl_device_vfb vfb;
-    libxl_device_vkb vkb;
+    int ret;
+    libxl_device_vfb *vfb;
+    libxl_device_vkb *vkb;
     char **args;
     struct xs_permissions perm[2];
     xs_transaction_t t;
@@ -783,10 +785,12 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info);
     if (ret) goto out;
 
-    libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, &vfb, &vkb);
-    dm_config->vfbs = &vfb;
+    GCNEW(vfb);
+    GCNEW(vkb);
+    libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb);
+    dm_config->vfbs = vfb;
     dm_config->num_vfbs = 1;
-    dm_config->vkbs = &vkb;
+    dm_config->vkbs = vkb;
     dm_config->num_vkbs = 1;
 
     stubdom_state->pv_kernel.path
@@ -844,22 +848,54 @@ retry_transaction:
         if (errno == EAGAIN)
             goto retry_transaction;
 
-    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;
-    }
+    sdss->aodevs.size = dm_config->num_disks;
+    sdss->aodevs.callback = spawn_stub_launch_dm;
+    libxl__prepare_ao_devices(ao, &sdss->aodevs);
+    libxl__add_disks(egc, ao, dm_domid, 0, dm_config, &sdss->aodevs);
+
+    free(args);
+    return;
+
+out_free:
+    free(args);
+out:
+    assert(ret);
+    spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
+}
+
+static void spawn_stub_launch_dm(libxl__egc *egc,
+                                 libxl__ao_devices *aodevs, int ret)
+{
+    libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(aodevs, *sdss, aodevs);
+    STATE_AO_GC(sdss->dm.spawn.ao);
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int i, num_console = STUBDOM_SPECIAL_CONSOLES;
+    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;
+
+    if (ret) {
+        LOG(ERROR, "error connecting disk devices");
+        goto out;
+     }
+
     for (i = 0; i < dm_config->num_nics; i++) {
         ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->nics[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++;
@@ -867,7 +903,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++) {
@@ -903,7 +939,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.spawn.ao = ao;
@@ -914,11 +950,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 cb2975a..d570fdb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -72,6 +72,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_QEMU_BODGE_TIMEOUT 2
@@ -1297,8 +1298,6 @@ _hidden void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path);
 _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 char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);
 
@@ -1883,6 +1882,27 @@ struct libxl__ao_devices {
  *                   DONE.
  */
 
+/* AO operation to connect a disk device, called by
+ * libxl_device_disk_add and libxl__add_disks. This function calls
+ * libxl__wait_device_connection to wait for the device to
+ * finish the connection (might involve executing hotplug scripts).
+ *
+ * Once finished, aodev->callback will be executed.
+ */
+_hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
+                                    libxl_device_disk *disk,
+                                    libxl__ao_device *aodev);
+
+/* Waits for the passed device to reach state XenbusStateInitWait.
+ * This is not really useful by itself, but is important when executing
+ * hotplug scripts, since we need to be sure the device is in the correct
+ * state before executing them.
+ *
+ * Once finished, aodev->callback will be executed.
+ */
+_hidden void libxl__wait_device_connection(libxl__egc*,
+                                           libxl__ao_device *aodev);
+
 /* 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
@@ -2253,6 +2273,19 @@ _hidden void libxl__destroy_domid(libxl__egc *egc,
 _hidden void libxl__devices_destroy(libxl__egc *egc,
                                     libxl__devices_remove_state *drs);
 
+/* Helper function to add a bunch of disks. This should be used when
+ * the caller is inside an async op. "devices" will NOT be prepared by this
+ * function, so the caller must make sure to call _prepare before calling this
+ * function. The start parameter contains the position inside the aodevs array
+ * that should be used to store the state of this devices.
+ *
+ * The "callback" will be called for each device, and the user is responsible
+ * for calling libxl__ao_device_check_last on the callback.
+ */
+_hidden void libxl__add_disks(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+                              int start, libxl_domain_config *d_config,
+                              libxl__ao_devices *aodevs);
+
 /*----- device model creation -----*/
 
 /* First layer; wraps libxl__spawn_spawn. */
@@ -2287,6 +2320,7 @@ typedef struct {
     libxl__domain_build_state dm_state;
     libxl__dm_spawn_state pvqemu;
     libxl__destroy_domid_state dis;
+    libxl__ao_devices aodevs;
 } libxl__stub_dm_spawn_state;
 
 _hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
@@ -2318,6 +2352,7 @@ struct libxl__domain_create_state {
     libxl__save_helper_state shs;
     /* necessary if the domain creation failed and we have to destroy it */
     libxl__domain_destroy_state dds;
+    libxl__ao_devices aodevs;
 };
 
 /*----- Domain suspend (save) functions -----*/
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index ad5849d..a1c3f50 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5354,7 +5354,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;
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index 53751b1..a158351 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -247,7 +247,7 @@ value stub_xl_device_disk_add(value info, value domid)
 	device_disk_val(&gc, &lg, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_disk_add(ctx, Int_val(domid), &c_info);
+	ret = libxl_device_disk_add(ctx, Int_val(domid), &c_info, 0);
 	if (ret != 0)
 		failwith_xl("disk_add", &lg);
 	FREE_CTX();
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 06/17] libxl: convert libxl_device_nic_add to an async operation
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (4 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 05/17] libxl: convert libxl_device_disk_add to an async op Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 07/17] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 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.

Changes since v9:

 * Fixed Ocaml bindings.

Changes since v5:

 * Updated to use the new ao_devices struct.

 * Renamed ao_device in nic_add to aodev.

Changes since v4:

 * Used the macro defined in previous patch to define the generic
   callback to wait for nics to be connected.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c                  |   29 ++++++++++++++-----
 tools/libxl/libxl.h                  |    3 +-
 tools/libxl/libxl_create.c           |   51 ++++++++++++++++++++++++++++-----
 tools/libxl/libxl_device.c           |    2 +
 tools/libxl/libxl_dm.c               |   38 +++++++++++++++++++++++--
 tools/libxl/libxl_internal.h         |    9 ++++++
 tools/libxl/xl_cmdimpl.c             |    2 +-
 tools/ocaml/libs/xl/xenlight_stubs.c |    2 +-
 8 files changed, 114 insertions(+), 22 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index bc0ad77..2b44f57 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2429,12 +2429,13 @@ static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
-int libxl_device_nic_add(libxl_ctx *ctx, 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 *aodev)
 {
-    GC_INIT(ctx);
+    STATE_AO_GC(aodev->ao);
     flexarray_t *front;
     flexarray_t *back;
-    libxl__device device;
+    libxl__device *device;
     char *dompath, **l;
     unsigned int nb, rc;
 
@@ -2465,7 +2466,8 @@ int libxl_device_nic_add(libxl_ctx *ctx, 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");
@@ -2506,6 +2508,9 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
     flexarray_append(back, libxl__strdup(gc, nic->bridge));
     flexarray_append(back, "handle");
     flexarray_append(back, libxl__sprintf(gc, "%d", nic->devid));
+    flexarray_append(back, "type");
+    flexarray_append(back, libxl__strdup(gc,
+                                     libxl_nic_type_to_string(nic->nictype)));
 
     flexarray_append(front, "backend-id");
     flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domid));
@@ -2516,18 +2521,22 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
     flexarray_append(front, "mac");
     flexarray_append(front, libxl__sprintf(gc,
                                     LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
-    libxl__device_generic_add(gc, XBT_NULL, &device,
+    libxl__device_generic_add(gc, XBT_NULL, device,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
-    /* FIXME: wait for plug */
+    aodev->dev = device;
+    aodev->action = DEVICE_CONNECT;
+    libxl__wait_device_connection(egc, aodev);
+
     rc = 0;
 out_free:
     flexarray_free(back);
     flexarray_free(front);
 out:
-    GC_FREE;
-    return rc;
+    aodev->rc = rc;
+    if (rc) aodev->callback(egc, aodev);
+    return;
 }
 
 static void libxl__device_nic_from_xs_be(libxl__gc *gc,
@@ -3019,6 +3028,7 @@ DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
 /* Macro for defining device addition functions in a compact way */
 /* The following functions are defined:
  * libxl_device_disk_add
+ * libxl_device_nic_add
  */
 
 #define DEFINE_DEVICE_ADD(type)                                         \
@@ -3042,6 +3052,9 @@ DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
 /* disk */
 DEFINE_DEVICE_ADD(disk)
 
+/* nic */
+DEFINE_DEVICE_ADD(nic)
+
 #undef DEFINE_DEVICE_ADD
 
 /******************************************************************************/
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index f3161d5..d548090 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -696,7 +696,8 @@ int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
 int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, 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 5c7a242..2180e81 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -562,6 +562,9 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
                                 int ret);
 
+static void domcreate_attach_pci(libxl__egc *egc, libxl__ao_devices *aodevs,
+                                 int ret);
+
 static void domcreate_console_available(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
 
@@ -896,13 +899,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
         goto error_out;
     }
     for (i = 0; i < d_config->num_nics; i++) {
-        ret = libxl_device_nic_add(ctx, domid, &d_config->nics[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->nics[i]);
     }
     switch (d_config->c_info.type) {
     case LIBXL_DOMAIN_TYPE_HVM:
@@ -975,7 +976,6 @@ static void domcreate_devmodel_started(libxl__egc *egc,
 {
     libxl__domain_create_state *dcs = CONTAINER_OF(dmss, *dcs, dmss.dm);
     STATE_AO_GC(dmss->spawn.ao);
-    int i;
     libxl_ctx *ctx = CTX;
     int domid = dcs->guest_domid;
 
@@ -995,6 +995,41 @@ static void domcreate_devmodel_started(libxl__egc *egc,
         }
     }
 
+    /* Plug nic interfaces */
+    if (d_config->num_nics > 0) {
+        /* Attach nics */
+        dcs->aodevs.size = d_config->num_nics;
+        dcs->aodevs.callback = domcreate_attach_pci;
+        libxl__prepare_ao_devices(ao, &dcs->aodevs);
+        libxl__add_nics(egc, ao, domid, 0, d_config, &dcs->aodevs);
+        return;
+    }
+
+    domcreate_attach_pci(egc, &dcs->aodevs, 0);
+    return;
+
+error_out:
+    assert(ret);
+    domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_attach_pci(libxl__egc *egc, libxl__ao_devices *aodevs,
+                                 int ret)
+{
+    libxl__domain_create_state *dcs = CONTAINER_OF(aodevs, *dcs, aodevs);
+    STATE_AO_GC(dcs->ao);
+    int i;
+    libxl_ctx *ctx = CTX;
+    int domid = dcs->guest_domid;
+
+    /* convenience aliases */
+    libxl_domain_config *const d_config = dcs->guest_config;
+
+    if (ret) {
+        LOG(ERROR, "unable to add nic devices");
+        goto error_out;
+    }
+
     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 73b8f53..8322b86 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -455,6 +455,7 @@ void libxl__ao_devices_callback(libxl__egc *egc, libxl__ao_device *aodev)
  *
  * The following functions are defined:
  * libxl__add_disks
+ * libxl__add_nics
  */
 
 #define DEFINE_DEVICES_ADD(type)                                               \
@@ -473,6 +474,7 @@ void libxl__ao_devices_callback(libxl__egc *egc, libxl__ao_device *aodev)
     }
 
 DEFINE_DEVICES_ADD(disk)
+DEFINE_DEVICES_ADD(nic)
 
 #undef DEFINE_DEVICES_ADD
 
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 07ba94c..eed9062 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -715,6 +715,10 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
 static void spawn_stub_launch_dm(libxl__egc *egc,
                                  libxl__ao_devices *aodevs, int ret);
 
+static void stubdom_pvqemu_cb(libxl__egc *egc,
+                              libxl__ao_devices *aodevs,
+                              int rc);
+
 static void spaw_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
                                            libxl__destroy_domid_state *dis,
                                            int rc);
@@ -886,9 +890,11 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
      }
 
     for (i = 0; i < dm_config->num_nics; i++) {
-        ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->nics[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->nics[i]);
     }
     ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
     if (ret)
@@ -965,9 +971,35 @@ 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;
 
     if (rc) goto out;
 
+    if (d_config->num_nics > 0) {
+        sdss->aodevs.size = d_config->num_nics;
+        sdss->aodevs.callback = stubdom_pvqemu_cb;
+        libxl__prepare_ao_devices(ao, &sdss->aodevs);
+        libxl__add_nics(egc, ao, dm_domid, 0, d_config, &sdss->aodevs);
+        return;
+    }
+
+out:
+    stubdom_pvqemu_cb(egc, &sdss->aodevs, rc);
+}
+
+static void stubdom_pvqemu_cb(libxl__egc *egc,
+                              libxl__ao_devices *aodevs,
+                              int rc)
+{
+    libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(aodevs, *sdss, aodevs);
+    STATE_AO_GC(sdss->dm.spawn.ao);
+    uint32_t dm_domid = sdss->pvqemu.guest_domid;
+
+    if (rc) {
+        LOGE(ERROR, "error connecting nics devices");
+        goto out;
+    }
+
     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 d570fdb..8c3c721 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1893,6 +1893,11 @@ _hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
                                     libxl_device_disk *disk,
                                     libxl__ao_device *aodev);
 
+/* AO operation to connect a nic device */
+_hidden void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+                                   libxl_device_nic *nic,
+                                   libxl__ao_device *aodev);
+
 /* Waits for the passed device to reach state XenbusStateInitWait.
  * This is not really useful by itself, but is important when executing
  * hotplug scripts, since we need to be sure the device is in the correct
@@ -2286,6 +2291,10 @@ _hidden void libxl__add_disks(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
                               int start, libxl_domain_config *d_config,
                               libxl__ao_devices *aodevs);
 
+_hidden void libxl__add_nics(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+                             int start, libxl_domain_config *d_config,
+                             libxl__ao_devices *aodevs);
+
 /*----- device model creation -----*/
 
 /* First layer; wraps libxl__spawn_spawn. */
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a1c3f50..61f9926 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5243,7 +5243,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;
     }
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index a158351..0e9c65e 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -281,7 +281,7 @@ value stub_xl_device_nic_add(value info, value domid)
 	device_nic_val(&gc, &lg, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_nic_add(ctx, Int_val(domid), &c_info);
+	ret = libxl_device_nic_add(ctx, Int_val(domid), &c_info, 0);
 	if (ret != 0)
 		failwith_xl("nic_add", &lg);
 	FREE_CTX();
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 07/17] libxl: add option to choose who executes hotplug scripts
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (5 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 06/17] libxl: convert libxl_device_nic_add to an async operation Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 08/17] libxl: rename _IOEMU nic type to VIF_IOEMU Roger Pau Monne
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 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.

Changes since v8:

 * Free list of returned VMs

 * Use libxl__xs_{write,rm}_checked.

 * Change initialization of dm_config->c_info.run_hotplug_scripts.

Changes since v2:

 * Change atoi(...) to !!atoi(...) to prevent returning negative
   values from xenstore (which will be handled as errors).

 * Check for errors on the return value of libxl__hotplug_settings.

Changes since v1:

 * Used an auxiliary function to check for the current setting.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: 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   |   40 +++++++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl_dm.c       |    3 +++
 tools/libxl/libxl_internal.c |   19 +++++++++++++++++++
 tools/libxl/libxl_internal.h |    3 +++
 tools/libxl/libxl_types.idl  |    1 +
 tools/libxl/xl.c             |    4 ++++
 tools/libxl/xl.h             |    1 +
 tools/libxl/xl_cmdimpl.c     |    1 +
 10 files changed, 84 insertions(+), 1 deletions(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 149430c..23932be 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 ebf057c..28ab796 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -15,3 +15,8 @@
 
 # first block device to be used for temporary VM disk mounts
 #blkdev_start="xvda"
+
+# 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 2180e81..9330012 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -70,6 +70,8 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
         libxl_defbool_setdefault(&c_info->oos, true);
     }
 
+    libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true);
+
     return 0;
 }
 
@@ -382,7 +384,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];
@@ -390,6 +392,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info,
     struct xs_permissions noperm[1];
     xs_transaction_t t = 0;
     xen_domain_handle_t handle;
+    libxl_vminfo *vm_list;
 
 
     assert(!libxl_domid_valid_guest(*domid));
@@ -503,6 +506,41 @@ retry_transaction:
             libxl__sprintf(gc, "%s/hvmloader/generation-id-address", dom_path),
                         rwperm, ARRAY_SIZE(rwperm));
 
+                    vm_list = libxl_list_vm(ctx, &nb_vm);
+    if (!vm_list) {
+        LOG(ERROR, "cannot get number of running guests");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    libxl_vminfo_list_free(vm_list, nb_vm);
+    int hotplug_setting = libxl__hotplug_settings(gc, t);
+    if (hotplug_setting < 0) {
+        LOG(ERROR, "unable to get current hotplug scripts execution setting");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (libxl_defbool_val(info->run_hotplug_scripts) != hotplug_setting &&
+        (nb_vm - 1)) {
+        LOG(ERROR, "cannot change hotplug execution option once set, "
+                    "please shutdown all guests before changing it");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    if (libxl_defbool_val(info->run_hotplug_scripts)) {
+        rc = libxl__xs_write_checked(gc, t, DISABLE_UDEV_PATH, "1");
+        if (rc) {
+            LOGE(ERROR, "unable to write %s = 1", DISABLE_UDEV_PATH);
+            goto out;
+        }
+    } else {
+        rc = libxl__xs_rm_checked(gc, t, DISABLE_UDEV_PATH);
+        if (rc) {
+            LOGE(ERROR, "unable to delete %s", DISABLE_UDEV_PATH);
+            goto out;
+        }
+    }
+
     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_dm.c b/tools/libxl/libxl_dm.c
index eed9062..a6f2181 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -784,6 +784,9 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 
     libxl__dm_vifs_from_hvm_guest_config(gc, guest_config, dm_config);
 
+    dm_config->c_info.run_hotplug_scripts =
+        guest_config->c_info.run_hotplug_scripts;
+
     ret = libxl__domain_create_info_setdefault(gc, &dm_config->c_info);
     if (ret) goto out;
     ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info);
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index fbff7d0..4775bb6 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -353,6 +353,25 @@ libxl_device_model_version libxl__device_model_version_running(libxl__gc *gc,
     return value;
 }
 
+int libxl__hotplug_settings(libxl__gc *gc, xs_transaction_t t)
+{
+    int rc = 0;
+    char *val;
+
+    val = libxl__xs_read(gc, t, DISABLE_UDEV_PATH);
+    if (!val && errno != ENOENT) {
+        LOGE(ERROR, "cannot read %s from xenstore", DISABLE_UDEV_PATH);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (!val) val = "0";
+
+    rc = !!atoi(val);
+
+out:
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8c3c721..2add151 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -91,6 +91,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]))
 
@@ -1470,6 +1471,8 @@ _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s);
 _hidden libxl_device_model_version
 libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
 
+/* Check how executes hotplug script currently */
+int libxl__hotplug_settings(libxl__gc *gc, xs_transaction_t t);
 
 /*
  * Calling context and GC for event-generating functions:
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index c3fbe77..9c83436 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -231,6 +231,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 d6f2b3e..f31e836 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -39,6 +39,7 @@ int dryrun_only;
 int force_execution;
 int autoballoon = 1;
 char *blkdev_start;
+int run_hotplug_scripts = 1;
 char *lockfile;
 char *default_vifscript = NULL;
 char *default_bridge = NULL;
@@ -70,6 +71,9 @@ static void parse_global_config(const char *configfile,
     if (!xlu_cfg_get_long (config, "autoballoon", &l, 0))
         autoballoon = l;
 
+    if (!xlu_cfg_get_long (config, "run_hotplug_scripts", &l, 0))
+        run_hotplug_scripts = l;
+
     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 b0ba357..0b2f848 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -140,6 +140,7 @@ int xl_child_pid(xlchildnum); /* returns 0 if child struct is not in use */
 
 /* global options */
 extern int autoballoon;
+extern int 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 61f9926..1f36e80 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -595,6 +595,7 @@ static void parse_config_data(const char *config_source,
         }
     }
 
+    libxl_defbool_set(&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] 30+ messages in thread

* [PATCH v12 08/17] libxl: rename _IOEMU nic type to VIF_IOEMU
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (6 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 07/17] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 09/17] libxl: set nic type of stub to PV instead of copying from the parent Roger Pau Monne
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

This change will avoid the confusion caused by the fact that IOEMU
means both PV and TAP network interfaces.

Changes since v10:

 * Fixed a comment in libxl__device_nic_from_xs_be.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c         |    6 +++---
 tools/libxl/libxl_dm.c      |    8 ++++----
 tools/libxl/libxl_types.idl |    2 +-
 tools/libxl/xl_cmdimpl.c    |    4 ++--
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2b44f57..e7c9334 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2411,7 +2411,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_IOEMU;
     return 0;
 }
 
@@ -2574,7 +2574,7 @@ static void libxl__device_nic_from_xs_be(libxl__gc *gc,
     nic->script = xs_read(ctx->xsh, XBT_NULL,
                       libxl__sprintf(gc, "%s/script", be_path), &len);
 
-    /* XXX ioemu nics are not in xenstore at all? */
+    /* vif_ioemu nics use the same xenstore entries as vif interfaces */
     nic->nictype = LIBXL_NIC_TYPE_VIF;
     nic->model = NULL; /* XXX Only for TYPE_IOEMU */
     nic->ifname = NULL; /* XXX Only for TYPE_IOEMU */
@@ -2707,7 +2707,7 @@ const char *libxl__device_nic_devname(libxl__gc *gc,
     switch (type) {
     case LIBXL_NIC_TYPE_VIF:
         return GCSPRINTF("vif%u.%d", domid, devid);
-    case LIBXL_NIC_TYPE_IOEMU:
+    case LIBXL_NIC_TYPE_VIF_IOEMU:
         return GCSPRINTF("vif%u.%d" TAP_DEVICE_SUFFIX, domid, devid);
     default:
         abort();
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a6f2181..06cd2bf 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -215,12 +215,12 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
         free(s);
 
         for (i = 0; i < num_nics; i++) {
-            if (nics[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
+            if (nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
                 char *smac = libxl__sprintf(gc,
                                    LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nics[i].mac));
                 const char *ifname = libxl__device_nic_devname(gc,
                                                 domid, nics[i].devid,
-                                                LIBXL_NIC_TYPE_IOEMU);
+                                                LIBXL_NIC_TYPE_VIF_IOEMU);
                 flexarray_vappend(dm_args,
                                   "-net",
                                   GCSPRINTF(
@@ -469,12 +469,12 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                                                          b_info->max_vcpus));
         }
         for (i = 0; i < num_nics; i++) {
-            if (nics[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
+            if (nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
                 char *smac = libxl__sprintf(gc,
                                 LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nics[i].mac));
                 const char *ifname = libxl__device_nic_devname(gc,
                                                 guest_domid, nics[i].devid,
-                                                LIBXL_NIC_TYPE_IOEMU);
+                                                LIBXL_NIC_TYPE_VIF_IOEMU);
                 flexarray_append(dm_args, "-device");
                 flexarray_append(dm_args,
                    libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s",
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 9c83436..1a6c4a2 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -60,7 +60,7 @@ libxl_disk_backend = Enumeration("disk_backend", [
     ])
 
 libxl_nic_type = Enumeration("nic_type", [
-    (1, "IOEMU"),
+    (1, "VIF_IOEMU"),
     (2, "VIF"),
     ])
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 1f36e80..4028f54 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -978,7 +978,7 @@ static void parse_config_data(const char *config_source,
                     nic->bridge = strdup(p2 + 1);
                 } else if (!strcmp(p, "type")) {
                     if (!strcmp(p2 + 1, "ioemu"))
-                        nic->nictype = LIBXL_NIC_TYPE_IOEMU;
+                        nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
                     else
                         nic->nictype = LIBXL_NIC_TYPE_VIF;
                 } else if (!strcmp(p, "ip")) {
@@ -5195,7 +5195,7 @@ int main_networkattach(int argc, char **argv)
             if (!strcmp("vif", oparg)) {
                 nic.nictype = LIBXL_NIC_TYPE_VIF;
             } else if (!strcmp("ioemu", oparg)) {
-                nic.nictype = LIBXL_NIC_TYPE_IOEMU;
+                nic.nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
             } else {
                 fprintf(stderr, "Invalid parameter `type'.\n");
                 return 1;
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 09/17] libxl: set nic type of stub to PV instead of copying from the parent
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (7 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 08/17] libxl: rename _IOEMU nic type to VIF_IOEMU Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-26 10:38   ` Ian Jackson
  2012-07-25 11:19 ` [PATCH v12 10/17] libxl: set correct nic type depending on the guest Roger Pau Monne
                   ` (9 subsequent siblings)
  18 siblings, 1 reply; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell, Ian Jackson, Roger Pau Monne

Set the type of the nics used in sutbdoms to PV unconditianlly, or the
call to setdefaults later is going to fail.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl_dm.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 06cd2bf..dec7c19 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -626,6 +626,7 @@ static void libxl__dm_vifs_from_hvm_guest_config(libxl__gc *gc,
 
     for (i=0; i<nr; i++) {
         dm_config->nics[i] = guest_config->nics[i];
+        dm_config->nics[i].nictype = LIBXL_NIC_TYPE_VIF;
         if (dm_config->nics[i].ifname)
             dm_config->nics[i].ifname = GCSPRINTF("%s" TAP_DEVICE_SUFFIX,
                                                   dm_config->nics[i].ifname);
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 10/17] libxl: set correct nic type depending on the guest
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (8 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 09/17] libxl: set nic type of stub to PV instead of copying from the parent Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-26 11:08   ` Ian Jackson
  2012-07-25 11:19 ` [PATCH v12 11/17] libxl: use libxl__xs_path_cleanup on device_destroy Roger Pau Monne
                   ` (8 subsequent siblings)
  18 siblings, 1 reply; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

Fix the use of nic type, which results in the following for each type
of domain:

 * HVM: let the user choose, if none specified use VIF_IOEMU.
 * PV: use VIF is none provided, return error if VIF_IOEMU requested.

Changes since v11:

 * Added logging to setdefault.

 * Catch error on callers.

Changes since v10:

 * Return an error if trying to create a guest with an IOEMU nic.

Changes since v9:

 * Don't force nic type for HVM-stubdom guests.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c          |   24 ++++++++++++++++++++----
 tools/libxl/libxl_create.c   |    4 +++-
 tools/libxl/libxl_dm.c       |    4 +++-
 tools/libxl/libxl_internal.h |    3 ++-
 4 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e7c9334..b0e2ec1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2380,7 +2380,8 @@ out:
 
 /******************************************************************************/
 
-int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
+int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
+                                 uint32_t domid)
 {
     if (!nic->mtu)
         nic->mtu = 1492;
@@ -2410,8 +2411,23 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
     if ( !nic->script && asprintf(&nic->script, "%s/vif-bridge",
                                   libxl__xen_script_dir_path()) < 0 )
         return ERROR_FAIL;
-    if (!nic->nictype)
-        nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
+
+    switch (libxl__domain_type(gc, domid)) {
+    case LIBXL_DOMAIN_TYPE_HVM:
+        if (!nic->nictype)
+            nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
+        break;
+    case LIBXL_DOMAIN_TYPE_PV:
+        if (nic->nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
+            LOG(ERROR, "trying to create PV guest with an emulated interface");
+            return ERROR_INVAL;
+        }
+        nic->nictype = LIBXL_NIC_TYPE_VIF;
+        break;
+    default:
+        abort();
+    }
+
     return 0;
 }
 
@@ -2439,7 +2455,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
     char *dompath, **l;
     unsigned int nb, rc;
 
-    rc = libxl__device_nic_setdefault(gc, nic);
+    rc = libxl__device_nic_setdefault(gc, nic, domid);
     if (rc) goto out;
 
     front = flexarray_make(16, 1);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9330012..1cacfad 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -941,7 +941,9 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
          * called libxl_device_nic_add at this point, but qemu needs
          * the nic information to be complete.
          */
-        libxl__device_nic_setdefault(gc, &d_config->nics[i]);
+        ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
+        if (ret)
+            goto error_out;
     }
     switch (d_config->c_info.type) {
     case LIBXL_DOMAIN_TYPE_HVM:
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index dec7c19..606cc29 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -898,7 +898,9 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
          * called libxl_device_nic_add at this point, but qemu needs
          * the nic information to be complete.
          */
-        libxl__device_nic_setdefault(gc, &dm_config->nics[i]);
+        ret = libxl__device_nic_setdefault(gc, &dm_config->nics[i], dm_domid);
+        if (ret)
+            goto out;
     }
     ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
     if (ret)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2add151..aca21ca 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -897,7 +897,8 @@ _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
                                         libxl_domain_build_info *b_info);
 _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
                                           libxl_device_disk *disk);
-_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic);
+_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
+                                         uint32_t domid);
 _hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
 _hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
 _hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 11/17] libxl: use libxl__xs_path_cleanup on device_destroy
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (9 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 10/17] libxl: set correct nic type depending on the guest Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 12/17] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

Since the hotplug script that was in charge of cleaning the backend is
no longer launched, we need to clean the backend by ourselves, so use
libxl__xs_path_cleanup instead of xs_rm.

Changes sinve v2:

 * Changed the goto construction to a do-while loop.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl_device.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 8322b86..ed33f6f 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -482,16 +482,26 @@ DEFINE_DEVICES_ADD(nic)
 
 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_transaction_t t = 0;
+    int rc = 0;
 
-    xs_rm(ctx->xsh, XBT_NULL, be_path);
-    xs_rm(ctx->xsh, XBT_NULL, fe_path);
+    do {
+        t = xs_transaction_start(CTX->xsh);
+        libxl__xs_path_cleanup(gc, t, fe_path);
+        libxl__xs_path_cleanup(gc, t, be_path);
+        rc = !xs_transaction_end(CTX->xsh, t, 0);
+    } while (rc && errno == EAGAIN);
+    if (rc) {
+        LOGE(ERROR, "unable to finish transaction");
+        goto out;
+    }
 
     libxl__device_destroy_tapdisk(gc, be_path);
 
-    return 0;
+out:
+    return rc;
 }
 
 /* Callback for device destruction */
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 12/17] libxl: call hotplug scripts for disk devices from libxl
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (10 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 11/17] libxl: use libxl__xs_path_cleanup on device_destroy Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 13/17] libxl: call hotplug scripts for nic " Roger Pau Monne
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 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.

Changes since v10:

 * Changed error message of hotplug script error.

Changes since v8:

 * Print "hotplug_error" xenstore entry if hotplug execution fails and
   hotplug_error contains information.

Changes since v5:

 * Added common exit point using the device_hotplug_done function.

 * Changed the "what" field in ao_device to "const char *".

Changes since v4:

 * Init args and env to NULL.

 * Correctly propagate errors from libxl__get_hotplug_script_info.

 * Replaced if in libxl__ev_child_inuse with asserts.

 * Renamed fork_cb to child_death_cb.

 * Move deregistration of device_hotplug_timeout_cb to the top of the
   function.

 * Removed "pid" field from libxl__ao_device struct.

 * Moved arraysize declaration.

 * Fixed diff complain about no newline at the end of libxl_netbsd.c.

Changes since v2:

 * Added array size check with assert.

 * Added NetBSD code (so compilation is not broken).

 * Removed a check for null in device_hotplug_timeout_cb.

Changes since v1:

 * Moved all the event related code that was inside libxl_linux.c into
   libxl_device.c, so the flow of the device addition/removal event is
   all in the same file.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: 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/libxl.c                       |   10 ++
 tools/libxl/libxl_device.c                |  132 ++++++++++++++++++++++++++++-
 tools/libxl/libxl_internal.h              |   20 ++++-
 tools/libxl/libxl_linux.c                 |   97 +++++++++++++++++++++
 tools/libxl/libxl_netbsd.c                |    8 ++
 7 files changed, 273 insertions(+), 6 deletions(-)

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/libxl.c b/tools/libxl/libxl.c
index b0e2ec1..14793f3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1841,6 +1841,11 @@ static void 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:
@@ -1856,6 +1861,11 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
                     libxl__device_disk_string_of_format(disk->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:
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index ed33f6f..a80dee8 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -412,9 +412,12 @@ void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev)
     aodev->ao = ao;
     aodev->rc = 0;
     aodev->dev = NULL;
-    /* Initialize timer for QEMU Bodge */
+    /* Initialize timer for QEMU Bodge and hotplug execution */
     libxl__ev_time_init(&aodev->timeout);
     aodev->active = 1;
+    /* We init this here because we might call device_hotplug_done
+     * without actually calling any hotplug script */
+    libxl__ev_child_init(&aodev->child);
 }
 
 void libxl__prepare_ao_devices(libxl__ao *ao, libxl__ao_devices *aodevs)
@@ -624,6 +627,15 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
 static void device_backend_cleanup(libxl__gc *gc,
                                    libxl__ao_device *aodev);
 
+static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev);
+
+static void device_hotplug_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
+                                      const struct timeval *requested_abs);
+
+static void device_hotplug_child_death_cb(libxl__egc *egc,
+                                          libxl__ev_child *child,
+                                          pid_t pid, int status);
+
 static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev);
 
 void libxl__wait_device_connection(libxl__egc *egc, libxl__ao_device *aodev)
@@ -649,7 +661,7 @@ void libxl__wait_device_connection(libxl__egc *egc, libxl__ao_device *aodev)
          * If Qemu is running, it will set the state of the device to
          * 4 directly, without waiting in state 2 for any hotplug execution.
          */
-        device_hotplug_done(egc, aodev);
+        device_hotplug(egc, aodev);
         return;
     }
 
@@ -783,6 +795,9 @@ static void device_qemu_timeout(libxl__egc *egc, libxl__ev_time *ev,
     rc = libxl__xs_write_checked(gc, XBT_NULL, state_path, "6");
     if (rc) goto out;
 
+    device_hotplug(egc, aodev);
+    return;
+
 out:
     aodev->rc = rc;
     device_hotplug_done(egc, aodev);
@@ -808,6 +823,9 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
         goto out;
     }
 
+    device_hotplug(egc, aodev);
+    return;
+
 out:
     aodev->rc = rc;
     device_hotplug_done(egc, aodev);
@@ -820,6 +838,111 @@ static void device_backend_cleanup(libxl__gc *gc, libxl__ao_device *aodev)
     libxl__ev_devstate_cancel(gc, &aodev->backend_ds);
 }
 
+static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    char *be_path = libxl__device_backend_path(gc, aodev->dev);
+    char **args = NULL, **env = NULL;
+    int rc = 0;
+    int hotplug;
+    pid_t pid;
+
+    /* Check if we have to execute hotplug scripts for this device
+     * and return the necessary args/env vars for execution */
+    hotplug = libxl__get_hotplug_script_info(gc, aodev->dev, &args, &env,
+                                             aodev->action);
+    switch (hotplug) {
+    case 0:
+        /* no hotplug script to execute */
+        goto out;
+    case 1:
+        /* execute hotplug script */
+        break;
+    default:
+        /* everything else is an error */
+        LOG(ERROR, "unable to get args/env to execute hotplug script for "
+                   "device %s", libxl__device_backend_path(gc, aodev->dev));
+        rc = hotplug;
+        goto out;
+    }
+
+    /* Set hotplug timeout */
+    rc = libxl__ev_time_register_rel(gc, &aodev->timeout,
+                                     device_hotplug_timeout_cb,
+                                     LIBXL_HOTPLUG_TIMEOUT * 1000);
+    if (rc) {
+        LOG(ERROR, "unable to register timeout for hotplug device %s", be_path);
+        goto out;
+    }
+
+    aodev->what = GCSPRINTF("%s %s", args[0], args[1]);
+    LOG(DEBUG, "calling hotplug script: %s %s", args[0], args[1]);
+
+    /* fork and execute hotplug script */
+    pid = libxl__ev_child_fork(gc, &aodev->child, device_hotplug_child_death_cb);
+    if (pid == -1) {
+        LOG(ERROR, "unable to fork");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    if (!pid) {
+        /* child */
+        libxl__exec(gc, -1, -1, -1, args[0], args, env);
+        /* notreached */
+        abort();
+    }
+
+    assert(libxl__ev_child_inuse(&aodev->child));
+
+    return;
+
+out:
+    aodev->rc = rc;
+    device_hotplug_done(egc, aodev);
+    return;
+}
+
+static void device_hotplug_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
+                                      const struct timeval *requested_abs)
+{
+    libxl__ao_device *aodev = CONTAINER_OF(ev, *aodev, timeout);
+    STATE_AO_GC(aodev->ao);
+
+    libxl__ev_time_deregister(gc, &aodev->timeout);
+
+    assert(libxl__ev_child_inuse(&aodev->child));
+    LOG(DEBUG, "killing hotplug script %s because of timeout", aodev->what);
+    if (kill(aodev->child.pid, SIGKILL)) {
+        LOGEV(ERROR, errno, "unable to kill hotplug script %s [%ld]",
+                            aodev->what, (unsigned long)aodev->child.pid);
+    }
+
+    return;
+}
+
+static void device_hotplug_child_death_cb(libxl__egc *egc,
+                                          libxl__ev_child *child,
+                                          pid_t pid, int status)
+{
+    libxl__ao_device *aodev = CONTAINER_OF(child, *aodev, child);
+    STATE_AO_GC(aodev->ao);
+    char *be_path = libxl__device_backend_path(gc, aodev->dev);
+    char *hotplug_error;
+
+    if (status) {
+        libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
+                                      aodev->what, pid, status);
+        hotplug_error = libxl__xs_read(gc, XBT_NULL,
+                                       GCSPRINTF("%s/hotplug-error", be_path));
+        if (hotplug_error)
+            LOG(ERROR, "script: %s", hotplug_error);
+        aodev->rc = ERROR_FAIL;
+    }
+
+    device_hotplug_done(egc, aodev);
+}
+
 static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
@@ -828,6 +951,11 @@ static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev)
     xs_transaction_t t = 0;
     int rc;
 
+    /* Clean events and check reentrancy */
+    libxl__ev_time_deregister(gc, &aodev->timeout);
+    assert(!libxl__ev_child_inuse(&aodev->child));
+
+    /* Clean xenstore if it's a disconnection */
     if (aodev->action == DEVICE_DISCONNECT) {
         for (;;) {
             rc = libxl__xs_transaction_start(gc, &t);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index aca21ca..8d701ef 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -74,6 +74,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_QEMU_BODGE_TIMEOUT 2
 #define LIBXL_XENCONSOLE_LIMIT 1048576
@@ -1792,11 +1793,14 @@ struct libxl__ao_device {
     int active;
     int rc;
     libxl__ev_devstate backend_ds;
-    /* Bodge for Qemu devices */
+    /* Bodge for Qemu devices, also used for timeout of hotplug execution */
     libxl__ev_time timeout;
     /* Used internally to have a reference to the upper libxl__ao_devices
      * struct when present */
     libxl__ao_devices *aodevs;
+    /* device hotplug execution */
+    const char *what;
+    libxl__ev_child child;
 };
 
 /* Helper struct to simply the plug/unplug of multiple devices at the same
@@ -1926,6 +1930,20 @@ _hidden void libxl__wait_device_connection(libxl__egc*,
 _hidden void libxl__initiate_device_remove(libxl__egc *egc,
                                            libxl__ao_device *aodev);
 
+/*
+ * libxl__get_hotplug_script_info returns the args and env that should
+ * be passed to the hotplug script for the requested device.
+ *
+ * Since a device might not need to execute any hotplug script, this function
+ * can return the following values:
+ * < 0: Error
+ * 0: No need to execute hotplug script
+ * 1: Execute hotplug script
+ */
+_hidden int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
+                                           char ***args, char ***env,
+                                           libxl__device_action action);
+
 /*----- local disk attach: attach a disk locally to run the bootloader -----*/
 
 typedef struct libxl__disk_local_state libxl__disk_local_state;
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 0169b2f..97b3fd4 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -77,3 +77,100 @@ char *libxl__devid_to_localdev(libxl__gc *gc, int devid)
                 "%d", minor & (nr_parts - 1));
     return ret;
 }
+
+/* Hotplug scripts helpers */
+
+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;
+    }
+
+    const int arraysize = 9;
+    GCNEW_ARRAY(env, arraysize);
+    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;
+    assert(nr == arraysize);
+
+    return env;
+}
+
+/* Hotplug scripts caller functions */
+
+static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
+                               char ***args, char ***env,
+                               libxl__device_action action)
+{
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *script;
+    int nr = 0, rc = 0;
+
+    script = libxl__xs_read(gc, XBT_NULL,
+                            GCSPRINTF("%s/%s", be_path, "script"));
+    if (!script) {
+        LOGEV(ERROR, errno, "unable to read script from %s", be_path);
+        rc = ERROR_FAIL;
+        goto error;
+    }
+
+    *env = get_hotplug_env(gc, dev);
+    if (!*env) {
+        rc = ERROR_FAIL;
+        goto error;
+    }
+
+    const int arraysize = 3;
+    GCNEW_ARRAY(*args, arraysize);
+    (*args)[nr++] = script;
+    (*args)[nr++] = action == DEVICE_CONNECT ? "add" : "remove";
+    (*args)[nr++] = NULL;
+    assert(nr == arraysize);
+
+    rc = 1;
+
+error:
+    return rc;
+}
+
+int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
+                                   char ***args, char ***env,
+                                   libxl__device_action action)
+{
+    char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH);
+    int rc;
+
+    /* Check if we have to run hotplug scripts */
+    if (!disable_udev) {
+        rc = 0;
+        goto out;
+    }
+
+    switch (dev->backend_kind) {
+    case LIBXL__DEVICE_KIND_VBD:
+        rc = libxl__hotplug_disk(gc, dev, args, env, action);
+        break;
+    default:
+        /* No need to execute any hotplug scripts */
+        rc = 0;
+        break;
+    }
+
+out:
+    return rc;
+}
diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index dbf5f71..a2f8d3f 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -30,3 +30,11 @@ char *libxl__devid_to_localdev(libxl__gc *gc, int devid)
     /* TODO */
     return NULL;
 }
+
+/* Hotplug scripts caller functions */
+int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
+                                   char ***args, char ***env,
+                                   libxl__device_action action)
+{
+    return 0;
+}
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 13/17] libxl: call hotplug scripts for nic devices from libxl
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (11 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 12/17] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 14/17] libxl: convert libxl_device_vkb_add to an async operation Roger Pau Monne
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 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.

Added another parameter to libxl__get_hotplug_script_info, that is
used to know the number of times hotplug scripts have been called for
that device. This is currently used by IOEMU nics on Linux.

Changes since v10:

 * Removed strdups.

 * Added comment on VIF_IOEMU fall through for get_hotplug_env.

 * Don't execute tap hotplug scripts if domain has a studbom.

Changes since v8:

 * Correctly detect errors from hotplug execution and cancel domain
   creation if an error is found.

Changes since v6:

 * Added an rc = 0 before exitting libxl__nic_type if successful.

Changes since v4:

 * Add num_exec to NetBSD dummy function.

 * Better comment in the prototype of libxl__get_hotplug_script_info.

 * Remove nasty use of goto.

 * Keep calling device_hotplug until libxl__get_hotplug_script_info
   returns <= 0. This is used by TAP nics which also have a VIF
   interface (PV_IOEMU).

Changes since v2:

 * Change libxl__nic_type to return the value in a parameter passed by
   the caller.

 * Rename vif_execute to num_exec, to represent the number of times
   hotplug scripts have been called for that device.

Changes since v1:

 * Move event code to libxl_device.c (as in previous patch).

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: 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_device.c            |   60 +++++++++++++++++-
 tools/libxl/libxl_internal.h          |   14 ++++-
 tools/libxl/libxl_linux.c             |  108 +++++++++++++++++++++++++++++++-
 tools/libxl/libxl_netbsd.c            |    3 +-
 5 files changed, 178 insertions(+), 13 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_device.c b/tools/libxl/libxl_device.c
index a80dee8..da0c3ea 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -102,6 +102,31 @@ out:
     return numdevs;
 }
 
+int libxl__nic_type(libxl__gc *gc, libxl__device *dev, libxl_nic_type *nictype)
+{
+    char *snictype, *be_path;
+    int rc = 0;
+
+    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);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    rc = libxl_nic_type_from_string(snictype, nictype);
+    if (rc) {
+        LOGE(ERROR, "unable to parse nictype from %s", be_path);
+        goto out;
+    }
+
+    rc = 0;
+
+out:
+    return rc;
+}
+
 int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
         libxl__device *device, char **bents, char **fents)
 {
@@ -638,6 +663,8 @@ static void device_hotplug_child_death_cb(libxl__egc *egc,
 
 static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev);
 
+static void device_hotplug_clean(libxl__gc *gc, libxl__ao_device *aodev);
+
 void libxl__wait_device_connection(libxl__egc *egc, libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
@@ -850,7 +877,8 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
     /* Check if we have to execute hotplug scripts for this device
      * and return the necessary args/env vars for execution */
     hotplug = libxl__get_hotplug_script_info(gc, aodev->dev, &args, &env,
-                                             aodev->action);
+                                             aodev->action,
+                                             aodev->num_exec);
     switch (hotplug) {
     case 0:
         /* no hotplug script to execute */
@@ -930,6 +958,8 @@ static void device_hotplug_child_death_cb(libxl__egc *egc,
     char *be_path = libxl__device_backend_path(gc, aodev->dev);
     char *hotplug_error;
 
+    device_hotplug_clean(gc, aodev);
+
     if (status) {
         libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
                                       aodev->what, pid, status);
@@ -938,8 +968,25 @@ static void device_hotplug_child_death_cb(libxl__egc *egc,
         if (hotplug_error)
             LOG(ERROR, "script: %s", hotplug_error);
         aodev->rc = ERROR_FAIL;
+        if (aodev->action == DEVICE_CONNECT)
+            /*
+             * Only fail on device connection, on disconnection
+             * ignore error, and continue with the remove process
+             */
+             goto error;
     }
 
+    /* Increase num_exec and call hotplug scripts again if necessary
+     * If no more executions are needed, device_hotplug will call
+     * device_hotplug_done breaking the loop.
+     */
+    aodev->num_exec++;
+    device_hotplug(egc, aodev);
+
+    return;
+
+error:
+    assert(aodev->rc);
     device_hotplug_done(egc, aodev);
 }
 
@@ -951,9 +998,7 @@ static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev)
     xs_transaction_t t = 0;
     int rc;
 
-    /* Clean events and check reentrancy */
-    libxl__ev_time_deregister(gc, &aodev->timeout);
-    assert(!libxl__ev_child_inuse(&aodev->child));
+    device_hotplug_clean(gc, aodev);
 
     /* Clean xenstore if it's a disconnection */
     if (aodev->action == DEVICE_DISCONNECT) {
@@ -975,6 +1020,13 @@ out:
     return;
 }
 
+static void device_hotplug_clean(libxl__gc *gc, libxl__ao_device *aodev)
+{
+    /* Clean events and check reentrancy */
+    libxl__ev_time_deregister(gc, &aodev->timeout);
+    assert(!libxl__ev_child_inuse(&aodev->child));
+}
+
 static void devices_remove_callback(libxl__egc *egc, libxl__ao_devices *aodevs,
                                     int rc)
 {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8d701ef..be6ca49 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -879,6 +879,8 @@ _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__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
+_hidden int libxl__nic_type(libxl__gc *gc, libxl__device *dev,
+                            libxl_nic_type *nictype);
 
 /*
  * For each aggregate type which can be used as an input we provide:
@@ -1800,6 +1802,7 @@ struct libxl__ao_device {
     libxl__ao_devices *aodevs;
     /* device hotplug execution */
     const char *what;
+    int num_exec;
     libxl__ev_child child;
 };
 
@@ -1939,10 +1942,19 @@ _hidden void libxl__initiate_device_remove(libxl__egc *egc,
  * < 0: Error
  * 0: No need to execute hotplug script
  * 1: Execute hotplug script
+ *
+ * The last parameter, "num_exec" refeers to the number of times hotplug
+ * scripts have been called for this device.
+ *
+ * The main body of libxl will, for each device, keep calling
+ * libxl__get_hotplug_script_info, with incrementing values of
+ * num_exec, and executing the resulting script accordingly,
+ * until libxl__get_hotplug_script_info returns<=0.
  */
 _hidden int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                            char ***args, char ***env,
-                                           libxl__device_action action);
+                                           libxl__device_action action,
+                                           int num_exec);
 
 /*----- local disk attach: attach a disk locally to run the bootloader -----*/
 
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 97b3fd4..9ae0b4b 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -87,6 +87,7 @@ static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
     const char *type = libxl__device_kind_to_string(dev->backend_kind);
     char **env;
     int nr = 0;
+    libxl_nic_type nictype;
 
     script = libxl__xs_read(gc, XBT_NULL,
                             GCSPRINTF("%s/%s", be_path, "script"));
@@ -95,24 +96,106 @@ static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
         return NULL;
     }
 
-    const int arraysize = 9;
+    const int arraysize = 13;
     GCNEW_ARRAY(env, arraysize);
     env[nr++] = "script";
     env[nr++] = script;
     env[nr++] = "XENBUS_TYPE";
-    env[nr++] = libxl__strdup(gc, type);
+    env[nr++] = (char *) 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";
+    if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
+        if (libxl__nic_type(gc, dev, &nictype)) {
+            LOG(ERROR, "unable to get nictype");
+            return NULL;
+        }
+        switch (nictype) {
+        case LIBXL_NIC_TYPE_VIF_IOEMU:
+            env[nr++] = "INTERFACE";
+            env[nr++] = (char *) libxl__device_nic_devname(gc, dev->domid,
+                                                    dev->devid,
+                                                    LIBXL_NIC_TYPE_VIF_IOEMU);
+            /*
+             * We need to fall through because for PV_IOEMU nic types we need
+             * to execute both the vif and the tap hotplug script, and we
+             * don't know which one we are executing in this call, so provide
+             * both env variables.
+             */
+        case LIBXL_NIC_TYPE_VIF:
+            env[nr++] = "vif";
+            env[nr++] = (char *) libxl__device_nic_devname(gc, dev->domid,
+                                                    dev->devid,
+                                                    LIBXL_NIC_TYPE_VIF);
+            break;
+        default:
+            return NULL;
+        }
+    }
+
     env[nr++] = NULL;
-    assert(nr == arraysize);
+    assert(nr <= arraysize);
 
     return env;
 }
 
 /* Hotplug scripts caller functions */
 
+static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev,
+                               char ***args, char ***env,
+                               libxl__device_action action, int num_exec)
+{
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *script;
+    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);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = libxl__nic_type(gc, dev, &nictype);
+    if (rc) {
+        LOG(ERROR, "error when fetching nic type");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (nictype == LIBXL_NIC_TYPE_VIF && num_exec != 0) {
+        rc = 0;
+        goto out;
+    }
+
+    *env = get_hotplug_env(gc, dev);
+    if (!env) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    const int arraysize = 4;
+    GCNEW_ARRAY(*args, arraysize);
+    (*args)[nr++] = script;
+
+    if (nictype == LIBXL_NIC_TYPE_VIF_IOEMU && num_exec) {
+        (*args)[nr++] = action == DEVICE_CONNECT ? "add" : "remove";
+        (*args)[nr++] = "type_if=tap";
+        (*args)[nr++] = NULL;
+    } else {
+        (*args)[nr++] = action == DEVICE_CONNECT ? "online" : "offline";
+        (*args)[nr++] = "type_if=vif";
+        (*args)[nr++] = NULL;
+    }
+    assert(nr == arraysize);
+    rc = 1;
+
+out:
+    return rc;
+}
+
 static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
                                char ***args, char ***env,
                                libxl__device_action action)
@@ -150,7 +233,8 @@ error:
 
 int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    char ***args, char ***env,
-                                   libxl__device_action action)
+                                   libxl__device_action action,
+                                   int num_exec)
 {
     char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH);
     int rc;
@@ -163,8 +247,24 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
 
     switch (dev->backend_kind) {
     case LIBXL__DEVICE_KIND_VBD:
+        if (num_exec != 0) {
+            rc = 0;
+            goto out;
+        }
         rc = libxl__hotplug_disk(gc, dev, args, env, action);
         break;
+    case LIBXL__DEVICE_KIND_VIF:
+        /*
+         * If domain has a stubdom we don't have to execute hotplug scripts
+         * for emulated interfaces
+         */
+        if ((num_exec > 1) ||
+            (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) {
+            rc = 0;
+            goto out;
+        }
+        rc = libxl__hotplug_nic(gc, dev, args, env, action, num_exec);
+        break;
     default:
         /* No need to execute any hotplug scripts */
         rc = 0;
diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index a2f8d3f..28cdf21 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -34,7 +34,8 @@ char *libxl__devid_to_localdev(libxl__gc *gc, int devid)
 /* Hotplug scripts caller functions */
 int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    char ***args, char ***env,
-                                   libxl__device_action action)
+                                   libxl__device_action action,
+                                   int num_exec)
 {
     return 0;
 }
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 14/17] libxl: convert libxl_device_vkb_add to an async operation
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (12 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 13/17] libxl: call hotplug scripts for nic " Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-26 10:41   ` Ian Jackson
  2012-07-25 11:19 ` [PATCH v12 15/17] libxl: convert libxl_device_vfb_add " Roger Pau Monne
                   ` (4 subsequent siblings)
  18 siblings, 1 reply; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

Split libxl_device_vkb_add into libxl__device_vkb_add (to be used
inside already running ao's), and make libxl_device_vkb_add a stub to
call libxl__device_vkb_add.

Changes since v11:

 * libxl__device_vkb_add is no longer an ao operation.

Changes since v9:

 * Fixed Ocaml bindings.

 * Don't abort if num_vkbs > 0.

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                  |    3 ++-
 tools/libxl/libxl_create.c           |    4 ++--
 tools/libxl/libxl_dm.c               |    2 +-
 tools/libxl/libxl_internal.h         |    4 ++++
 tools/ocaml/libs/xl/xenlight_stubs.c |    2 +-
 6 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 14793f3..8d82526 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2840,9 +2840,26 @@ static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
-int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb)
+int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb,
+                         const libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx, domid, ao_how);
+    int rc;
+
+    rc = libxl__device_vkb_add(gc, domid, vkb);
+    if (rc) {
+        LOG(ERROR, "unable to add vkb device");
+        goto out;
+    }
+
+out:
+    libxl__ao_complete(egc, ao, rc);
+    return AO_INPROGRESS;
+}
+
+int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
+                          libxl_device_vkb *vkb)
 {
-    GC_INIT(ctx);
     flexarray_t *front;
     flexarray_t *back;
     libxl__device device;
@@ -2887,7 +2904,6 @@ out_free:
     flexarray_free(back);
     flexarray_free(front);
 out:
-    GC_FREE;
     return rc;
 }
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index d548090..84d4efd 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -710,7 +710,8 @@ int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_nic *nic, libxl_nicinfo *nicinfo);
 
 /* Keyboard */
-int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
+int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb,
+                         const libxl_asyncop_how *ao_how);
 int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
                             libxl_device_vkb *vkb,
                             const libxl_asyncop_how *ao_how);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1cacfad..0a0ba5c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -958,7 +958,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
         libxl__device_console_dispose(&console);
 
         libxl_device_vkb_init(&vkb);
-        libxl_device_vkb_add(ctx, domid, &vkb);
+        libxl__device_vkb_add(gc, domid, &vkb);
         libxl_device_vkb_dispose(&vkb);
 
         dcs->dmss.dm.guest_domid = domid;
@@ -975,7 +975,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
 
         for (i = 0; i < d_config->num_vfbs; i++) {
             libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]);
-            libxl_device_vkb_add(ctx, domid, &d_config->vkbs[i]);
+            libxl__device_vkb_add(gc, domid, &d_config->vkbs[i]);
         }
 
         ret = init_console_info(&console, 0);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 606cc29..47a812f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -905,7 +905,7 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
     ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
     if (ret)
         goto out;
-    ret = libxl_device_vkb_add(ctx, dm_domid, &dm_config->vkbs[0]);
+    ret = libxl__device_vkb_add(gc, dm_domid, &dm_config->vkbs[0]);
     if (ret)
         goto out;
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index be6ca49..1e2d96b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1909,6 +1909,10 @@ _hidden void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
                                    libxl_device_nic *nic,
                                    libxl__ao_device *aodev);
 
+/* Internal function to connect a vkb device */
+_hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
+                                  libxl_device_vkb *vkb);
+
 /* Waits for the passed device to reach state XenbusStateInitWait.
  * This is not really useful by itself, but is important when executing
  * hotplug scripts, since we need to be sure the device is in the correct
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index 0e9c65e..3d2493b 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -315,7 +315,7 @@ value stub_xl_device_vkb_add(value info, value domid)
 	device_vkb_val(&gc, &lg, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_vkb_add(ctx, Int_val(domid), &c_info);
+	ret = libxl_device_vkb_add(ctx, Int_val(domid), &c_info, 0);
 	if (ret != 0)
 		failwith_xl("vkb_add", &lg);
 	FREE_CTX();
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 15/17] libxl: convert libxl_device_vfb_add to an async operation
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (13 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 14/17] libxl: convert libxl_device_vkb_add to an async operation Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-26 10:41   ` Ian Jackson
  2012-07-25 11:19 ` [PATCH v12 16/17] xl: main_blockdetach don't call destroy if remove succeeds Roger Pau Monne
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

Split libxl_device_vfb_add into libxl__device_vfb_add (to be used
inside already running ao's), and make libxl_device_vfb_add a stub
to call libxl__device_vfb_add.

Changes since v11:

 * libxl__device_vfb_add is no longer an ao operation.

Changes since v9:

 * Fixed Ocaml bindings.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c                  |   21 ++++++++++++++++++---
 tools/libxl/libxl.h                  |    3 ++-
 tools/libxl/libxl_create.c           |    3 +--
 tools/libxl/libxl_dm.c               |    2 +-
 tools/libxl/libxl_internal.h         |    4 ++++
 tools/ocaml/libs/xl/xenlight_stubs.c |    2 +-
 6 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 8d82526..8bd54f9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2940,9 +2940,25 @@ static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
-int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb)
+int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb,
+                         const libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx, domid, ao_how);
+    int rc;
+
+    rc = libxl__device_vfb_add(gc, domid, vfb);
+    if (rc) {
+        LOG(ERROR, "unable to add vfb device");
+        goto out;
+    }
+
+out:
+    libxl__ao_complete(egc, ao, rc);
+    return AO_INPROGRESS;
+}
+
+int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
 {
-    GC_INIT(ctx);
     flexarray_t *front;
     flexarray_t *back;
     libxl__device device;
@@ -3000,7 +3016,6 @@ out_free:
     flexarray_free(front);
     flexarray_free(back);
 out:
-    GC_FREE;
     return rc;
 }
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 84d4efd..414475e 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -720,7 +720,8 @@ int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid,
                              const libxl_asyncop_how *ao_how);
 
 /* Framebuffer */
-int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
+int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb,
+                         const libxl_asyncop_how *ao_how);
 int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
                             libxl_device_vfb *vfb,
                             const libxl_asyncop_how *ao_how);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 0a0ba5c..5859c09 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -930,7 +930,6 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
     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;
 
     if (ret) {
         LOG(ERROR, "unable to add disk devices");
@@ -974,7 +973,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
         libxl__device_console console;
 
         for (i = 0; i < d_config->num_vfbs; i++) {
-            libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]);
+            libxl__device_vfb_add(gc, domid, &d_config->vfbs[i]);
             libxl__device_vkb_add(gc, domid, &d_config->vkbs[i]);
         }
 
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 47a812f..f2e9572 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -902,7 +902,7 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
         if (ret)
             goto out;
     }
-    ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
+    ret = libxl__device_vfb_add(gc, dm_domid, &dm_config->vfbs[0]);
     if (ret)
         goto out;
     ret = libxl__device_vkb_add(gc, dm_domid, &dm_config->vkbs[0]);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1e2d96b..5f0d789 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1913,6 +1913,10 @@ _hidden void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
 _hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
                                   libxl_device_vkb *vkb);
 
+/* Internal function to connect a vfb device */
+_hidden int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid,
+                                  libxl_device_vfb *vfb);
+
 /* Waits for the passed device to reach state XenbusStateInitWait.
  * This is not really useful by itself, but is important when executing
  * hotplug scripts, since we need to be sure the device is in the correct
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index 3d2493b..adda67f 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -369,7 +369,7 @@ value stub_xl_device_vfb_add(value info, value domid)
 	device_vfb_val(&gc, &lg, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_vfb_add(ctx, Int_val(domid), &c_info);
+	ret = libxl_device_vfb_add(ctx, Int_val(domid), &c_info, 0);
 	if (ret != 0)
 		failwith_xl("vfb_add", &lg);
 	FREE_CTX();
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 16/17] xl: main_blockdetach don't call destroy if remove succeeds
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (14 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 15/17] libxl: convert libxl_device_vfb_add " Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-25 11:19 ` [PATCH v12 17/17] libxl: libxl__xs_path_cleanup don't print error if ENOENT Roger Pau Monne
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne

xl was calling libxl_device_disk_destroy after a successful call to
libxl_device_disk_remove, which leads to an error.

Changes since v10:

 * Return error if libxl_device_disk_remove fails.

 * Call libxl_device_disk_dispose unconditionally.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4028f54..6df1fbd 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5399,7 +5399,7 @@ int main_blocklist(int argc, char **argv)
 
 int main_blockdetach(int argc, char **argv)
 {
-    int opt;
+    int opt, rc = 0;
     libxl_device_disk disk;
 
     if ((opt = def_getopt(argc, argv, "", "block-detach", 2)) != -1)
@@ -5413,11 +5413,12 @@ int main_blockdetach(int argc, char **argv)
         fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
         return 1;
     }
-    if (libxl_device_disk_remove(ctx, domid, &disk, 0)) {
+    rc = libxl_device_disk_remove(ctx, domid, &disk, 0);
+    if (rc) {
         fprintf(stderr, "libxl_device_disk_remove failed.\n");
-    } else
-        libxl_device_disk_destroy(ctx, domid, &disk, 0);
-    return 0;
+    }
+    libxl_device_disk_dispose(&disk);
+    return rc;
 }
 
 static char *uptime_to_string(unsigned long time, int short_mode)
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v12 17/17] libxl: libxl__xs_path_cleanup don't print error if ENOENT
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (15 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 16/17] xl: main_blockdetach don't call destroy if remove succeeds Roger Pau Monne
@ 2012-07-25 11:19 ` Roger Pau Monne
  2012-07-26  9:36 ` [PATCH v12 00/17] execute hotplug scripts from libxl Ian Campbell
  2012-07-26  9:55 ` Ian Campbell
  18 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-25 11:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell, Roger Pau Monne

Cc: Ian Campbell <ian.campbell@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl_xshelp.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index 7ca1732..855ac85 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -219,7 +219,8 @@ int libxl__xs_path_cleanup(libxl__gc *gc, xs_transaction_t t, char *user_path)
 
     path = libxl__strdup(gc, user_path);
     if (!xs_rm(CTX->xsh, t, path)) {
-        LOGE(DEBUG, "unable to remove path %s", path);
+        if (errno != ENOENT)
+            LOGE(DEBUG, "unable to remove path %s", path);
         rc = ERROR_FAIL;
         goto out;
     }
@@ -235,7 +236,8 @@ int libxl__xs_path_cleanup(libxl__gc *gc, xs_transaction_t t, char *user_path)
         if (!libxl__xs_directory(gc, t, path, &nb) || nb != 0) break;
 
         if (!xs_rm(CTX->xsh, t, path)) {
-            LOGE(DEBUG, "unable to remove path %s", path);
+            if (errno != ENOENT)
+                LOGE(DEBUG, "unable to remove path %s", path);
             rc = ERROR_FAIL;
             goto out;
         }
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 00/17] execute hotplug scripts from libxl
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (16 preceding siblings ...)
  2012-07-25 11:19 ` [PATCH v12 17/17] libxl: libxl__xs_path_cleanup don't print error if ENOENT Roger Pau Monne
@ 2012-07-26  9:36 ` Ian Campbell
  2012-07-26  9:41   ` Roger Pau Monne
  2012-07-26  9:55 ` Ian Campbell
  18 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-07-26  9:36 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

On Wed, 2012-07-25 at 12:19 +0100, Roger Pau Monne wrote:
> Patches 4, 8, 11 and 16, that are already Acked can be commited, since 
> they don't require the previous patches, and should apply cleanly 
> since they touch different parts of the code.

I have applied these.

>       libxl: set nic type of stub to PV instead of copying from the 

I also Acked+applied this one.

Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 00/17] execute hotplug scripts from libxl
  2012-07-26  9:36 ` [PATCH v12 00/17] execute hotplug scripts from libxl Ian Campbell
@ 2012-07-26  9:41   ` Roger Pau Monne
  0 siblings, 0 replies; 30+ messages in thread
From: Roger Pau Monne @ 2012-07-26  9:41 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell wrote:
> On Wed, 2012-07-25 at 12:19 +0100, Roger Pau Monne wrote:
>> Patches 4, 8, 11 and 16, that are already Acked can be commited, since 
>> they don't require the previous patches, and should apply cleanly 
>> since they touch different parts of the code.
> 
> I have applied these.
> 
>>       libxl: set nic type of stub to PV instead of copying from the 
> 
> I also Acked+applied this one.

Thanks.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 00/17] execute hotplug scripts from libxl
  2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
                   ` (17 preceding siblings ...)
  2012-07-26  9:36 ` [PATCH v12 00/17] execute hotplug scripts from libxl Ian Campbell
@ 2012-07-26  9:55 ` Ian Campbell
  18 siblings, 0 replies; 30+ messages in thread
From: Ian Campbell @ 2012-07-26  9:55 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

On Wed, 2012-07-25 at 12:19 +0100, Roger Pau Monne wrote:
> Reworked patch 1 and 9, and fixed the local attach problems. Not 
> having an Ack on patch 1 should not prevent the rest from going in.
> 
> Patches 4, 8, 11 and 16, that are already Acked can be commited, since 
> they don't require the previous patches, and should apply cleanly 
> since they touch different parts of the code.

#11 is:
>      *libxl: use libxl__xs_path_cleanup on device_destroy

which is a bit spammy without:
>       libxl: libxl__xs_path_cleanup don't print error if ENOENT

Therefore I have tested, acked + applied that too.

Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 09/17] libxl: set nic type of stub to PV instead of copying from the parent
  2012-07-25 11:19 ` [PATCH v12 09/17] libxl: set nic type of stub to PV instead of copying from the parent Roger Pau Monne
@ 2012-07-26 10:38   ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-07-26 10:38 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: Ian Campbell, xen-devel

Roger Pau Monne writes ("[PATCH v12 09/17] libxl: set nic type of stub to PV instead of copying from the parent"):
> Set the type of the nics used in sutbdoms to PV unconditianlly, or the
> call to setdefaults later is going to fail.
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@eu.citrix.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 01/17] libxl: fix removal of secondary consoles
  2012-07-25 11:19 ` [PATCH v12 01/17] libxl: fix removal of secondary consoles Roger Pau Monne
@ 2012-07-26 10:39   ` Ian Campbell
  2012-07-26 10:43   ` Ian Campbell
  1 sibling, 0 replies; 30+ messages in thread
From: Ian Campbell @ 2012-07-26 10:39 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: Ian Jackson, xen-devel

On Wed, 2012-07-25 at 12:19 +0100, Roger Pau Monne wrote:
> Secondary consoles are processed by libxl with the rest of the
> devices by calling libxl__initiate_device_remove that waits for the
> device to reach state 6 before procceeding with the removal.
> 
> When libxl is destroying the console devices, Qemu is already dead or
> dying, and xenconsoled completely ignores the state backend entry for
> console devices, since it performs the cleanup based on the result of
> reads/writes to the tty.
> 
> Since we don't want to execute hotplug scripts for consoles, leave the
> behaviour as it was previously, and just nuke the backend/frontend
> xenstore entries by calling libxl__device_destroy.
> 
> Report: http://markmail.org/message/yqgppcsdip6tnmh6
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Reported-by: Ian Campbell <ian.campbell@eu.citrix.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> ---
>  tools/libxl/libxl_device.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index a94beab..2120cfb 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -81,6 +81,8 @@ static int libxl__num_devices(libxl__gc *gc, uint32_t domid)
>      for (i = 0; i < num_kinds; i++) {
>          if (libxl__device_kind_from_string(kinds[i], &kind))
>              continue;
> +        if (kind == LIBXL__DEVICE_KIND_CONSOLE)
> +            continue;
>  
>          path = GCSPRINTF("/local/domain/%d/device/%s", domid, kinds[i]);
>          devs = libxl__xs_directory(gc, XBT_NULL, path, &num_devs);
> @@ -512,10 +514,18 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
>              path = libxl__xs_read(gc, XBT_NULL, path);
>              GCNEW(dev);
>              if (path && libxl__parse_backend_path(gc, path, dev) == 0) {
> -                aodev = &aodevs->array[numdev];
>                  dev->domid = domid;
>                  dev->kind = kind;
>                  dev->devid = atoi(devs[j]);
> +                if (dev->backend_kind == LIBXL__DEVICE_KIND_CONSOLE) {
> +                    /* Currently console devices can be destroyed
> +                     * synchronously by just removing xenstore entries,
> +                     * this is what libxl__device_destroy does.
> +                     */
> +                    libxl__device_destroy(gc, dev);
> +                    continue;
> +                }
> +                aodev = &aodevs->array[numdev];

I don't think this is worth a respin but perhaps as a thought for a
future cleanup having the special case extent into both
libxl__num_devices and libxl__devices_destroy is potentially confusing.

Much clearer would be to leave num_devices alone and always numdev++
even when applying the special case.

I'd probably write it as:

if (path && libxl__parse_backend_path(gc, path, dev) == 0) {
    ...
        switch (dev->backend_kind) {
        case LIBXL__DEVICE_KIND_CONSOLE:
        	/* comment */
        	libxl_device_destroy...
        	break;
        default:
        	aodev = ...
                aodev->action = ...
                aodev->dev = dev;
         	...
        	break
        }
        numdev++;
}

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 14/17] libxl: convert libxl_device_vkb_add to an async operation
  2012-07-25 11:19 ` [PATCH v12 14/17] libxl: convert libxl_device_vkb_add to an async operation Roger Pau Monne
@ 2012-07-26 10:41   ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-07-26 10:41 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH v12 14/17] libxl: convert libxl_device_vkb_add to an async operation"):
> Split libxl_device_vkb_add into libxl__device_vkb_add (to be used
> inside already running ao's), and make libxl_device_vkb_add a stub to
> call libxl__device_vkb_add.
> 
> Changes since v11:
> 
>  * libxl__device_vkb_add is no longer an ao operation.
> 
> Changes since v9:
> 
>  * Fixed Ocaml bindings.
> 
>  * Don't abort if num_vkbs > 0.
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 15/17] libxl: convert libxl_device_vfb_add to an async operation
  2012-07-25 11:19 ` [PATCH v12 15/17] libxl: convert libxl_device_vfb_add " Roger Pau Monne
@ 2012-07-26 10:41   ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-07-26 10:41 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH v12 15/17] libxl: convert libxl_device_vfb_add to an async operation"):
> Split libxl_device_vfb_add into libxl__device_vfb_add (to be used
> inside already running ao's), and make libxl_device_vfb_add a stub
> to call libxl__device_vfb_add.

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 01/17] libxl: fix removal of secondary consoles
  2012-07-25 11:19 ` [PATCH v12 01/17] libxl: fix removal of secondary consoles Roger Pau Monne
  2012-07-26 10:39   ` Ian Campbell
@ 2012-07-26 10:43   ` Ian Campbell
  2012-07-26 10:48     ` Ian Campbell
  1 sibling, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-07-26 10:43 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: Ian Jackson, xen-devel

On Wed, 2012-07-25 at 12:19 +0100, Roger Pau Monne wrote:
> Secondary consoles are processed by libxl with the rest of the
> devices by calling libxl__initiate_device_remove that waits for the
> device to reach state 6 before procceeding with the removal.
> 
> When libxl is destroying the console devices, Qemu is already dead or
> dying, and xenconsoled completely ignores the state backend entry for
> console devices, since it performs the cleanup based on the result of
> reads/writes to the tty.
> 
> Since we don't want to execute hotplug scripts for consoles, leave the
> behaviour as it was previously, and just nuke the backend/frontend
> xenstore entries by calling libxl__device_destroy.
> 
> Report: http://markmail.org/message/yqgppcsdip6tnmh6
> 
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Reported-by: Ian Campbell <ian.campbell@eu.citrix.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> ---
>  tools/libxl/libxl_device.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index a94beab..2120cfb 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c

Reading through the entire funciton at the end of libxl__device_destroy
I see:
    if (!numdev) drs->callback(egc, drs, rc);

Does this not wedge up if there are no devices (existing issue) or now
if there is only console devices?

Who calls the callback in this case?

Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 01/17] libxl: fix removal of secondary consoles
  2012-07-26 10:43   ` Ian Campbell
@ 2012-07-26 10:48     ` Ian Campbell
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Campbell @ 2012-07-26 10:48 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: Ian Jackson, xen-devel

On Thu, 2012-07-26 at 11:43 +0100, Ian Campbell wrote:
> On Wed, 2012-07-25 at 12:19 +0100, Roger Pau Monne wrote:
> > Secondary consoles are processed by libxl with the rest of the
> > devices by calling libxl__initiate_device_remove that waits for the
> > device to reach state 6 before procceeding with the removal.
> > 
> > When libxl is destroying the console devices, Qemu is already dead or
> > dying, and xenconsoled completely ignores the state backend entry for
> > console devices, since it performs the cleanup based on the result of
> > reads/writes to the tty.
> > 
> > Since we don't want to execute hotplug scripts for consoles, leave the
> > behaviour as it was previously, and just nuke the backend/frontend
> > xenstore entries by calling libxl__device_destroy.
> > 
> > Report: http://markmail.org/message/yqgppcsdip6tnmh6
> > 
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Reported-by: Ian Campbell <ian.campbell@eu.citrix.com>
> > Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> > ---
> >  tools/libxl/libxl_device.c |   12 +++++++++++-
> >  1 files changed, 11 insertions(+), 1 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> > index a94beab..2120cfb 100644
> > --- a/tools/libxl/libxl_device.c
> > +++ b/tools/libxl/libxl_device.c
> 
> Reading through the entire funciton at the end of libxl__device_destroy
> I see:
>     if (!numdev) drs->callback(egc, drs, rc);
> 
> Does this not wedge up if there are no devices (existing issue) or now
> if there is only console devices?
> 
> Who calls the callback in this case?

Nevermind, I had it backwards -- we call the callback iff we didn't kick
off any async destroys, if we did then the callback happens there via
devices_remove_callbac

Ian.

> 
> Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 02/17] libxl: refactor disk addition to take a helper
  2012-07-25 11:19 ` [PATCH v12 02/17] libxl: refactor disk addition to take a helper Roger Pau Monne
@ 2012-07-26 11:06   ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-07-26 11:06 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH v12 02/17] libxl: refactor disk addition to take a helper"):
> Change libxl__device_disk_add to no longer take a xs transaction and
> instead pass a helper for the local attach case that's used to get the
> free vdev.
...
> +static int device_disk_add(libxl__gc *gc, uint32_t domid,
> +                           libxl_device_disk *disk,
> +                           void *get_vdev(libxl__gc *, void *,
> +                                          xs_transaction_t),
> +                           void *get_vdev_user)

get_vdev needs to return a char*.

The point of the void* is that if another caller of device_disk_add
needs to use this feature, the type of get_vdev_user may need to be
something different for this user; this is why passthrough pointers
like this need to be void*s.

But another user of this feature will have to have their get_vdev
return a char*, since that's how the value is used.

So:
  +static int device_disk_add(libxl__gc *gc, uint32_t domid,
  +                           libxl_device_disk *disk,
  +                           char *get_vdev(libxl__gc *, void *,
  +                                          xs_transaction_t),
  +                           void *get_vdev_user)


> +                                    (void *) libxl__alloc_vdev,

This is not permitted.  The (malicious) compiler might choose to
misoptimise the program as a result.  You must not cast a function
from one type to another.  The actual type of the function must be the
same where it is defined and called.

So either libxl__alloc_vdev needs to change its type to match, or you
need to make a wrapper function which contains the upcast from void*
to const char*.

Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 03/17] libxl: convert libxl__device_disk_local_attach to an async op
  2012-07-25 11:19 ` [PATCH v12 03/17] libxl: convert libxl__device_disk_local_attach to an async op Roger Pau Monne
@ 2012-07-26 11:07   ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-07-26 11:07 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: Stefano Stabellini, xen-devel

Roger Pau Monne writes ("[PATCH v12 03/17] libxl: convert libxl__device_disk_local_attach to an async op"):
> This will be needed in future patches, when libxl__device_disk_add
> becomes async also. Create a new status structure that defines the
> local attach of a disk device and use it in
> libxl__device_disk_local_attach.
> 
> This is done in this patch to split the changes introduced when
> libxl__device_disk_add becomes async.

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v12 10/17] libxl: set correct nic type depending on the guest
  2012-07-25 11:19 ` [PATCH v12 10/17] libxl: set correct nic type depending on the guest Roger Pau Monne
@ 2012-07-26 11:08   ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-07-26 11:08 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH v12 10/17] libxl: set correct nic type depending on the guest"):
> Fix the use of nic type, which results in the following for each type
> of domain:
> 
>  * HVM: let the user choose, if none specified use VIF_IOEMU.
>  * PV: use VIF is none provided, return error if VIF_IOEMU requested.
...
> +    switch (libxl__domain_type(gc, domid)) {
> +    case LIBXL_DOMAIN_TYPE_HVM:
> +        if (!nic->nictype)
> +            nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> +        break;
> +    case LIBXL_DOMAIN_TYPE_PV:
> +        if (nic->nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
> +            LOG(ERROR, "trying to create PV guest with an emulated interface");
> +            return ERROR_INVAL;
> +        }
> +        nic->nictype = LIBXL_NIC_TYPE_VIF;
> +        break;
> +    default:
> +        abort();
> +    }

You are missing
  +    case LIBXL_DOMAIN_TYPE_INVALID:
  +        return ERROR_FAIL;

Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2012-07-26 11:08 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-25 11:19 [PATCH v12 00/17] execute hotplug scripts from libxl Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 01/17] libxl: fix removal of secondary consoles Roger Pau Monne
2012-07-26 10:39   ` Ian Campbell
2012-07-26 10:43   ` Ian Campbell
2012-07-26 10:48     ` Ian Campbell
2012-07-25 11:19 ` [PATCH v12 02/17] libxl: refactor disk addition to take a helper Roger Pau Monne
2012-07-26 11:06   ` Ian Jackson
2012-07-25 11:19 ` [PATCH v12 03/17] libxl: convert libxl__device_disk_local_attach to an async op Roger Pau Monne
2012-07-26 11:07   ` Ian Jackson
2012-07-25 11:19 ` [PATCH v12 04/17] libxl: rename vifs to nics Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 05/17] libxl: convert libxl_device_disk_add to an async op Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 06/17] libxl: convert libxl_device_nic_add to an async operation Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 07/17] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 08/17] libxl: rename _IOEMU nic type to VIF_IOEMU Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 09/17] libxl: set nic type of stub to PV instead of copying from the parent Roger Pau Monne
2012-07-26 10:38   ` Ian Jackson
2012-07-25 11:19 ` [PATCH v12 10/17] libxl: set correct nic type depending on the guest Roger Pau Monne
2012-07-26 11:08   ` Ian Jackson
2012-07-25 11:19 ` [PATCH v12 11/17] libxl: use libxl__xs_path_cleanup on device_destroy Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 12/17] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 13/17] libxl: call hotplug scripts for nic " Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 14/17] libxl: convert libxl_device_vkb_add to an async operation Roger Pau Monne
2012-07-26 10:41   ` Ian Jackson
2012-07-25 11:19 ` [PATCH v12 15/17] libxl: convert libxl_device_vfb_add " Roger Pau Monne
2012-07-26 10:41   ` Ian Jackson
2012-07-25 11:19 ` [PATCH v12 16/17] xl: main_blockdetach don't call destroy if remove succeeds Roger Pau Monne
2012-07-25 11:19 ` [PATCH v12 17/17] libxl: libxl__xs_path_cleanup don't print error if ENOENT Roger Pau Monne
2012-07-26  9:36 ` [PATCH v12 00/17] execute hotplug scripts from libxl Ian Campbell
2012-07-26  9:41   ` Roger Pau Monne
2012-07-26  9:55 ` Ian Campbell

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.