diff mbox series

[v2,3/3] libxl: Enable stubdom cdrom changing

Message ID 20240201214004.238858-4-jandryuk@gmail.com (mailing list archive)
State New, archived
Headers show
Series libxl: Stubdom cd-rom changing support | expand

Commit Message

Jason Andryuk Feb. 1, 2024, 9:40 p.m. UTC 
To change the cd-rom medium, libxl will:
 - QMP eject the medium from QEMU
 - block-detach the old PV disk
 - block-attach the new PV disk
 - QMP change the medium to the new PV disk by fdset-id

The QMP code is reused, and remove and attach are implemented here.

The stubdom must internally handle adding /dev/xvdc to the appropriate
fdset.  libxl in dom0 doesn't see the result of adding to the fdset as
that is internal to the stubdom, so a delay and retries are added to
around calling cdrom_insert_addfd_cb().

For cd-eject, we still need to attach the empty vbd.  This is necessary
since xenstore is used to determine that hdc exists.  Otherwise after
eject, hdc would be gone and the cd-insert would fail to find the drive
to insert new media.

The ERROR_JSON_CONFIG_EMPTY check in cdrom_insert_inserted() is because
a stubdom don't have a json config.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
v2:
Only allow for Linux stubdoms (Marek)
Fix cd-eject
Fix errant hard tabs
Move the debug print and into the special case.
---
 docs/misc/stubdom.txt         |  16 ++
 tools/libs/light/libxl_disk.c | 298 +++++++++++++++++++++++++++++++---
 2 files changed, 288 insertions(+), 26 deletions(-)

Comments

Anthony PERARD Feb. 13, 2024, 11:35 a.m. UTC | #1 
On Thu, Feb 01, 2024 at 04:40:04PM -0500, Jason Andryuk wrote:
> To change the cd-rom medium, libxl will:
>  - QMP eject the medium from QEMU
>  - block-detach the old PV disk
>  - block-attach the new PV disk
>  - QMP change the medium to the new PV disk by fdset-id
> 
> The QMP code is reused, and remove and attach are implemented here.
> 
> The stubdom must internally handle adding /dev/xvdc to the appropriate
> fdset.  libxl in dom0 doesn't see the result of adding to the fdset as
> that is internal to the stubdom, so a delay and retries are added to
> around calling cdrom_insert_addfd_cb().

Why do you use "fdset" in linux stubdom, can't QEMU open /dev/xvdc
directly? And so why don't libxl tell QEMU to open /dev/xvdc?

I think the only reason I've implemented cd-insert via fdset in libxl
was because that was the only way to have libxl open a file on behalf of
QEMU when it's deprivileged and don't have access to any file. But that
doesn't mean we need to use an fdset with the stubdom.


And BTW, there's a "query-fdsets" command that could be used to findout
when and fdset is available, there's even an "opaque" field that could
be used to communicate which fdset to use instead of needing to rely on
a static fdset-id.

