[PATCH 0/3] libxl cd-insert/eject with qemu-xen

[PATCH 0/3] libxl cd-insert/eject with qemu-xen

[Anthony PERARD]

This patch series provides the facility to eject and insert a cdrom when the
used device-model is qemu-xen. The only difference between both device-model is
a call to a QMP command as `xl cd-insert ...` will still update xenstore, even
if it's not used by QEMU.

Anthony PERARD (3):
  libxl_qmp, Introduce libxl__qmp_insert_cdrom.
  libxl_dm: Set an id to cdrom drives with qemuu.
  libxl: Fix cd-insert with qemu-xen.

 tools/libxl/libxl.c          |   12 ++++++------
 tools/libxl/libxl_dm.c       |    7 ++++---
 tools/libxl/libxl_internal.h |    1 +
 tools/libxl/libxl_qmp.c      |   35 +++++++++++++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 9 deletions(-)

-- 
Anthony PERARD

[PATCH 1/3] libxl_qmp, Introduce libxl__qmp_insert_cdrom.

[Anthony PERARD]

This function can eject or change the CDROM for a guest that use qemu-xen as a
device-model.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_internal.h |    1 +
 tools/libxl/libxl_qmp.c      |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c369fd3..eb8786c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1389,6 +1389,7 @@ _hidden int libxl__qmp_resume(libxl__gc *gc, int domid);
 _hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
 /* Set dirty bitmap logging status */
 _hidden int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool enable);
