[PATCH RFC 03/10] libxl: add new "method" parameter to xl disk config

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

This new diskspec parameters will set script to the value passed in
method, and hotplug_version to 1.

This patch adds the basic support to handle this new scripts, by
adding the prepare/unprepare private libxl functions, and modifying
the current domain creation/destruction to call this functions at the
appropriate spots.

This patch also adds some helpers, that will be used here and in later
patches.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/libxl/libxl.c          |  131 +++++++++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl_create.c   |   62 +++++++++++++++++++-
 tools/libxl/libxl_device.c   |   80 +++++++++++++++++++------
 tools/libxl/libxl_internal.h |   34 +++++++++++
 tools/libxl/libxl_types.idl  |    1 +
 tools/libxl/libxlu_disk_l.l  |    2 +
 6 files changed, 288 insertions(+), 22 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 8d921bc..e141379 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1999,6 +1999,108 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
+void libxl__device_disk_prepare(libxl__egc *egc, uint32_t domid,
+                                libxl_device_disk *disk,
+                                libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    char *hotplug_path;
+    char *script_path;
+    int rc;
+    xs_transaction_t t = XBT_NULL;
+
+    if (disk->hotplug_version == 0) {
+        aodev->callback(egc, aodev);
+        return;
+    }
+
+    GCNEW(aodev->dev);
+    rc = libxl__device_from_disk(gc, domid, disk, aodev->dev);
+    if (rc != 0) {
+        LOG(ERROR, "Invalid or unsupported virtual disk identifier %s",
+                   disk->vdev);
+        goto error;
+    }
+
+    script_path = libxl__abs_path(gc, disk->script,
+                                  libxl__xen_script_dir_path());
+
+    hotplug_path = libxl__device_xs_hotplug_path(gc, aodev->dev);
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto error;
+
+        rc = libxl__xs_write_checked(gc, t,
+                                 GCSPRINTF("%s/params", hotplug_path),
+                                 disk->pdev_path);
+        if (rc)
+            goto error;
+
+        rc = libxl__xs_write_checked(gc, t,
+                                 GCSPRINTF("%s/script", hotplug_path),
+                                 script_path);
+        if (rc)
+            goto error;
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto error;
+    }
+
+    aodev->action = DEVICE_PREPARE;
+    aodev->hotplug_version = disk->hotplug_version;
+    libxl__device_hotplug(egc, aodev);
+    return;
+
+error:
+    assert(rc);
+    libxl__xs_transaction_abort(gc, &t);
+    aodev->rc = rc;
+    aodev->callback(egc, aodev);
+    return;
+}
+
+void libxl__device_disk_unprepare(libxl__egc *egc, uint32_t domid,
+                                  libxl_device_disk *disk,
+                                  libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    char *hotplug_path;
+    int rc;
+
+    if (disk->hotplug_version == 0) {
+        aodev->callback(egc, aodev);
+        return;
+    }
+
+    GCNEW(aodev->dev);
+    rc = libxl__device_from_disk(gc, domid, disk, aodev->dev);
+    if (rc != 0) {
+        LOG(ERROR, "Invalid or unsupported virtual disk identifier %s",
+                   disk->vdev);
+        goto error;
+    }
+
+    hotplug_path = libxl__device_xs_hotplug_path(gc, aodev->dev);
+    if (!libxl__xs_read(gc, XBT_NULL, hotplug_path)) {
+        LOG(DEBUG, "unable to unprepare device because %s doesn't exist",
+                   hotplug_path);
+        aodev->callback(egc, aodev);
+        return;
+    }
+
+    aodev->action = DEVICE_UNPREPARE;
+    aodev->hotplug_version = disk->hotplug_version;
+    libxl__device_hotplug(egc, aodev);
+    return;
+
+error:
+    assert(rc);
+    aodev->rc = rc;
+    aodev->callback(egc, aodev);
+    return;
+}
+
 /* Specific function called directly only by local disk attach,
  * all other users should instead use the regular
  * libxl__device_disk_add wrapper
@@ -2058,8 +2160,15 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
                 dev = disk->pdev_path;
 
         do_backend_phy:
-                flexarray_append(back, "params");
-                flexarray_append(back, dev);
+                if (disk->hotplug_version == 0) {
+                    /*
+                     * If the new hotplug version is used params is
+                     * stored under a private path, since it can contain
+                     * data that the guest should not see.
+                     */
+                    flexarray_append(back, "params");
+                    flexarray_append(back, dev);
+                }
 
                 script = libxl__abs_path(gc, disk->script?: "block",
                                          libxl__xen_script_dir_path());
