[PATCH 08/13] libxl: convert libxl_device_disk_add to an async operation

[Roger Pau Monne <roger.pau@citrix.com>]

This patch converts libxl_device_disk_add to an ao operation that
waits for device backend to reach state XenbusStateInitWait and then
marks the operation as completed. This is not really useful now, but
will be used by latter patches that will launch hotplug scripts after
we reached the desired xenbus state.

As usual, libxl_device_disk_add callers have been modified, and the
internal function libxl__device_disk_add has been used if the call was
inside an already running ao.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c          |   22 +++++++++-----
 tools/libxl/libxl.h          |    4 ++-
 tools/libxl/libxl_create.c   |   51 ++++++++++++++++++++++++++++----
 tools/libxl/libxl_device.c   |   66 ++++++++++++++++++++++++++++++++++++------
 tools/libxl/libxl_dm.c       |   62 +++++++++++++++++++++++++++++++--------
 tools/libxl/libxl_internal.h |   21 ++++++++++++-
 tools/libxl/xl_cmdimpl.c     |    2 +-
 7 files changed, 187 insertions(+), 41 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index dabe92b..5edfbdc 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1255,15 +1255,19 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
     return rc;
 }
 
-int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
+int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
+                          libxl_device_disk *disk,
+                          const libxl_asyncop_how *ao_how)
 {
-    GC_INIT(ctx);
-    int rc;
+    AO_CREATE(ctx, domid, ao_how);
+    libxl__ao_device *device;
 
-    rc = libxl__device_disk_add(gc, domid, disk);
+    GCNEW(device);
+    libxl__init_ao_device(device, ao, NULL);
+    device->callback = libxl__device_cb;
+    libxl__device_disk_add(egc, domid, disk, device);
 
-    GC_FREE;
-    return rc;
+    return AO_INPROGRESS;
 }
 
 int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
@@ -1508,11 +1512,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
     ret = 0;
 
     libxl_device_disk_remove(ctx, domid, disks + i, 0);
-    libxl_device_disk_add(ctx, domid, disk);
+    /* fixme-ao */
+    libxl_device_disk_add(ctx, domid, disk, 0);
     stubdomid = libxl_get_stubdom_id(ctx, domid);
     if (stubdomid) {
         libxl_device_disk_remove(ctx, stubdomid, disks + i, 0);
-        libxl_device_disk_add(ctx, stubdomid, disk);
+        /* fixme-ao */
+        libxl_device_disk_add(ctx, stubdomid, disk, 0);
     }
 out:
     for (i = 0; i < num; i++)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 2f4694e..f76b2e6 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -663,7 +663,9 @@ void libxl_vminfo_list_free(libxl_vminfo *list, int nr);
  */
 
 /* Disks */
-int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
+int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
+                          libxl_device_disk *disk,
+                          const libxl_asyncop_how *ao_how);
 int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
                              libxl_device_disk *disk,
                              const libxl_asyncop_how *ao_how);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bd8e6a6..9a65c45 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -575,6 +575,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
                                       libxl__bootloader_state *bl,
                                       int rc);
 
+static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
 static void domcreate_console_available(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
 
@@ -705,15 +707,50 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 
     store_libxl_entry(gc, domid, &d_config->b_info);
 
+    GCNEW_ARRAY(dcs->devices, d_config->num_disks);
+    dcs->num_devices = d_config->num_disks;
     for (i = 0; i < d_config->num_disks; i++) {
-        ret = libxl_device_disk_add(ctx, domid, &d_config->disks[i]);
-        if (ret) {
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                       "cannot add disk %d to domain: %d", i, ret);
-            ret = ERROR_FAIL;
-            goto error_out;
-        }
+        libxl__init_ao_device(&dcs->devices[i], ao, &dcs->devices);
+        dcs->devices[i].callback = domcreate_disk_connected;
+        libxl__device_disk_add(egc, domid, &d_config->disks[i],
+                               &dcs->devices[i]);
     }