+_hidden int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid, const libxl_device_disk *disk);
 /* close and free the QMP handler */
 _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
 /* remove the socket file, if the file has already been removed,
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 2c5559f..aebc733 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1024,6 +1024,41 @@ out:
     return rc;
 }
 
+int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid,
+                            const libxl_device_disk *disk)
+{
+    libxl__qmp_handler *qmp = NULL;
+    char *device_id;
+    int rc = 0;
+
+    device_id = libxl__sprintf(gc, "ide-%s", disk->vdev);
+    if (!device_id)
+        return ERROR_NOMEM;
+
+    qmp = libxl__qmp_initialize(gc, domid);
+    if (!qmp)
+        return ERROR_FAIL;
+
+    if (disk->format == LIBXL_DISK_FORMAT_EMPTY) {
+        libxl__json_object *args = NULL;
+
+        args = qmp_parameters_add_string(gc, NULL, "device", device_id);
+        if (!args) {
+            rc = ERROR_NOMEM;
+            goto out;
+        }
+        rc = qmp_synchronous_send(qmp, "eject", args, NULL, NULL, qmp->timeout);
+        libxl__json_object_free(gc, args);
+    } else {
+        char *file = disk->pdev_path;
+        rc = qmp_change(gc, qmp, device_id, file, NULL);
+    }
+
+out:
+    libxl__qmp_close(qmp);
+    return rc;
+}
+
 int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
                                const libxl_domain_config *guest_config)
 {
-- 
Anthony PERARD

[PATCH 2/3] libxl_dm: Set an id to cdrom drives with qemuu.

[Anthony PERARD]

In order to eject and change a cdrom when using qemu-xen, this patch adds an id
the cdrom driver when starting the device model.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_dm.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f2e9572..885fc46 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -549,11 +549,12 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
             if (disks[i].is_cdrom) {
                 if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY)
                     drive = libxl__sprintf
-                        (gc, "if=ide,index=%d,media=cdrom", disk);
+                        (gc, "if=ide,index=%d,media=cdrom,id=ide-%s",
+                         disk, disks[i].vdev);
                 else
                     drive = libxl__sprintf
-                        (gc, "file=%s,if=ide,index=%d,media=cdrom,format=%s",
-                         disks[i].pdev_path, disk, format);
+                        (gc, "file=%s,if=ide,index=%d,media=cdrom,format=%s,id=ide-%s",
+                         disks[i].pdev_path, disk, format, disks[i].vdev);
             } else {
                 if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
                     LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "cannot support"
-- 
Anthony PERARD

[PATCH 3/3] libxl: Fix cd-insert with qemu-xen.

[Anthony PERARD]

If qemu-xen is used as a device model, the command to insert, change or eject a
cdrom will go through QMP. XenStore is still updated even if QEMU will not read
from it.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 00ddc0e..ed89929 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2167,12 +2167,6 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         rc = ERROR_FAIL;
         goto out;
     }
-    if (dm_ver != LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
-        LOG(ERROR, "cdrom-insert does not work with %s",
-            libxl_device_model_version_to_string(dm_ver));
-        rc = ERROR_INVAL;
-        goto out;
-    }
 
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
@@ -2196,6 +2190,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
 
     rc = libxl__device_from_disk(gc, domid, disk, &device);
     if (rc) goto out;
+
+    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+        rc = libxl__qmp_insert_cdrom(gc, domid, disk);
+        if (rc) goto out;
+    }
+
     path = libxl__device_backend_path(gc, &device);
 
     insert = flexarray_make(4, 1);
-- 
Anthony PERARD

Re: [PATCH 0/3] libxl cd-insert/eject with qemu-xen

[Anthony PERARD]

On Fri, Jul 27, 2012 at 6:17 PM, Anthony PERARD
<anthony.perard@citrix.com> wrote:
> This patch series provides the facility to eject and insert a cdrom when the
> used device-model is qemu-xen. The only difference between both device-model is
> a call to a QMP command as `xl cd-insert ...` will still update xenstore, even
> if it's not used by QEMU.

I forgot to tell, but this series is based on my previous series "Set
dirty log on qemu-xen." (for the libxl_qmp part)

> Anthony PERARD (3):
>   libxl_qmp, Introduce libxl__qmp_insert_cdrom.
>   libxl_dm: Set an id to cdrom drives with qemuu.
>   libxl: Fix cd-insert with qemu-xen.



-- 
Anthony PERARD

Re: [PATCH 0/3] libxl cd-insert/eject with qemu-xen

[Ian Jackson]

Anthony PERARD writes ("[PATCH 0/3] libxl cd-insert/eject with qemu-xen"):
> This patch series provides the facility to eject and insert a cdrom
> when the used device-model is qemu-xen. The only difference between
> both device-model is a call to a QMP command as `xl cd-insert ...`
> will still update xenstore, even if it's not used by QEMU.

Thanks.  I'm afraid that this has missed the boat for 4.2 I think.

I'll review it anyway while I've got much of the related stuff
relatively fresh in my mind.

Ian.

Re: [PATCH 1/3] libxl_qmp, Introduce libxl__qmp_insert_cdrom.

[Ian Jackson]

Anthony PERARD writes ("[PATCH 1/3] libxl_qmp, Introduce libxl__qmp_insert_cdrom."):
> This function can eject or change the CDROM for a guest that use qemu-xen as a
> device-model.
...
> +int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid,
> +                            const libxl_device_disk *disk)
> +{
> +    libxl__qmp_handler *qmp = NULL;
> +    char *device_id;
> +    int rc = 0;
> +
> +    device_id = libxl__sprintf(gc, "ide-%s", disk->vdev);
> +    if (!device_id)
> +        return ERROR_NOMEM;

Use GCSPRINTF (and ditch the error check) ?

> +    qmp = libxl__qmp_initialize(gc, domid);
> +    if (!qmp)
> +        return ERROR_FAIL;

AIUI qemu only supports one concurrent user of its qmp.  You don't
seem to have anything here to deal with that.  Personally I think that
needs to be fixed in qemu.

> +    if (disk->format == LIBXL_DISK_FORMAT_EMPTY) {
> +        libxl__json_object *args = NULL;
> +
> +        args = qmp_parameters_add_string(gc, NULL, "device", device_id);

What is this function ?  I don't seem to be able to find it.  Perhaps
it's in another patch series of yours ?

Thanks,
Ian.

Re: [PATCH 2/3] libxl_dm: Set an id to cdrom drives with qemuu.

[Ian Jackson]

Anthony PERARD writes ("[PATCH 2/3] libxl_dm: Set an id to cdrom drives with qemuu."):
> In order to eject and change a cdrom when using qemu-xen, this patch
> adds an id the cdrom driver when starting the device model.

> +                        (gc, "if=ide,index=%d,media=cdrom,id=ide-%s",
> +                         disk, disks[i].vdev);

You should not use the vdev string for this purpose because several
different vdev strings can refer to the same virtual device.

You should use the dev number instead.

Ian.

Re: [PATCH 3/3] libxl: Fix cd-insert with qemu-xen.

[Ian Jackson]

Anthony PERARD writes ("[PATCH 3/3] libxl: Fix cd-insert with qemu-xen."):
> If qemu-xen is used as a device model, the command to insert, change
> or eject a cdrom will go through QMP. XenStore is still updated even
> if QEMU will not read from it.

This looks plausible although I haven't tested it.

Ian.

Re: [PATCH 0/3] libxl cd-insert/eject with qemu-xen

[Ian Jackson]

Anthony PERARD writes ("Re: [Xen-devel] [PATCH 0/3] libxl cd-insert/eject with qemu-xen"):
> On Fri, Jul 27, 2012 at 6:17 PM, Anthony PERARD
> <anthony.perard@citrix.com> wrote:
> > This patch series provides the facility to eject and insert a cdrom when the
> > used device-model is qemu-xen. The only difference between both device-model is
> > a call to a QMP command as `xl cd-insert ...` will still update xenstore, even
> > if it's not used by QEMU.
> 
> I forgot to tell, but this series is based on my previous series "Set
> dirty log on qemu-xen." (for the libxl_qmp part)

Aha, that's where that missing function is.

Ian.