[PATCH v8 06/15] libxl: convert libxl__device_disk_local_attach to an async op

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

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 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            |  100 +++++++++++++++++++++++++++++++---------
 tools/libxl/libxl_bootloader.c |   93 +++++++++++++++++++++++++++++++------
 tools/libxl/libxl_internal.h   |   66 +++++++++++++++++++++------
 3 files changed, 208 insertions(+), 51 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4d832bb..b2a163e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2143,20 +2143,31 @@ 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_init(libxl__disk_local_state *dls, libxl__ao *ao,
+                                   libxl_device_disk *disk,
+                                   const char *blkdev_start,
+                                   libxl__disk_local_state_callback *callback)
 {
-    libxl_ctx *ctx = gc->owner;
+    dls->ao = ao;
+    dls->in_disk = disk;
+    dls->blkdev_start = blkdev_start;
+    dls->callback = callback;
+}
+
+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 *ret = NULL;
     int rc;
     libxl__device device;
     xs_transaction_t t = XBT_NULL;
+    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);
@@ -2192,6 +2203,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             default:
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                            "unrecognized disk format: %d", disk->format);
+                rc = ERROR_FAIL;
                 break;
             }
             break;
@@ -2200,6 +2212,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
                 if (device_disk_add(gc, LIBXL_TOOLSTACK_DOMID, disk,
                                     libxl__alloc_vdev, blkdev_start)) {
                     LOG(ERROR, "libxl_device_disk_add failed");
+                    rc = ERROR_FAIL;
                     goto out;
                 }
                 dev = GCSPRINTF("/dev/%s", disk->vdev);
@@ -2211,6 +2224,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
                 "type: %d", disk->backend);
+            rc = ERROR_FAIL;
             break;
     }
 
@@ -2224,42 +2238,84 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             goto out;
     }
     if (dev != NULL)
-        ret = strdup(dev);
-    return ret;
+        dls->diskpath = strdup(dev);
+
+    dls->callback(egc, dls, 0);
+    return;
 
  out:
+    assert(rc);
     if (t != XBT_NULL)
         xs_transaction_end(ctx->xsh, t, 1);
-    else
-        libxl__device_disk_local_detach(gc, disk);
-    return NULL;
+    dls->callback(egc, dls, rc);
 }
 
-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;
+
+    if (!dls->diskpath) {
+        dls->callback(egc, dls, rc);
+        return;
+    }
 
     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;
+
+                libxl__prepare_ao_device(ao, aodev);
+                aodev->action = DEVICE_DISCONNECT;
+                aodev->dev = device;
+                aodev->callback = local_device_detach_cb;
+                aodev->force = 0;
+                libxl__initiate_device_remove(egc, aodev);
+                return;
             }
-            break;
         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;
+            dls->callback(egc, dls, rc);
+            return;
     }
 
+out:
+    assert(rc);
+    dls->callback(egc, dls, rc);
+    return;
+}
+
+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);
 
-    return 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:
+    dls->callback(egc, dls, aodev->rc);
+    return;
 }
 
 /******************************************************************************/
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 7ebc0df..7ba3502 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,35 @@ 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_finished_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_finished_cb;
+    libxl__device_disk_local_initiate_detach(egc, &bl->dls);
+}
+
+static void bootloader_finished_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);
+
+    free(bl->dls.diskpath);
+    bl->dls.diskpath = 0;
+
+    if (rc) {
+        LOG(ERROR, "unable to detach locally attached disk");
+    }
+
     bl->callback(egc, bl, rc);
 }
 
@@ -275,6 +296,16 @@ 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);
+
+static void bootloader_disk_failed_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 +313,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,11 +374,32 @@ 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;
-        goto out;
+    libxl__device_disk_local_init(&bl->dls, ao, bl->disk, info->blkdev_start,
+                                  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");
+        dls->callback = bootloader_disk_failed_cb;
+        libxl__device_disk_local_initiate_detach(egc, dls);
+        return;
     }
 
     LOG(DEBUG, "Config bootloader value: %s", info->u.pv.bootloader);
@@ -389,11 +440,23 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
 
  out:
     assert(rc);
- out_ok:
-    free(logfile_tmp);
     bootloader_callback(egc, bl, rc);
 }
 
+static void bootloader_disk_failed_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, "failed to detach locally attached disk");
+    }
+
+    bootloader_callback(egc, bl, ERROR_FAIL);
+}
+
 static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
 {
     libxl__bootloader_state *bl = CONTAINER_OF(op, *bl, openpty);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1befdc5..8e256d9 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1293,17 +1293,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 {
@@ -1823,6 +1812,56 @@ 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, Detached.
+ */
+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;
+};
+
+/* Initializes the state of a libxl__disk_local_state to init,
+ * can be called multiple times.
+ * State Undefined -> Idle
+ */
+_hidden void libxl__device_disk_local_init(libxl__disk_local_state *dls,
+                                           libxl__ao *ao,
+                                           libxl_device_disk *disk,
+                                           const char *blkdev_start,
+                                           libxl__disk_local_state_callback
+                                           *callback);
+
+/* Make a disk available in this (the control) domain. Always calls
+ * dls->callback when finished.
+ * State Idle -> Attached
+ */
+_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 -> 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;
@@ -2012,10 +2051,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
@@ -2027,7 +2066,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)