+    return;
+
+ error_out:
+    assert(ret);
+    domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+    STATE_AO_GC(aorm->ao);
+    libxl__domain_create_state *dcs = CONTAINER_OF(aorm->base, *dcs, devices);
+    int i, last, ret = 0;
+
+    /* convenience aliases */
+    const uint32_t domid = dcs->guest_domid;
+    libxl_domain_config *const d_config = dcs->guest_config;
+    libxl__domain_build_state *const state = &dcs->build_state;
+    libxl_ctx *const ctx = CTX;
+
+    ret = libxl__ao_device_check_last(gc, aorm, dcs->devices,
+                                      dcs->num_devices, &last);
+    if (!last) return;
+    if (last && ret) {
+        LOGE(ERROR, "error connecting disk devices");
+        goto error_out;
+    }
+
+    /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm.
+     * Fill in any field required by either, including both relevant
+     * callbacks (_spawn_stub_dm will overwrite our trespass if needed). */
+    dcs->dmss.dm.spawn.ao = ao;
+    dcs->dmss.dm.guest_config = dcs->guest_config;
+    dcs->dmss.dm.build_state = &dcs->build_state;
+    dcs->dmss.dm.callback = domcreate_devmodel_started;
+    dcs->dmss.callback = domcreate_devmodel_started;
+
     for (i = 0; i < d_config->num_vifs; i++) {
         ret = libxl_device_nic_add(ctx, domid, &d_config->vifs[i]);
         if (ret) {
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 577324c..96e6ec4 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -329,13 +329,15 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
-int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
-                           libxl_device_disk *disk)
+void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
+                            libxl_device_disk *disk,
+                            libxl__ao_device *aorm)
 {
+    STATE_AO_GC(aorm->ao);
     flexarray_t *front;
     flexarray_t *back;
     char *dev, *format;
-    libxl__device device;
+    libxl__device *device;
     int major, minor, rc;
 
     rc = libxl__device_disk_setdefault(gc, disk);
@@ -358,7 +360,8 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
         goto out_free;
     }
 
-    rc = libxl__device_from_disk(gc, domid, disk, &device);
+    GCNEW(device);
+    rc = libxl__device_from_disk(gc, domid, disk, device);
     if (rc != 0) {
         LOG(ERROR, "Invalid or unsupported virtual disk identifier %s",
                    disk->vdev);
@@ -377,7 +380,7 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
             flexarray_append(back, "params");
             flexarray_append(back, dev);
 
-            assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
+            assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
             break;
         case LIBXL_DISK_BACKEND_TAP:
             dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
@@ -395,7 +398,7 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
         case LIBXL_DISK_BACKEND_QDISK:
             flexarray_append(back, "params");
             flexarray_append(back, GCSPRINTF("%s:%s", format, disk->pdev_path));
-            assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
+            assert(device->backend_kind == LIBXL__DEVICE_KIND_QDISK);
             break;
         default:
             LOG(ERROR, "unrecognized disk backend type: %d\n", disk->backend);
@@ -427,21 +430,27 @@ int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
     flexarray_append(front, "state");
     flexarray_append(front, GCSPRINTF("%d", 1));
     flexarray_append(front, "virtual-device");
-    flexarray_append(front, GCSPRINTF("%d", device.devid));
+    flexarray_append(front, GCSPRINTF("%d", device->devid));
     flexarray_append(front, "device-type");
     flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
 
-    libxl__device_generic_add(gc, &device,
+    libxl__device_generic_add(gc, device,
                         libxl__xs_kvs_of_flexarray(gc, back, back->count),
                         libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
+    aorm->dev = device;
+    aorm->action = DEVICE_CONNECT;
+    libxl__initiate_device_add(egc, aorm);
+
     rc = 0;
 
 out_free:
     flexarray_free(back);
     flexarray_free(front);
 out:
-    return rc;
+    aorm->rc = rc;
+    if (aorm->rc) aorm->callback(egc,aorm);
+    return;
 }
 
 int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
@@ -705,6 +714,45 @@ int libxl__ao_device_check_last(libxl__gc *gc, libxl__ao_device *device,
 static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
                                     int rc);
 
+void libxl__initiate_device_add(libxl__egc *egc, libxl__ao_device *aorm)
+{
+    STATE_AO_GC(aorm->ao);
+    char *be_path = libxl__device_backend_path(gc, aorm->dev);
+    char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+    char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+    int rc = 0;
+
+    if (aorm->dev->backend_kind == LIBXL__DEVICE_KIND_QDISK) {
+        aorm->callback(egc, aorm);
+        return;
+    }
+
+    if (atoi(state) == XenbusStateInitWait)
+        goto out_ok;
+
+    libxl__ev_devstate_init(&aorm->ds);
+    rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_backend_callback,
+                                 state_path, XenbusStateInitWait,
+                                 LIBXL_INIT_TIMEOUT * 1000);
+    if (rc) {
+        LOGE(ERROR, "unable to initialize device %s", be_path);
+        goto out_fail;
+    }
+
+    return;
+
+out_fail:
+    assert(rc);
+    aorm->rc = rc;
+    aorm->callback(egc, aorm);
+    return;
+
+out_ok:
+    assert(!rc);
+    aorm->callback(egc, aorm);
+    return;
+}
+
 void libxl__initiate_device_remove(libxl__egc *egc, libxl__ao_device *aorm)
 {
     STATE_AO_GC(aorm->ao);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4019309..2b63a15 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -673,6 +673,8 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
                                 libxl__dm_spawn_state *stubdom_dmss,
                                 int rc);
 
+static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
 static void spaw_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
                                            libxl__destroy_domid_state *dis,
                                            int rc);