> For cd-eject, we still need to attach the empty vbd.  This is necessary
> since xenstore is used to determine that hdc exists.  Otherwise after
> eject, hdc would be gone and the cd-insert would fail to find the drive
> to insert new media.
> 
> The ERROR_JSON_CONFIG_EMPTY check in cdrom_insert_inserted() is because
> a stubdom don't have a json config.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
> diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
> index 09082ffb58..6354982c05 100644
> --- a/tools/libs/light/libxl_disk.c
> +++ b/tools/libs/light/libxl_disk.c
> @@ -905,6 +938,18 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
>          disk->format = LIBXL_DISK_FORMAT_EMPTY;
>      }
>  
> +#define STUBDOM_FDSET_CD 8000
> +    if (strncmp(disk->vdev, "hd", 2) == 0) {
> +        cis->stubdom_fdset = STUBDOM_FDSET_CD + disk->vdev[2] - 'a';
> +    } else if (strncmp(disk->vdev, "xvd", 3) == 0) {
> +        cis->stubdom_fdset = STUBDOM_FDSET_CD + disk->vdev[3] - 'a';

Currently, cd-rom eject/insert also works if one use "sdc", so we
probably want to check for "sd".

Also, you could use device_virtdisk_matches() like
libxl__device_disk_dev_number(). I'm not sure if having vdev been
something like "xvdaa" is going to work well, and that case isn't going
to produce the expected fdset. (Without the patch, "xvdaa" produce a
warning when trying to generate QEMU's command line, so maybe it's fine
to not handle this case.)

> +    } else {
> +        LOGD(ERROR, cis->domid, "disk->vdev \"%s\" isn't hdX or xvdY",
> +             disk->vdev);
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
>  out:
>      libxl__device_list_free(&libxl__disk_devtype, disks, num);
>      if (rc) {
> @@ -923,6 +968,7 @@ static void cdrom_insert_lock_acquired(libxl__egc *egc,
>      libxl__cdrom_insert_state *cis = CONTAINER_OF(lock, *cis, qmp_lock);
>      STATE_AO_GC(cis->ao);
>  
> +    LOGD(DEBUG, cis->domid, "rc=%d", rc);

Left over debug logging? The new one in cdrom_insert_done() might be
enough? Also, I think libxl__ao_complete() does log this rc value as
well.

>      if (rc) goto out;
>  
>      rc = libxl__ev_time_register_rel(ao, &cis->time,


> +static void cdrom_insert_stubdom_ejected(libxl__egc *egc, libxl__ev_qmp *qmp,
> +                                         const libxl__json_object *response,
> +                                         int rc)
> +{
> +    libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
> +    libxl__device *device;
> +    STATE_AO_GC(cis->ao);
> +    domid_t stubdomid = libxl_get_stubdom_id(CTX, cis->domid);

cis->disk_domid?

> +
> +    LOGD(DEBUG, cis->domid, "rc=%d", rc);
> +    /* cis->stubdom_fdset is initially empty, so remove-fd fails the first
> +     * call with:
> +     * {"error": {"class": "GenericError",
> +     *            "desc": "File descriptor named 'fdset-id:8675' not found"}}
> +     * Carry on in that case. */
> +    if (rc && rc != ERROR_QMP_GENERIC_ERROR) goto out;
> +
> +    GCNEW(device);
> +    rc = libxl__device_from_disk(gc, stubdomid, cis->disk, device);
> +    if (rc != 0) goto out;

If (rc) is more common without "!= 0".


> +
> +    /* block dev eject */
> +    /* Below is basically an open coding of:
> +     * libxl_device_disk__remove(CTX, cis->domid, cis->disk, 0);

One too many '_'?

> +     * ...since we can't call it from within libxl.

Well, not exactly, we also need to set our own callback and own struct
here.

> +     */
> +    libxl__prepare_ao_device(ao, &cis->aodev_del);
> +    cis->aodev_del.action = LIBXL__DEVICE_ACTION_REMOVE;
> +    cis->aodev_del.dev = device;
> +    cis->aodev_del.callback = cdrom_insert_stubdom_disk_ejected_aocomplete;
> +    cis->aodev_del.force.flag = LIBXL__FORCE_OFF;
> +    libxl__initiate_device_generic_remove(egc, &cis->aodev_del);

It doesn't looks like you need an extra `aodev_del` field, and you can
probably reuse the same `aodev` field twice.

> +    return;
> +
> + out:
> +    cdrom_insert_done(egc, cis, rc); /* must be last */
> +}
> +
> +static void cdrom_insert_stubdom_disk_ejected_aocomplete(libxl__egc *egc,
> +                                                         libxl__ao_device *aodev)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    libxl__cdrom_insert_state *cis = CONTAINER_OF(aodev, *cis, aodev_del);
> +
> +    LOGD(DEBUG, cis->domid, "rc=%d", aodev->rc);
> +    if (aodev->rc) {
> +        if (aodev->dev) {

I'm pretty sure "aodev->dev" should be set, so this if doesn't seems
needed.

> +            LOGD(ERROR, aodev->dev->domid, "Unable to %s %s with id %u",
> +                        libxl__device_action_to_string(aodev->action),

We already know the action is "remove".

> +                        libxl__device_kind_to_string(aodev->dev->kind),
> +                        aodev->dev->devid);

This whole message could be personalised to what we are trying to
achieve, that is, remove the cd-rom from the stubdomain.

> +        } else {
> +            LOG(ERROR, "unable to %s device",
> +                       libxl__device_action_to_string(aodev->action));
> +        }
> +        goto out;
> +    }
> +
> +    cdrom_insert_stubdom_disk_ejected(egc, &cis->qmp, NULL, aodev->rc);
> +    return;
> +
> + out:
> +    cdrom_insert_done(egc, cis, aodev->rc);
> +}
> +
> +static void cdrom_insert_stubdom_disk_ejected(libxl__egc *egc,
> +                                              libxl__ev_qmp *qmp,
> +                                              const libxl__json_object *response,
> +                                              int rc)

This functions prototype looks like a ev_qmp callback but it's never
used like that. Also, it could be merged into the previous function.


>  static void cdrom_insert_inserted(libxl__egc *egc,
>                                    libxl__ev_qmp *qmp,
>                                    const libxl__json_object *response,
>                                    int rc)
>  {
> -    EGC_GC;
>      libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
> +    STATE_AO_GC(cis->ao);

Why do you replace EGC_GC by AO_GC ? Is there an allocation that need to
last longer in the current function? If it's just to have "ao" in the
current scope, you can use "cis->ao" below.

>      libxl__flock *data_lock = NULL;
>      libxl_domain_config d_config;
>      flexarray_t *insert = NULL;
> @@ -1171,9 +1401,22 @@ static void cdrom_insert_inserted(libxl__egc *egc,
>  
>      libxl_domain_config_init(&d_config);
>  
> -    if (rc) goto out;
> +    LOGD(DEBUG, cis->domid, "rc=%d", rc);
> +
> +    if (rc) {
> +        if (cis->retries++ < 10 ) {
> +            LOGD(DEBUG, qmp->domid, "Retrying QMP cdrom change\n");
> +            rc = libxl__ev_time_register_rel(ao, &cis->timeout_retry,
> +                                             cdrom_insert_addfd_retry, 100);
> +            if (rc) goto out;
>  
> -    rc = libxl__device_from_disk(gc, domid, disk, &device);
> +            return;
> +        } else {
> +            goto out;
> +        }
> +    }
> +
> +    rc = libxl__device_from_disk(gc, cis->disk_domid, disk, &device);
>      if (rc) goto out;
>      be_path = libxl__device_backend_path(gc, &device);
>      libxl_path = libxl__device_libxl_path(gc, &device);
> @@ -1257,7 +1500,10 @@ static void cdrom_insert_done(libxl__egc *egc,
>  {
>      EGC_GC;
>  
> +    LOGD(DEBUG, cis->domid, "rc=%d", rc);

I don't think this is useful because libxl__ao_complete() will print rc
as well.

>      libxl__ev_time_deregister(gc, &cis->time);
> +    libxl__ev_time_deregister(gc, &cis->timeout_retry);
>      libxl__ev_qmp_dispose(gc, &cis->qmp);
>      if (cis->qmp.payload_fd >= 0) close(cis->qmp.payload_fd);
>      libxl__ev_slowlock_unlock(gc, &cis->qmp_lock);
diff mbox series

Patch

diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
index c717a95d17..1b2380ae8f 100644
--- a/docs/misc/stubdom.txt
+++ b/docs/misc/stubdom.txt
@@ -127,6 +127,22 @@  Limitations:
  - at most 26 emulated disks are supported (more are still available as PV disks)
  - graphics output (VNC/SDL/Spice) not supported
 
+CD-ROM changing:
+
+To change the CD-ROM medium, libxl will:
+ - QMP eject the medium from QEMU
+ - block-detach the old PV disk
+ - block-attach the new PV disk
+ - QMP change the medium to the new PV disk by fdset-id
+
+The QMP change insert uses fdset-id STUBDOM_FDSET_CD + $disk - 'a'.
+That is, hda -> 'a', so
+STUBDOM_FDSET_CD + 'a' - 'a' = STUBDOM_FDSET_CD.
+For hdc:
+STUBDOM_FDSET_CD + 'c' - 'a' = STUBDOM_FDSET_CD + 2.
+
+The stubdom must internally handle adding /dev/xvdc to the appropriate
+fdset.
 
                                    PV-GRUB
                                    =======
diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
index 09082ffb58..6354982c05 100644
--- a/tools/libs/light/libxl_disk.c
+++ b/tools/libs/light/libxl_disk.c
@@ -808,25 +808,46 @@  int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
 
 typedef struct {
     libxl__ao *ao;
+    libxl__ao_device aodev;
+    libxl__ao_device aodev_del;
     libxl_domid domid;
+    libxl_domid disk_domid;
     libxl_device_disk *disk;
     libxl_device_disk disk_saved;
     libxl__ev_slowlock qmp_lock;
     int dm_ver;
     libxl__ev_time time;
+    libxl__ev_time timeout_retry;
     libxl__ev_qmp qmp;
+    int retries;
+    int stubdom_fdset;
 } libxl__cdrom_insert_state;
 
 static void cdrom_insert_lock_acquired(libxl__egc *, libxl__ev_slowlock *,
                                        int rc);
 static void cdrom_insert_qmp_connected(libxl__egc *, libxl__ev_qmp *,
                                        const libxl__json_object *, int rc);
+static void cdrom_insert_stubdom_removefd(libxl__egc *egc, libxl__ev_qmp *qmp,
+                                          const libxl__json_object *response,
+                                          int rc);
+static void cdrom_insert_stubdom_ejected(libxl__egc *egc, libxl__ev_qmp *,
+                                         const libxl__json_object *, int rc);
+static void cdrom_insert_stubdom_disk_ejected_aocomplete(libxl__egc *egc,
+                                                         libxl__ao_device *aodev);
+static void cdrom_insert_stubdom_disk_ejected(libxl__egc *egc, libxl__ev_qmp *,
+                                              const libxl__json_object *,
+                                              int rc);
+static void cdrom_insert_ejected_aodevcb(libxl__egc *egc,
+                                         libxl__ao_device *aodev);
 static void cdrom_insert_ejected(libxl__egc *egc, libxl__ev_qmp *,
                                  const libxl__json_object *, int rc);
 static void cdrom_insert_addfd_cb(libxl__egc *egc, libxl__ev_qmp *,
                                   const libxl__json_object *, int rc);
 static void cdrom_insert_inserted(libxl__egc *egc, libxl__ev_qmp *,
                                   const libxl__json_object *, int rc);
+static void cdrom_insert_addfd_retry(libxl__egc *egc, libxl__ev_time *ev,
+                                     const struct timeval *requested_abs,
+                                     int rc);
 static void cdrom_insert_timout(libxl__egc *egc, libxl__ev_time *ev,
                                 const struct timeval *requested_abs,
                                 int rc);
@@ -842,6 +863,7 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     libxl_device_disk *disks = NULL;
     int rc;
     libxl__cdrom_insert_state *cis;
+    libxl_domid stubdomid;
 
     GCNEW(cis);
     cis->ao = ao;
@@ -853,6 +875,8 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     cis->qmp_lock.ao = ao;
     cis->qmp_lock.domid = domid;
     libxl__ev_time_init(&cis->time);
+    libxl__ev_time_init(&cis->timeout_retry);
+    cis->retries = 0;
     libxl__ev_qmp_init(&cis->qmp);
     cis->qmp.ao = ao;
     cis->qmp.domid = domid;
@@ -869,12 +893,6 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         goto out;
     }
 
-    if (libxl_get_stubdom_id(ctx, domid) != 0) {
-        LOGD(ERROR, domid, "cdrom-insert doesn't work for stub domains");
-        rc = ERROR_INVAL;
-        goto out;
-    }
-
     cis->dm_ver = libxl__device_model_version_running(gc, domid);
     if (cis->dm_ver == -1) {
         LOGD(ERROR, domid, "Cannot determine device model version");
@@ -882,7 +900,22 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         goto out;
     }
 
-    disks = libxl__device_list(gc, &libxl__disk_devtype, domid, &num);
+    stubdomid = libxl_get_stubdom_id(CTX, cis->domid);
+    if (stubdomid == 0) {
+        cis->disk_domid = domid;
+    } else {
+        cis->disk_domid = stubdomid;
+        disk->backend = LIBXL_DISK_BACKEND_PHY;
+    }
+
+    if (cis->dm_ver == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL &&
+        stubdomid) {
+        LOGD(ERROR, domid, "cdrom-insert doesn't work for Mini-OS stubdoms");
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    disks = libxl__device_list(gc, &libxl__disk_devtype, cis->disk_domid, &num);
     for (i = 0; i < num; i++) {
         if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
         {
@@ -897,7 +930,7 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         goto out;
     }
 
-    rc = libxl__device_disk_setdefault(gc, domid, disk, false);
+    rc = libxl__device_disk_setdefault(gc, cis->disk_domid, disk, false);
     if (rc) goto out;
 
     if (!disk->pdev_path) {
@@ -905,6 +938,18 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         disk->format = LIBXL_DISK_FORMAT_EMPTY;
     }
 
+#define STUBDOM_FDSET_CD 8000
+    if (strncmp(disk->vdev, "hd", 2) == 0) {
+        cis->stubdom_fdset = STUBDOM_FDSET_CD + disk->vdev[2] - 'a';
+    } else if (strncmp(disk->vdev, "xvd", 3) == 0) {
+        cis->stubdom_fdset = STUBDOM_FDSET_CD + disk->vdev[3] - 'a';
+    } else {
+        LOGD(ERROR, cis->domid, "disk->vdev \"%s\" isn't hdX or xvdY",
+             disk->vdev);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
 out:
     libxl__device_list_free(&libxl__disk_devtype, disks, num);
     if (rc) {
@@ -923,6 +968,7 @@  static void cdrom_insert_lock_acquired(libxl__egc *egc,
     libxl__cdrom_insert_state *cis = CONTAINER_OF(lock, *cis, qmp_lock);
     STATE_AO_GC(cis->ao);
 
+    LOGD(DEBUG, cis->domid, "rc=%d", rc);
     if (rc) goto out;
 
     rc = libxl__ev_time_register_rel(ao, &cis->time,
@@ -971,7 +1017,12 @@  static void cdrom_insert_qmp_connected(libxl__egc *egc, libxl__ev_qmp *qmp,
         QMP_PARAMETERS_SPRINTF(&args, "id", "ide-%i", devid);
     else
         QMP_PARAMETERS_SPRINTF(&args, "device", "ide-%i", devid);
-    qmp->callback = cdrom_insert_ejected;
+
+    if (libxl_get_stubdom_id(CTX, cis->domid))
+        qmp->callback = cdrom_insert_stubdom_removefd;
+    else
+        qmp->callback = cdrom_insert_ejected;
+
     rc = libxl__ev_qmp_send(egc, qmp, "eject", args);
     if (rc) goto out;
     return;
@@ -979,6 +1030,148 @@  out:
     cdrom_insert_done(egc, cis, rc); /* must be last */
 }
 
+static void cdrom_insert_stubdom_removefd(libxl__egc *egc, libxl__ev_qmp *qmp,
+                                          const libxl__json_object *response,
+                                          int rc)
+{
+    libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
+    STATE_AO_GC(cis->ao);
+
+    if (rc) goto out;
+
+    /* Only called for qemu-xen/linux stubdom. */
+    assert(cis->dm_ver == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN);
+    libxl__json_object *args = NULL;
+
+    libxl__qmp_param_add_integer(gc, &args, "fdset-id", cis->stubdom_fdset);
+
+    cis->qmp.callback = cdrom_insert_stubdom_ejected;
+
+    rc = libxl__ev_qmp_send(egc, &cis->qmp, "remove-fd", args);
+    if (rc) goto out;
+
+    return;
+
+out:
+    cdrom_insert_done(egc, cis, rc); /* must be last */
+}
+
+static void cdrom_insert_stubdom_ejected(libxl__egc *egc, libxl__ev_qmp *qmp,
+                                         const libxl__json_object *response,
+                                         int rc)
+{
+    libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
+    libxl__device *device;
+    STATE_AO_GC(cis->ao);
+    domid_t stubdomid = libxl_get_stubdom_id(CTX, cis->domid);
+
+    LOGD(DEBUG, cis->domid, "rc=%d", rc);
+    /* cis->stubdom_fdset is initially empty, so remove-fd fails the first
+     * call with:
+     * {"error": {"class": "GenericError",
+     *            "desc": "File descriptor named 'fdset-id:8675' not found"}}
+     * Carry on in that case. */
+    if (rc && rc != ERROR_QMP_GENERIC_ERROR) goto out;
+
+    GCNEW(device);
+    rc = libxl__device_from_disk(gc, stubdomid, cis->disk, device);
+    if (rc != 0) goto out;
+
+    /* block dev eject */
+    /* Below is basically an open coding of:
+     * libxl_device_disk__remove(CTX, cis->domid, cis->disk, 0);
+     * ...since we can't call it from within libxl.
+     */
+    libxl__prepare_ao_device(ao, &cis->aodev_del);
+    cis->aodev_del.action = LIBXL__DEVICE_ACTION_REMOVE;
+    cis->aodev_del.dev = device;
+    cis->aodev_del.callback = cdrom_insert_stubdom_disk_ejected_aocomplete;
+    cis->aodev_del.force.flag = LIBXL__FORCE_OFF;
+    libxl__initiate_device_generic_remove(egc, &cis->aodev_del);
+    return;
+
+ out:
+    cdrom_insert_done(egc, cis, rc); /* must be last */
+}
+
+static void cdrom_insert_stubdom_disk_ejected_aocomplete(libxl__egc *egc,
+                                                         libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__cdrom_insert_state *cis = CONTAINER_OF(aodev, *cis, aodev_del);
+
+    LOGD(DEBUG, cis->domid, "rc=%d", aodev->rc);
+    if (aodev->rc) {
+        if (aodev->dev) {
+            LOGD(ERROR, aodev->dev->domid, "Unable to %s %s with id %u",
+                        libxl__device_action_to_string(aodev->action),
+                        libxl__device_kind_to_string(aodev->dev->kind),
+                        aodev->dev->devid);
+        } else {
+            LOG(ERROR, "unable to %s device",
+                       libxl__device_action_to_string(aodev->action));
+        }
+        goto out;
+    }
+
+    cdrom_insert_stubdom_disk_ejected(egc, &cis->qmp, NULL, aodev->rc);
+    return;
+
+ out:
+    cdrom_insert_done(egc, cis, aodev->rc);
+}
+
+static void cdrom_insert_stubdom_disk_ejected(libxl__egc *egc,
+                                              libxl__ev_qmp *qmp,
+                                              const libxl__json_object *response,
+                                              int rc)
+{
+    libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
+    STATE_AO_GC(cis->ao);
+    domid_t stubdomid = libxl_get_stubdom_id(CTX, cis->domid);
+
+    LOGD(DEBUG, cis->domid, "rc=%d", rc);
+    if (rc) goto out;
+
+    /* block dev insert - this may be inserting an empty disk for eject. */
+    libxl__prepare_ao_device(ao, &cis->aodev);
+    /* set an ao callback to end up in cdrom_insert_ejected */
+    cis->aodev.callback = cdrom_insert_ejected_aodevcb;
+    libxl__device_disk_add(egc, stubdomid, cis->disk, &cis->aodev);
+
+    return;
+
+ out:
+    cdrom_insert_done(egc, cis, rc); /* must be last */
+}
+
+static void cdrom_insert_ejected_aodevcb(libxl__egc *egc,
+                                         libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__cdrom_insert_state *cis = CONTAINER_OF(aodev, *cis, aodev);
+
+    LOGD(DEBUG, cis->domid, "rc=%d", aodev->rc);
+    if (aodev->rc) {
+        if (aodev->dev) {
+            LOGD(ERROR, aodev->dev->domid, "Unable to %s %s with id %u",
+                        libxl__device_action_to_string(aodev->action),
+                        libxl__device_kind_to_string(aodev->dev->kind),
+                        aodev->dev->devid);
+        } else {
+            LOG(ERROR, "unable to %s device",
+                       libxl__device_action_to_string(aodev->action));
+        }
+        goto out;
+    }
+
+    cdrom_insert_ejected(egc, &cis->qmp, NULL, aodev->rc);
+    return;
+
+ out:
+    cdrom_insert_done(egc, cis, aodev->rc);
+}
+
 static void cdrom_insert_ejected(libxl__egc *egc,
                                  libxl__ev_qmp *qmp,
                                  const libxl__json_object *response,
@@ -1001,9 +1194,10 @@  static void cdrom_insert_ejected(libxl__egc *egc,
 
     libxl_domain_config_init(&d_config);
 
+    LOGD(DEBUG, cis->domid, "rc=%d", rc);
     if (rc) goto out;
 
-    rc = libxl__device_from_disk(gc, domid, disk, &device);
+    rc = libxl__device_from_disk(gc, cis->disk_domid, disk, &device);
     if (rc) goto out;
     be_path = libxl__device_backend_path(gc, &device);
     libxl_path = libxl__device_libxl_path(gc, &device);
@@ -1050,7 +1244,7 @@  static void cdrom_insert_ejected(libxl__egc *egc,
      */
 
     rc = libxl__get_domain_configuration(gc, domid, &d_config);
-    if (rc) goto out;
+    if (rc && rc != ERROR_JSON_CONFIG_EMPTY) goto out;
 
     device_add_domain_config(gc, &d_config, &libxl__disk_devtype,
                              &cis->disk_saved);
@@ -1058,10 +1252,15 @@  static void cdrom_insert_ejected(libxl__egc *egc,
     rc = libxl__dm_check_start(gc, &d_config, domid);
     if (rc) goto out;
 
+    LOGD(DEBUG, cis->domid, "stubdom_id=%d",
+                libxl_get_stubdom_id(CTX, cis->domid));
+    /* A linux stubdom will perform add-fd with calculated stubdom_fdset. */
     if (cis->dm_ver == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN &&
+        libxl_get_stubdom_id(CTX, cis->domid) == 0 &&
         disk->format != LIBXL_DISK_FORMAT_EMPTY) {
         libxl__json_object *args = NULL;
 
+        LOGD(DEBUG, cis->domid, "Doing qmp add-fd path");
         assert(qmp->payload_fd == -1);
         qmp->payload_fd = open(disk->pdev_path, O_RDONLY);
         if (qmp->payload_fd < 0) {
@@ -1080,20 +1279,30 @@  static void cdrom_insert_ejected(libxl__egc *egc,
         if (rc) goto out;
         has_callback = true;
     } else {
+        LOGD(DEBUG, cis->domid, "Skipping qmp add-fd path");
         has_callback = false;
     }
 
     rc = 0;
 
 out:
+    LOGD(DEBUG, cis->domid, "out label rc=%d", rc);
     libxl__xs_transaction_abort(gc, &t);
     libxl_domain_config_dispose(&d_config);
     if (data_lock) libxl__unlock_file(data_lock);
     if (rc) {
         cdrom_insert_done(egc, cis, rc); /* must be last */
     } else if (!has_callback) {
-        /* Only called if no asynchronous callback are set. */
-        cdrom_insert_inserted(egc, qmp, NULL, 0); /* must be last */
+        if (libxl_get_stubdom_id(CTX, cis->domid) &&
+            disk->format != LIBXL_DISK_FORMAT_EMPTY) {
+            LOGD(DEBUG, cis->domid,
+                 "stubdom %d needs to perform add-fd internally",
+                 libxl_get_stubdom_id(CTX, cis->domid));
+            cdrom_insert_addfd_cb(egc, qmp, NULL, 0); /* must be last */
+        } else  {
+            /* Only called if no asynchronous callback are set. */
+            cdrom_insert_inserted(egc, qmp, NULL, 0); /* must be last */
+        }
     }
 }
 
@@ -1112,17 +1321,24 @@  static void cdrom_insert_addfd_cb(libxl__egc *egc,
     /* convenience aliases */
     libxl_device_disk *disk = cis->disk;
 
-    close(qmp->payload_fd);
-    qmp->payload_fd = -1;
+    LOGD(DEBUG, cis->domid, "rc=%d", rc);
 
     if (rc) goto out;
 
-    o = libxl__json_map_get("fdset-id", response, JSON_INTEGER);
-    if (!o) {
-        rc = ERROR_FAIL;
-        goto out;
+    /* response non-NULL only for non-stubdom */
+    if (response) {
+        close(qmp->payload_fd);
+        qmp->payload_fd = -1;
+
+        o = libxl__json_map_get("fdset-id", response, JSON_INTEGER);
+        if (!o) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+        fdset = libxl__json_object_get_integer(o);
+    } else {
+        fdset = cis->stubdom_fdset;
     }
-    fdset = libxl__json_object_get_integer(o);
 
     devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
     qmp->callback = cdrom_insert_inserted;
@@ -1135,8 +1351,13 @@  static void cdrom_insert_addfd_cb(libxl__egc *egc,
     if (libxl__qmp_ev_qemu_compare_version(qmp, 2, 8, 0) >= 0) {
         QMP_PARAMETERS_SPRINTF(&args, "id", "ide-%i", devid);
         QMP_PARAMETERS_SPRINTF(&args, "filename", "/dev/fdset/%d", fdset);
-        libxl__qmp_param_add_string(gc, &args, "format",
-            libxl__qemu_disk_format_string(disk->format));
+        if (response) {
+            libxl__qmp_param_add_string(gc, &args, "format",
+                libxl__qemu_disk_format_string(disk->format));
+        } else {
+            /* Stubdom is using blockdev /dev/xvd* */
+            libxl__qmp_param_add_string(gc, &args, "format", "host_device");
+        }
         rc = libxl__ev_qmp_send(egc, qmp, "blockdev-change-medium", args);
     } else {
         QMP_PARAMETERS_SPRINTF(&args, "device", "ide-%i", devid);
@@ -1150,13 +1371,22 @@  out:
         cdrom_insert_done(egc, cis, rc); /* must be last */
 }
 
+static void cdrom_insert_addfd_retry(libxl__egc *egc, libxl__ev_time *ev,
+                                     const struct timeval *requested_abs,
+                                     int rc)
+{
+    libxl__cdrom_insert_state *cis = CONTAINER_OF(ev, *cis, timeout_retry);
+
+    cdrom_insert_addfd_cb(egc, &cis->qmp, NULL, 0);
+}
+
 static void cdrom_insert_inserted(libxl__egc *egc,
                                   libxl__ev_qmp *qmp,
                                   const libxl__json_object *response,
                                   int rc)
 {
-    EGC_GC;
     libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
+    STATE_AO_GC(cis->ao);
     libxl__flock *data_lock = NULL;
     libxl_domain_config d_config;
     flexarray_t *insert = NULL;
@@ -1171,9 +1401,22 @@  static void cdrom_insert_inserted(libxl__egc *egc,
 
     libxl_domain_config_init(&d_config);
 
-    if (rc) goto out;
+    LOGD(DEBUG, cis->domid, "rc=%d", rc);
+
+    if (rc) {
+        if (cis->retries++ < 10 ) {
+            LOGD(DEBUG, qmp->domid, "Retrying QMP cdrom change\n");
+            rc = libxl__ev_time_register_rel(ao, &cis->timeout_retry,
+                                             cdrom_insert_addfd_retry, 100);
+            if (rc) goto out;
 
-    rc = libxl__device_from_disk(gc, domid, disk, &device);
+            return;
+        } else {
+            goto out;
+        }
+    }
+
+    rc = libxl__device_from_disk(gc, cis->disk_domid, disk, &device);
     if (rc) goto out;
     be_path = libxl__device_backend_path(gc, &device);
     libxl_path = libxl__device_libxl_path(gc, &device);
@@ -1185,7 +1428,7 @@  static void cdrom_insert_inserted(libxl__egc *egc,
     }
 
     rc = libxl__get_domain_configuration(gc, domid, &d_config);
-    if (rc) goto out;
+    if (rc && rc != ERROR_JSON_CONFIG_EMPTY) goto out;
 
     device_add_domain_config(gc, &d_config, &libxl__disk_devtype,
                              &cis->disk_saved);
@@ -1257,7 +1500,10 @@  static void cdrom_insert_done(libxl__egc *egc,
 {
     EGC_GC;
 
+    LOGD(DEBUG, cis->domid, "rc=%d", rc);
+
     libxl__ev_time_deregister(gc, &cis->time);
+    libxl__ev_time_deregister(gc, &cis->timeout_retry);
     libxl__ev_qmp_dispose(gc, &cis->qmp);
     if (cis->qmp.payload_fd >= 0) close(cis->qmp.payload_fd);
     libxl__ev_slowlock_unlock(gc, &cis->qmp_lock);