Xen devel - Jul 2012 - [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.

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

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

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

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

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

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.

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.

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.

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.

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.