@@ -681,8 +683,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 {
     STATE_AO_GC(sdss->dm.spawn.ao);
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    int i, num_console = STUBDOM_SPECIAL_CONSOLES, ret;
-    libxl__device_console *console;
+    int i, ret;
     libxl_device_vfb vfb;
     libxl_device_vkb vkb;
     char **args;
@@ -800,22 +801,60 @@ retry_transaction:
         if (errno == EAGAIN)
             goto retry_transaction;
 
+    GCNEW_ARRAY(sdss->devices, dm_config->num_disks);
+    sdss->num_devices = dm_config->num_disks;
     for (i = 0; i < dm_config->num_disks; i++) {
-        ret = libxl_device_disk_add(ctx, dm_domid, &dm_config->disks[i]);
-        if (ret)
-            goto out_free;
+        libxl__init_ao_device(&sdss->devices[i], ao, &sdss->devices);
+        sdss->devices[i].callback = spawn_stub_disk_connected;
+        libxl__device_disk_add(egc, dm_domid, &dm_config->disks[i],
+                               &sdss->devices[i]);
+    }
+
+    free(args);
+    return;
+
+out_free:
+    free(args);
+out:
+    assert(ret);
+    spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
+}
+
+static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+    STATE_AO_GC(aorm->ao);
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(aorm->base, *sdss, devices);
+    int i, num_console = STUBDOM_SPECIAL_CONSOLES, ret = 0, last;
+    libxl__device_console *console;
+
+    /* convenience aliases */
+    libxl_domain_config *const dm_config = &sdss->dm_config;
+    libxl_domain_config *const guest_config = sdss->dm.guest_config;
+    const int guest_domid = sdss->dm.guest_domid;
+    libxl__domain_build_state *const d_state = sdss->dm.build_state;
+    libxl__domain_build_state *const stubdom_state = &sdss->dm_state;
+    uint32_t dm_domid = sdss->pvqemu.guest_domid;
+
+    ret = libxl__ao_device_check_last(gc, aorm, sdss->devices,
+                                      sdss->num_devices, &last);
+    if (!last) return;
+    if (last && ret) {
+        LOGE(ERROR, "error connecting disk devices");
+        goto out;
     }
+
     for (i = 0; i < dm_config->num_vifs; i++) {
         ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->vifs[i]);
         if (ret)
-            goto out_free;
+            goto out;
     }
     ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
     if (ret)
-        goto out_free;
+        goto out;
     ret = libxl_device_vkb_add(ctx, dm_domid, &dm_config->vkbs[0]);
     if (ret)
-        goto out_free;
+        goto out;
 
     if (guest_config->b_info.u.hvm.serial)
         num_console++;
@@ -823,7 +862,7 @@ retry_transaction:
     console = libxl__calloc(gc, num_console, sizeof(libxl__device_console));
     if (!console) {
         ret = ERROR_NOMEM;
-        goto out_free;
+        goto out;
     }
 
     for (i = 0; i < num_console; i++) {
@@ -859,7 +898,7 @@ retry_transaction:
         ret = libxl__device_console_add(gc, dm_domid, &console[i],
                         i == STUBDOM_CONSOLE_LOGGING ? stubdom_state : NULL);
         if (ret)
-            goto out_free;
+            goto out;
     }
 
     sdss->pvqemu.guest_domid = dm_domid;
@@ -869,11 +908,8 @@ retry_transaction:
 
     libxl__spawn_local_dm(egc, &sdss->pvqemu);
 
-    free(args);
     return;
 
-out_free:
-    free(args);
 out:
     assert(ret);
     spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e324da2..2500b86 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -70,6 +70,7 @@
 #include "_libxl_types_internal.h"
 #include "_libxl_types_internal_json.h"
 
+#define LIBXL_INIT_TIMEOUT 10
 #define LIBXL_DESTROY_TIMEOUT 10
 #define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
 #define LIBXL_XENCONSOLE_LIMIT 1048576
@@ -783,8 +784,6 @@ _hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*);
 _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
                                     libxl_device_disk *disk,
                                     libxl__device *device);
-_hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
-                                   libxl_device_disk *disk);
 
 _hidden int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
                                    libxl_device_nic *nic,
@@ -1817,6 +1816,18 @@ struct libxl__ao_device {
     void *base;
 };
 
+/* Internal AO operation to connect a disk device */
+_hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
+                                    libxl_device_disk *disk,
+                                    libxl__ao_device *aorm);
+
+/* Arranges that dev will be added to the guest, and the
+ * hotplug scripts will be executed (if necessary). When
+ * this is done (or an error happens), the callback in
+ * aorm->callback will be called.
+ */
+_hidden void libxl__initiate_device_add(libxl__egc*, libxl__ao_device *aorm);
+
 /* Arranges that dev will be removed to the guest, and the
  * hotplug scripts will be executed (if necessary). When
  * this is done (or an error happens), the callback in
@@ -1941,6 +1952,9 @@ typedef struct {
     libxl__domain_build_state dm_state;
     libxl__dm_spawn_state pvqemu;
     libxl__destroy_domid_state dis;
+    /* used to store the state of devices being connected */
+    libxl__ao_device *devices;
+    int num_devices;
 } libxl__stub_dm_spawn_state;
 
 _hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
@@ -1969,6 +1983,9 @@ struct libxl__domain_create_state {
          * for the non-stubdom device model. */
     /* necessary if the domain creation failed and we have to destroy it */
     libxl__domain_destroy_state dds;
+    /* used to store the state of devices being connected */
+    libxl__ao_device *devices;
+    int num_devices;
 };
 
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3c02b69..69ce45a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5081,7 +5081,7 @@ int main_blockattach(int argc, char **argv)
         return 0;
     }
 
-    if (libxl_device_disk_add(ctx, fe_domid, &disk)) {
+    if (libxl_device_disk_add(ctx, fe_domid, &disk, 0)) {
         fprintf(stderr, "libxl_device_disk_add failed.\n");
     }
     return 0;
-- 
1.7.7.5 (Apple Git-26)