@@ -2152,6 +2261,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
 
     aodev->dev = device;
     aodev->action = DEVICE_CONNECT;
+    aodev->hotplug_version = disk->hotplug_version;
     libxl__wait_device_connection(egc, aodev);
 
     rc = 0;
@@ -2245,6 +2355,7 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid,
     GC_INIT(ctx);
     char *dompath, *path;
     int devid = libxl__device_disk_dev_number(vdev, NULL, NULL);
+    libxl__device dev;
     int rc = ERROR_FAIL;
 
     if (devid < 0)
@@ -2263,6 +2374,22 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid,
         goto out;
 
     rc = libxl__device_disk_from_xs_be(gc, path, disk);
+    if (rc) {
+        LOG(ERROR, "unable to parse disk device from path %s", path);
+        goto out;
+    }
+
+    /* Check if the device is using the new hotplug interface */
+    rc = libxl__device_from_disk(gc, domid, disk, &dev);
+    if (rc) {
+        LOG(ERROR, "invalid or unsupported virtual disk identifier %s",
+                    disk->vdev);
+        goto out;
+    }
+    path = libxl__device_xs_hotplug_path(gc, &dev);
+    if (libxl__xs_read(gc, XBT_NULL, path))
+        disk->hotplug_version = 1;
+
 out:
     GC_FREE;
     return rc;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9d20086..c176967 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -591,6 +591,10 @@ static int store_libxl_entry(libxl__gc *gc, uint32_t domid,
  */
 
 /* Event callbacks, in this order: */
+
+static void domcreate_launch_bootloader(libxl__egc *egc,
+                                        libxl__multidev *multidev,
+                                        int ret);
 static void domcreate_devmodel_started(libxl__egc *egc,
                                        libxl__dm_spawn_state *dmss,
                                        int rc);
@@ -627,6 +631,12 @@ static void domcreate_destruction_cb(libxl__egc *egc,
                                      libxl__domain_destroy_state *dds,
                                      int rc);
 
+/* If creation is not successful, this callback will be executed
+ * when devices have been unprepared */
+static void domcreate_unprepare_cb(libxl__egc *egc,
+                                   libxl__multidev *multidev,
+                                   int ret);
+
 static void initiate_domain_create(libxl__egc *egc,
                                    libxl__domain_create_state *dcs)
 {
@@ -637,7 +647,6 @@ static void initiate_domain_create(libxl__egc *egc,
 
     /* convenience aliases */
     libxl_domain_config *const d_config = dcs->guest_config;
-    const int restore_fd = dcs->restore_fd;
     memset(&dcs->build_state, 0, sizeof(dcs->build_state));
 
     domid = 0;
@@ -670,6 +679,33 @@ static void initiate_domain_create(libxl__egc *egc,
         if (ret) goto error_out;
     }
 
+    libxl__multidev_begin(ao, &dcs->multidev);
+    dcs->multidev.callback = domcreate_launch_bootloader;
+    libxl__prepare_disks(egc, ao, domid, d_config, &dcs->multidev);
+    libxl__multidev_prepared(egc, &dcs->multidev, 0);
+    return;
+
+error_out:
+    assert(ret);
+    domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_launch_bootloader(libxl__egc *egc,
+                                        libxl__multidev *multidev,
+                                        int ret)
+{
+    libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
+    STATE_AO_GC(dcs->ao);
+
+    /* convenience aliases */
+    const int restore_fd = dcs->restore_fd;
+    libxl_domain_config *const d_config = dcs->guest_config;
+
+    if (ret) {
+        LOG(ERROR, "unable to prepare devices");
+        goto error_out;
+    }
+
     dcs->bl.ao = ao;
     libxl_device_disk *bootdisk =
         d_config->num_disks > 0 ? &d_config->disks[0] : NULL;
@@ -1202,11 +1238,35 @@ static void domcreate_destruction_cb(libxl__egc *egc,
 {
     STATE_AO_GC(dds->ao);
     libxl__domain_create_state *dcs = CONTAINER_OF(dds, *dcs, dds);
+    uint32_t domid = dcs->guest_domid;
+    libxl_domain_config *const d_config = dcs->guest_config;
 
     if (rc)
         LOG(ERROR, "unable to destroy domain %u following failed creation",
                    dds->domid);
 
+    /*
+     * We might have devices that have been prepared, but with no
+     * frontend xenstore entries, so domain destruction fails to
+     * find them, that is why we have to unprepare them manually.
+     */
+    libxl__multidev_begin(ao, &dcs->multidev);
+    dcs->multidev.callback = domcreate_unprepare_cb;
+    libxl__unprepare_disks(egc, ao, domid, d_config, &dcs->multidev);
+    libxl__multidev_prepared(egc, &dcs->multidev, 0);
+    return;
+}
+
+static void domcreate_unprepare_cb(libxl__egc *egc,
+                                   libxl__multidev *multidev,
+                                   int ret)
+{
+    libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
+    STATE_AO_GC(dcs->ao);
+
+    if (ret)
+        LOG(ERROR, "unable to unprepare devices");
+
     dcs->callback(egc, dcs, ERROR_FAIL, dcs->guest_domid);
 }
 
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 85c9953..ce99d99 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -521,18 +521,21 @@ void libxl__multidev_prepared(libxl__egc *egc,
 
 /******************************************************************************/
 
-/* Macro for defining the functions that will add a bunch of disks when
+/* Macro for defining the functions that will operate a bunch of devices when
  * inside an async op with multidev.
  * This macro is added to prevent repetition of code.
  *
  * The following functions are defined:
+ * libxl__prepare_disks
+ * libxl__unprepare_disks
  * libxl__add_disks
  * libxl__add_nics
  * libxl__add_vtpms
  */
 
-#define DEFINE_DEVICES_ADD(type)                                        \
-    void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid, \
+#define DEFINE_DEVICES_FUNC(type, op)                                   \
+    void libxl__##op##_##type##s(libxl__egc *egc, libxl__ao *ao,        \
+                              uint32_t domid,                           \
                               libxl_domain_config *d_config,            \
                               libxl__multidev *multidev)                \
     {                                                                   \
@@ -540,16 +543,19 @@ void libxl__multidev_prepared(libxl__egc *egc,
         int i;                                                          \
         for (i = 0; i < d_config->num_##type##s; i++) {                 \
             libxl__ao_device *aodev = libxl__multidev_prepare(multidev);  \
-            libxl__device_##type##_add(egc, domid, &d_config->type##s[i], \
+            libxl__device_##type##_##op(egc, domid, &d_config->type##s[i], \
                                        aodev);                          \
         }                                                               \
     }
 
-DEFINE_DEVICES_ADD(disk)
-DEFINE_DEVICES_ADD(nic)
-DEFINE_DEVICES_ADD(vtpm)
+DEFINE_DEVICES_FUNC(disk, add)
+DEFINE_DEVICES_FUNC(nic, add)
+DEFINE_DEVICES_FUNC(vtpm, add)
 
-#undef DEFINE_DEVICES_ADD
+DEFINE_DEVICES_FUNC(disk, prepare)
+DEFINE_DEVICES_FUNC(disk, unprepare)
+
+#undef DEFINE_DEVICES_FUNC
 
 /******************************************************************************/
 
@@ -557,6 +563,7 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
 {
     const char *be_path = libxl__device_backend_path(gc, dev);
     const char *fe_path = libxl__device_frontend_path(gc, dev);
+    const char *hotplug_path = libxl__device_xs_hotplug_path(gc, dev);
     const char *tapdisk_path = GCSPRINTF("%s/%s", be_path, "tapdisk-params");
     const char *tapdisk_params;
     xs_transaction_t t = 0;
@@ -572,6 +579,7 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
 
         libxl__xs_path_cleanup(gc, t, fe_path);
         libxl__xs_path_cleanup(gc, t, be_path);
+        libxl__xs_path_cleanup(gc, t, hotplug_path);
 
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;
@@ -644,6 +652,14 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
                     continue;
                 }
                 aodev = libxl__multidev_prepare(multidev);
+                /*
+                 * Check if the device has hotplug entries in xenstore,
+                 * which would mean it's using the new hotplug calling
+                 * convention.
+                 */
+                path = libxl__device_xs_hotplug_path(gc, dev);
+                if (libxl__xs_read(gc, XBT_NULL, path))
+                    aodev->hotplug_version = 1;
                 aodev->action = DEVICE_DISCONNECT;
                 aodev->dev = dev;
                 aodev->force = drs->force;
@@ -696,8 +712,6 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
 static void device_backend_cleanup(libxl__gc *gc,
                                    libxl__ao_device *aodev);
 
-static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev);
-
 static void device_hotplug_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
                                       const struct timeval *requested_abs);
 
@@ -732,7 +746,7 @@ void libxl__wait_device_connection(libxl__egc *egc, libxl__ao_device *aodev)
          * If Qemu is running, it will set the state of the device to
          * 4 directly, without waiting in state 2 for any hotplug execution.
          */
-        device_hotplug(egc, aodev);
+        libxl__device_hotplug(egc, aodev);
         return;
     }
 
@@ -864,7 +878,7 @@ static void device_qemu_timeout(libxl__egc *egc, libxl__ev_time *ev,
     rc = libxl__xs_write_checked(gc, XBT_NULL, state_path, "6");
     if (rc) goto out;
 
-    device_hotplug(egc, aodev);
+    libxl__device_hotplug(egc, aodev);
     return;
 
 out:
@@ -893,7 +907,7 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
         goto out;
     }
 
-    device_hotplug(egc, aodev);
+    libxl__device_hotplug(egc, aodev);
     return;
 
 out:
@@ -908,7 +922,7 @@ static void device_backend_cleanup(libxl__gc *gc, libxl__ao_device *aodev)
     libxl__ev_devstate_cancel(gc, &aodev->backend_ds);
 }
 
-static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
+void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
     char *be_path = libxl__device_backend_path(gc, aodev->dev);
@@ -1012,10 +1026,13 @@ static void device_hotplug_child_death_cb(libxl__egc *egc,
         if (hotplug_error)
             LOG(ERROR, "script: %s", hotplug_error);
         aodev->rc = ERROR_FAIL;
-        if (aodev->action == DEVICE_CONNECT)
+        if (aodev->action == DEVICE_CONNECT ||
+            aodev->action == DEVICE_PREPARE ||
+            aodev->action == DEVICE_LOCALATTACH)
             /*
-             * Only fail on device connection, on disconnection
-             * ignore error, and continue with the remove process
+             * Only fail on device connection, prepare or localattach,
+             * on other cases ignore error, and continue with the remove
+             * process.
              */
              goto error;
     }
@@ -1025,7 +1042,7 @@ static void device_hotplug_child_death_cb(libxl__egc *egc,
      * device_hotplug_done breaking the loop.
      */
     aodev->num_exec++;
-    device_hotplug(egc, aodev);
+    libxl__device_hotplug(egc, aodev);
 
     return;
 
@@ -1041,8 +1058,33 @@ static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev)
 
     device_hotplug_clean(gc, aodev);
 
-    /* Clean xenstore if it's a disconnection */
     if (aodev->action == DEVICE_DISCONNECT) {
+        switch (aodev->hotplug_version) {
+        case 0:
+            /* Clean xenstore if it's a disconnection */
+            rc = libxl__device_destroy(gc, aodev->dev);
+            if (!aodev->rc)
+                aodev->rc = rc;
+            break;
+        case 1:
+            /*
+             * Chain unprepare hotplug execution
+             * after disconnection of device.
+             */
+            aodev->num_exec = 0;
+            aodev->action = DEVICE_UNPREPARE;
+            libxl__device_hotplug(egc, aodev);
+            return;
+        default:
+            LOG(ERROR, "unknown hotplug script version (%d)",
+                       aodev->hotplug_version);
+            if (!aodev->rc)
+                aodev->rc = ERROR_FAIL;
+            break;
+        }
+    }
+    /* Clean hotplug xenstore path */
+    if (aodev->action == DEVICE_UNPREPARE) {
         rc = libxl__device_destroy(gc, aodev->dev);
         if (!aodev->rc)
             aodev->rc = rc;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c41e608..16907ff 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2018,6 +2018,12 @@ struct libxl__multidev {
 _hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
                                     libxl_device_disk *disk,
                                     libxl__ao_device *aodev);
+_hidden void libxl__device_disk_prepare(libxl__egc *egc, uint32_t domid,
+                                        libxl_device_disk *disk,
+                                        libxl__ao_device *aodev);
+_hidden void libxl__device_disk_unprepare(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,
@@ -2094,6 +2100,19 @@ _hidden int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                            libxl__device_action action,
                                            int num_exec, int hotplug_version);
 
+/*
+ * libxl__device_hotplug runs the hotplug script associated
+ * with the device passed in aodev->dev.
+ *
+ * The libxl__ao_device passed to this function should be
+ * prepared using libxl__prepare_ao_device prior to calling
+ * this function.
+ *
+ * Once finished, aodev->callback will be executed.
+ */
+_hidden void libxl__device_hotplug(libxl__egc *egc,
+                                   libxl__ao_device *aodev);
+
 /*----- local disk attach: attach a disk locally to run the bootloader -----*/
 
 typedef struct libxl__disk_local_state libxl__disk_local_state;
@@ -2467,6 +2486,21 @@ _hidden void libxl__add_disks(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
                               libxl_domain_config *d_config,
                               libxl__multidev *multidev);
 
+_hidden void libxl__prepare_disks(libxl__egc *egc, libxl__ao *ao,
+                                  uint32_t domid,
+                                  libxl_domain_config *d_config,
+                                  libxl__multidev *multidev);
+
+_hidden void libxl__prepare_undisks(libxl__egc *egc, libxl__ao *ao,
+                                    uint32_t domid,
+                                    libxl_domain_config *d_config,
+                                    libxl__multidev *multidev);
+
+_hidden void libxl__unprepare_disks(libxl__egc *egc, libxl__ao *ao,
+                                    uint32_t domid,
+                                    libxl_domain_config *d_config,
+                                    libxl__multidev *multidev);
+
 _hidden void libxl__add_nics(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
                              libxl_domain_config *d_config,
                              libxl__multidev *multidev);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 7eac4a8..272ff54 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -366,6 +366,7 @@ libxl_device_disk = Struct("device_disk", [
     ("removable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),
+    ("hotplug_version", integer),
     ])
 
 libxl_device_nic = Struct("device_nic", [
diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
index bee16a1..4486c1a 100644
--- a/tools/libxl/libxlu_disk_l.l
+++ b/tools/libxl/libxlu_disk_l.l
@@ -172,6 +172,8 @@ backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }
 
 vdev=[^,]*,?	{ STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
 script=[^,]*,?	{ STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
+method=[^,]*,?	{ STRIP(','); SAVESTRING("script", script, FROMEQUALS);
+		  DPC->disk->hotplug_version = 1; }
 
  /* the target magic parameter, eats the rest of the string */
 
-- 
1.7.7.5 (Apple Git-26)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel