[PATCH 0/4] libxl: fix CDROM issues

[PATCH 0/4] libxl: fix CDROM issues

[Roger Pau Monne]

Due to recent changes (and intended fixes) CDROM has become quite unstable 
recently. This series aims to fix the issues reported with having empty 
CDROM devices in the HVM guest configuration files. It also contains some 
cleanups of more generic code in order to ease the implementation.

After some talk on IRC, the easiest solution at this point in the release is 
to force PV CDROM backends of HVM guests with a device model to use Qdisk, 
since the cd-{insert/eject} implementation is only able to deal with this 
kind of PV backend.

Roger.


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

[PATCH 1/4] libxl: set the device model version earlier in xenstore

[Roger Pau Monne]

So libxl doesn't have to pass the build info around just to get the device
model used by the guest. This allows to simplify
libxl__device_nic_setdefault.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c          | 24 +++++++-----------------
 tools/libxl/libxl_create.c   |  9 +++++++--
 tools/libxl/libxl_dm.c       |  3 +--
 tools/libxl/libxl_internal.h |  3 +--
 4 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 5cdc09e..d35fc33 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3292,7 +3292,7 @@ out:
 /******************************************************************************/
 
 int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
-                                 uint32_t domid, libxl_domain_build_info *info)
+                                 uint32_t domid)
 {
     int rc;
 
@@ -3330,21 +3330,11 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
     switch (libxl__domain_type(gc, domid)) {
     case LIBXL_DOMAIN_TYPE_HVM:
         if (!nic->nictype) {
-            if (info != NULL) {
-                /* Path taken at creation time. */
-                if (info->device_model_version ==
-                    LIBXL_DEVICE_MODEL_VERSION_NONE)
-                    nic->nictype = LIBXL_NIC_TYPE_VIF;
-                else
-                    nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
-            } else {
-                /* Path taken when hot-adding a nic. */
-                if (libxl__device_model_version_running(gc, domid) ==
-                    LIBXL_DEVICE_MODEL_VERSION_NONE)
-                    nic->nictype = LIBXL_NIC_TYPE_VIF;
-                else
-                    nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
-            }
+            if (libxl__device_model_version_running(gc, domid) ==
+                LIBXL_DEVICE_MODEL_VERSION_NONE)
+                nic->nictype = LIBXL_NIC_TYPE_VIF;
+            else
+                nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
         }
         break;
     case LIBXL_DOMAIN_TYPE_PV:
@@ -3394,7 +3384,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
     libxl_device_nic_init(&nic_saved);
     libxl_device_nic_copy(CTX, &nic_saved, nic);
 
-    rc = libxl__device_nic_setdefault(gc, nic, domid, NULL);
+    rc = libxl__device_nic_setdefault(gc, nic, domid);
     if (rc) goto out;
 
     front = flexarray_make(gc, 16, 1);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4b02de9..24f168b 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -916,6 +916,12 @@ static void initiate_domain_create(libxl__egc *egc,
         goto error_out;
     }
 
+    /*
+     * Set the dm version quite early so that libxl doesn't have to pass the
+     * build info around just to know if the domain has a device model or not.
+     */
+    store_libxl_entry(gc, domid, &d_config->b_info);
+
     for (i = 0; i < d_config->num_disks; i++) {
         ret = libxl__device_disk_setdefault(gc, &d_config->disks[i]);
         if (ret) {
@@ -940,8 +946,7 @@ static void initiate_domain_create(libxl__egc *egc,
          * called libxl_device_nic_add when domcreate_launch_dm gets called,
          * but qemu needs the nic information to be complete.
          */
-        ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid,
-                                           &d_config->b_info);
+        ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
         if (ret) {
             LOG(ERROR, "Unable to set nic defaults for nic %d", i);
             goto error_out;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index eac5501..a12c90d 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1810,8 +1810,7 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
          * called libxl_device_nic_add at this point, but qemu needs
          * the nic information to be complete.
          */
-        ret = libxl__device_nic_setdefault(gc, &dm_config->nics[i], dm_domid,
-                                           &dm_config->b_info);
+        ret = libxl__device_nic_setdefault(gc, &dm_config->nics[i], dm_domid);
         if (ret)
             goto out;
     }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 653c152..55896f8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1206,8 +1206,7 @@ _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
 _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
                                           libxl_device_disk *disk);
 _hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
-                                         uint32_t domid,
-                                         libxl_domain_build_info *info);
+                                         uint32_t domid);
 _hidden int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm);
 _hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
 _hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
-- 
2.6.4 (Apple Git-63)


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

[PATCH 2/4] libxl: set the backend type to Qdisk for CDROM devices on DM HVM guests

[Roger Pau Monne]

This is needed because the cd-{insert/eject} functions are not prepared to
deal with blkback, which would be used by default if no backend was
specified.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c          | 24 +++++++++++++++++++-----
 tools/libxl/libxl_create.c   |  2 +-
 tools/libxl/libxl_internal.h |  3 ++-
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index d35fc33..9d785a4 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2301,7 +2301,8 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
+int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk,
+                                  uint32_t domid)
 {
     int rc;
 
@@ -2312,6 +2313,19 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
     rc = libxl__resolve_domid(gc, disk->backend_domname, &disk->backend_domid);
     if (rc < 0) return rc;
 
+    /* Force Qdisk backend for CDROM devices of guests with a device model. */
+    if (disk->is_cdrom != 0 &&
+        libxl__domain_type(gc, domid) == LIBXL_DOMAIN_TYPE_HVM &&
+        libxl__device_model_version_running(gc, domid) !=
+        LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        if (!(disk->backend == LIBXL_DISK_BACKEND_QDISK ||
+              disk->backend == LIBXL_DISK_BACKEND_UNKNOWN)) {
+            LOG(ERROR, "Backend for CD devices on HVM guests must be Qdisk");
+            return ERROR_FAIL;
+        }
+        disk->backend = LIBXL_DISK_BACKEND_QDISK;
+    }
+
     rc = libxl__device_disk_set_backend(gc, disk);
     return rc;
 }
@@ -2427,7 +2441,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
             }
         }
 
-        rc = libxl__device_disk_setdefault(gc, disk);
+        rc = libxl__device_disk_setdefault(gc, disk, domid);
         if (rc) goto out;
 
         front = flexarray_make(gc, 16, 1);
@@ -2869,7 +2883,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     disk_empty.vdev = libxl__strdup(NOGC, disk->vdev);
     disk_empty.pdev_path = libxl__strdup(NOGC, "");
     disk_empty.is_cdrom = 1;
-    libxl__device_disk_setdefault(gc, &disk_empty);
+    libxl__device_disk_setdefault(gc, &disk_empty, domid);
 
     libxl_domain_type type = libxl__domain_type(gc, domid);
     if (type == LIBXL_DOMAIN_TYPE_INVALID) {
@@ -2910,7 +2924,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         goto out;
     }
 
-    rc = libxl__device_disk_setdefault(gc, disk);
+    rc = libxl__device_disk_setdefault(gc, disk, domid);
     if (rc) goto out;
 
     if (!disk->pdev_path) {
@@ -3173,7 +3187,7 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
             disk->script = libxl__strdup(gc, in_disk->script);
         disk->vdev = NULL;
         
-        rc = libxl__device_disk_setdefault(gc, disk);
+        rc = libxl__device_disk_setdefault(gc, disk, LIBXL_TOOLSTACK_DOMID);
         if (rc) goto out;
 
         libxl__prepare_ao_device(ao, &dls->aodev);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 24f168b..1a57907 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -923,7 +923,7 @@ static void initiate_domain_create(libxl__egc *egc,
     store_libxl_entry(gc, domid, &d_config->b_info);
 
     for (i = 0; i < d_config->num_disks; i++) {
-        ret = libxl__device_disk_setdefault(gc, &d_config->disks[i]);
+        ret = libxl__device_disk_setdefault(gc, &d_config->disks[i], domid);
         if (ret) {
             LOG(ERROR, "Unable to set disk defaults for disk %d", i);
             goto error_out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 55896f8..47aca76 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1204,7 +1204,8 @@ _hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
 _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
                                         libxl_domain_build_info *b_info);
 _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
-                                          libxl_device_disk *disk);
+                                          libxl_device_disk *disk,
+                                          uint32_t domid);
 _hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
                                          uint32_t domid);
 _hidden int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm);
-- 
2.6.4 (Apple Git-63)


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

[PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Roger Pau Monne]

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9d785a4..232e2c1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         goto out;
     }
 
+    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
         if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
-- 
2.6.4 (Apple Git-63)


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

[PATCH 4/4] libxl: remove code added to use the 'phy' backend with CDROM devices

[Roger Pau Monne]

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_device.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index ce53520..82405a7 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -191,20 +191,13 @@ static int disk_try_backend(disk_try_backend_args *a,
 
     switch (backend) {
     case LIBXL_DISK_BACKEND_PHY:
-        if (!(a->disk->format == LIBXL_DISK_FORMAT_RAW ||
-              a->disk->format == LIBXL_DISK_FORMAT_EMPTY)) {
+        if (a->disk->format != LIBXL_DISK_FORMAT_RAW) {
             goto bad_format;
         }
 
         if (libxl_defbool_val(a->disk->colo_enable))
             goto bad_colo;
 
-        if (a->disk->format == LIBXL_DISK_FORMAT_EMPTY) {
-            LOG(DEBUG, "Disk vdev=%s is empty, skipping physical device check",
-                a->disk->vdev);
-            return backend;
-        }
-
         if (a->disk->backend_domid != LIBXL_TOOLSTACK_DOMID) {
             LOG(DEBUG, "Disk vdev=%s, is using a storage driver domain, "
                        "skipping physical device check", a->disk->vdev);
-- 
2.6.4 (Apple Git-63)


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

Re: [PATCH 1/4] libxl: set the device model version earlier in xenstore

[Wei Liu]

On Thu, Apr 07, 2016 at 07:45:26PM +0200, Roger Pau Monne wrote:
> So libxl doesn't have to pass the build info around just to get the device
> model used by the guest. This allows to simplify
> libxl__device_nic_setdefault.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

I have some reservation on this approach. I would rather passing around
a struct than accessing xenstore. The latter is much more expensive.

Wei.

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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Wei Liu]

On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 9d785a4..232e2c1 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
>          goto out;
>      }
>  
> +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");

It's not specific to HVM, right?

Wei.

> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
>      disks = libxl_device_disk_list(ctx, domid, &num);
>      for (i = 0; i < num; i++) {
>          if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
> -- 
> 2.6.4 (Apple Git-63)
> 

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

Re: [PATCH 2/4] libxl: set the backend type to Qdisk for CDROM devices on DM HVM guests

[Wei Liu]

On Thu, Apr 07, 2016 at 07:45:27PM +0200, Roger Pau Monne wrote:
> This is needed because the cd-{insert/eject} functions are not prepared to
> deal with blkback, which would be used by default if no backend was
> specified.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH 4/4] libxl: remove code added to use the 'phy' backend with CDROM devices

[Wei Liu]

On Thu, Apr 07, 2016 at 07:45:29PM +0200, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  tools/libxl/libxl_device.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index ce53520..82405a7 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -191,20 +191,13 @@ static int disk_try_backend(disk_try_backend_args *a,
>  
>      switch (backend) {
>      case LIBXL_DISK_BACKEND_PHY:
> -        if (!(a->disk->format == LIBXL_DISK_FORMAT_RAW ||
> -              a->disk->format == LIBXL_DISK_FORMAT_EMPTY)) {
> +        if (a->disk->format != LIBXL_DISK_FORMAT_RAW) {
>              goto bad_format;
>          }
>  
>          if (libxl_defbool_val(a->disk->colo_enable))
>              goto bad_colo;
>  
> -        if (a->disk->format == LIBXL_DISK_FORMAT_EMPTY) {
> -            LOG(DEBUG, "Disk vdev=%s is empty, skipping physical device check",
> -                a->disk->vdev);
> -            return backend;
> -        }
> -
>          if (a->disk->backend_domid != LIBXL_TOOLSTACK_DOMID) {
>              LOG(DEBUG, "Disk vdev=%s, is using a storage driver domain, "
>                         "skipping physical device check", a->disk->vdev);
> -- 
> 2.6.4 (Apple Git-63)
> 

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

Re: [PATCH 1/4] libxl: set the device model version earlier in xenstore

[Roger Pau Monné]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

On Fri, 8 Apr 2016, Wei Liu wrote:
> On Thu, Apr 07, 2016 at 07:45:26PM +0200, Roger Pau Monne wrote:
> > So libxl doesn't have to pass the build info around just to get the device
> > model used by the guest. This allows to simplify
> > libxl__device_nic_setdefault.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> 
> I have some reservation on this approach. I would rather passing around
> a struct than accessing xenstore. The latter is much more expensive.

Sorry, my commit log wasn't very clear. The struct can't always be passed 
around, since it's only available at domain creation time, so we also need 
to xenstore way for hotplug.

Due to that, I think it's simpler to always use it, instead of having two 
different approaches depending on whether the build info struct is 
provided or not. TBH, this is only one xenstore read, so I would rather 
prefer to do it always this way in order to have simpler code.

Roger.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Roger Pau Monné]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

On Fri, 8 Apr 2016, Wei Liu wrote:
> On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  tools/libxl/libxl.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 9d785a4..232e2c1 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
> >          goto out;
> >      }
> >  
> > +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> > +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
> 
> It's not specific to HVM, right?

It's specific to guests with a device model (those are the only ones that 
can have an emulated CDROM device), and the only guests that can have a 
device model are HVM (although not all of them).

Roger.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1/4] libxl: set the device model version earlier in xenstore

[Wei Liu]

On Fri, Apr 08, 2016 at 04:16:19PM +0200, Roger Pau Monné wrote:
> On Fri, 8 Apr 2016, Wei Liu wrote:
> > On Thu, Apr 07, 2016 at 07:45:26PM +0200, Roger Pau Monne wrote:
> > > So libxl doesn't have to pass the build info around just to get the device
> > > model used by the guest. This allows to simplify
> > > libxl__device_nic_setdefault.
> > > 
> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > Cc: Wei Liu <wei.liu2@citrix.com>
> > 
> > I have some reservation on this approach. I would rather passing around
> > a struct than accessing xenstore. The latter is much more expensive.
> 
> Sorry, my commit log wasn't very clear. The struct can't always be passed 
> around, since it's only available at domain creation time, so we also need 
> to xenstore way for hotplug.
> 
> Due to that, I think it's simpler to always use it, instead of having two 
> different approaches depending on whether the build info struct is 
> provided or not. TBH, this is only one xenstore read, so I would rather 
> prefer to do it always this way in order to have simpler code.
> 

In this case:

Acked-by: Wei Liu <wei.liu2@citrix.com>

Wei.

> Roger.


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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Wei Liu]

On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote:
> On Fri, 8 Apr 2016, Wei Liu wrote:
> > On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > ---
> > >  tools/libxl/libxl.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > index 9d785a4..232e2c1 100644
> > > --- a/tools/libxl/libxl.c
> > > +++ b/tools/libxl/libxl.c
> > > @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
> > >          goto out;
> > >      }
> > >  
> > > +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> > > +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
> > 
> > It's not specific to HVM, right?
> 
> It's specific to guests with a device model (those are the only ones that 
> can have an emulated CDROM device), and the only guests that can have a 
> device model are HVM (although not all of them).
> 

Remind me again: how does PV guest CDROM work? ISTR it will also go
through this function? My memory is vague though...

Wei.

> Roger.


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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Ian Jackson]

Wei Liu writes ("Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}"):
> On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote:
> > It's specific to guests with a device model (those are the only ones that 
> > can have an emulated CDROM device), and the only guests that can have a 
> > device model are HVM (although not all of them).
> 
> Remind me again: how does PV guest CDROM work? ISTR it will also go
> through this function? My memory is vague though...

I think an "empty" cdrom drive is represented to the guest as a
0-length block device.

There seem to be varying views as to whether insert/remove is supposed
to involve a vbd state teardown/restart, or just a capacity change.

From what I think Roger said, Linux blkback doesn't spot changes to
params (obviously - it looks for physdev) and AIUI needs to be
prodded.

Ian.

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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Roger Pau Monné]

[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]

On Fri, 8 Apr 2016, Wei Liu wrote:
> On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote:
> > On Fri, 8 Apr 2016, Wei Liu wrote:
> > > On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
> > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > > ---
> > > >  tools/libxl/libxl.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > index 9d785a4..232e2c1 100644
> > > > --- a/tools/libxl/libxl.c
> > > > +++ b/tools/libxl/libxl.c
> > > > @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
> > > >          goto out;
> > > >      }
> > > >  
> > > > +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> > > > +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
> > > 
> > > It's not specific to HVM, right?
> > 
> > It's specific to guests with a device model (those are the only ones that 
> > can have an emulated CDROM device), and the only guests that can have a 
> > device model are HVM (although not all of them).
> > 
> 
> Remind me again: how does PV guest CDROM work? ISTR it will also go
> through this function? My memory is vague though...

No, AFAICT PV guests use block-attach/detach in order to manage CDROMs, 
the usage of the cd-eject/insert commands is already limited to HVM 
guests, my check is only further limiting it to HVM guests with a device 
model (so it's not used for PVH guests).

A little above of the check that I've added:

    libxl_domain_type type = libxl__domain_type(gc, domid);
    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
        rc = ERROR_FAIL;
        goto out;
    }
    if (type != LIBXL_DOMAIN_TYPE_HVM) {
        LOG(ERROR, "cdrom-insert requires an HVM domain");
        rc = ERROR_INVAL;
        goto out;
    }

Roger.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Wei Liu]

On Fri, Apr 08, 2016 at 04:35:22PM +0200, Roger Pau Monné wrote:
> On Fri, 8 Apr 2016, Wei Liu wrote:
> > On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote:
> > > On Fri, 8 Apr 2016, Wei Liu wrote:
> > > > On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
> > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > > > ---
> > > > >  tools/libxl/libxl.c | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > > 
> > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > > index 9d785a4..232e2c1 100644
> > > > > --- a/tools/libxl/libxl.c
> > > > > +++ b/tools/libxl/libxl.c
> > > > > @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
> > > > >          goto out;
> > > > >      }
> > > > >  
> > > > > +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> > > > > +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
> > > > 
> > > > It's not specific to HVM, right?
> > > 
> > > It's specific to guests with a device model (those are the only ones that 
> > > can have an emulated CDROM device), and the only guests that can have a 
> > > device model are HVM (although not all of them).
> > > 
> > 
> > Remind me again: how does PV guest CDROM work? ISTR it will also go
> > through this function? My memory is vague though...
> 
> No, AFAICT PV guests use block-attach/detach in order to manage CDROMs, 
> the usage of the cd-eject/insert commands is already limited to HVM 
> guests, my check is only further limiting it to HVM guests with a device 
> model (so it's not used for PVH guests).
> 
> A little above of the check that I've added:
> 
>     libxl_domain_type type = libxl__domain_type(gc, domid);
>     if (type == LIBXL_DOMAIN_TYPE_INVALID) {
>         rc = ERROR_FAIL;
>         goto out;
>     }
>     if (type != LIBXL_DOMAIN_TYPE_HVM) {
>         LOG(ERROR, "cdrom-insert requires an HVM domain");
>         rc = ERROR_INVAL;
>         goto out;
>     }

Right. Thanks for checking.  I realise quibbling over an error message
seems counter-productive.

Acked-by: Wei Liu <wei.liu2@citrix.com>


Wei.


> 
> Roger.


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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Roger Pau Monné]

[-- Attachment #1: Type: text/plain, Size: 2698 bytes --]

On Fri, 8 Apr 2016, Wei Liu wrote:
> On Fri, Apr 08, 2016 at 04:35:22PM +0200, Roger Pau Monné wrote:
> > On Fri, 8 Apr 2016, Wei Liu wrote:
> > > On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote:
> > > > On Fri, 8 Apr 2016, Wei Liu wrote:
> > > > > On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
> > > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > > > > ---
> > > > > >  tools/libxl/libxl.c | 6 ++++++
> > > > > >  1 file changed, 6 insertions(+)
> > > > > > 
> > > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > > > index 9d785a4..232e2c1 100644
> > > > > > --- a/tools/libxl/libxl.c
> > > > > > +++ b/tools/libxl/libxl.c
> > > > > > @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
> > > > > >          goto out;
> > > > > >      }
> > > > > >  
> > > > > > +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> > > > > > +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
> > > > > 
> > > > > It's not specific to HVM, right?
> > > > 
> > > > It's specific to guests with a device model (those are the only ones that 
> > > > can have an emulated CDROM device), and the only guests that can have a 
> > > > device model are HVM (although not all of them).
> > > > 
> > > 
> > > Remind me again: how does PV guest CDROM work? ISTR it will also go
> > > through this function? My memory is vague though...
> > 
> > No, AFAICT PV guests use block-attach/detach in order to manage CDROMs, 
> > the usage of the cd-eject/insert commands is already limited to HVM 
> > guests, my check is only further limiting it to HVM guests with a device 
> > model (so it's not used for PVH guests).
> > 
> > A little above of the check that I've added:
> > 
> >     libxl_domain_type type = libxl__domain_type(gc, domid);
> >     if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> >         rc = ERROR_FAIL;
> >         goto out;
> >     }
> >     if (type != LIBXL_DOMAIN_TYPE_HVM) {
> >         LOG(ERROR, "cdrom-insert requires an HVM domain");
> >         rc = ERROR_INVAL;
> >         goto out;
> >     }
> 
> Right. Thanks for checking.  I realise quibbling over an error message
> seems counter-productive.
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks, I don't mind changing the message to "Guests without a device 
model cannot use cd-insert", I just felt that adding HVM would make it 
clearer, but it might not. If you or the committer prefer to change the 
message that's fine for me (and I can send a new version if requested).

Roger.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Andrew Cooper]

On 08/04/16 15:51, Roger Pau Monné wrote:
> On Fri, 8 Apr 2016, Wei Liu wrote:
>> On Fri, Apr 08, 2016 at 04:35:22PM +0200, Roger Pau Monné wrote:
>>> On Fri, 8 Apr 2016, Wei Liu wrote:
>>>> On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote:
>>>>> On Fri, 8 Apr 2016, Wei Liu wrote:
>>>>>> On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
>>>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>>>>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>>>>>>> Cc: Wei Liu <wei.liu2@citrix.com>
>>>>>>> ---
>>>>>>>  tools/libxl/libxl.c | 6 ++++++
>>>>>>>  1 file changed, 6 insertions(+)
>>>>>>>
>>>>>>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>>>>>>> index 9d785a4..232e2c1 100644
>>>>>>> --- a/tools/libxl/libxl.c
>>>>>>> +++ b/tools/libxl/libxl.c
>>>>>>> @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
>>>>>>>          goto out;
>>>>>>>      }
>>>>>>>  
>>>>>>> +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
>>>>>>> +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
>>>>>> It's not specific to HVM, right?
>>>>> It's specific to guests with a device model (those are the only ones that 
>>>>> can have an emulated CDROM device), and the only guests that can have a 
>>>>> device model are HVM (although not all of them).
>>>>>
>>>> Remind me again: how does PV guest CDROM work? ISTR it will also go
>>>> through this function? My memory is vague though...
>>> No, AFAICT PV guests use block-attach/detach in order to manage CDROMs, 
>>> the usage of the cd-eject/insert commands is already limited to HVM 
>>> guests, my check is only further limiting it to HVM guests with a device 
>>> model (so it's not used for PVH guests).
>>>
>>> A little above of the check that I've added:
>>>
>>>     libxl_domain_type type = libxl__domain_type(gc, domid);
>>>     if (type == LIBXL_DOMAIN_TYPE_INVALID) {
>>>         rc = ERROR_FAIL;
>>>         goto out;
>>>     }
>>>     if (type != LIBXL_DOMAIN_TYPE_HVM) {
>>>         LOG(ERROR, "cdrom-insert requires an HVM domain");
>>>         rc = ERROR_INVAL;
>>>         goto out;
>>>     }
>> Right. Thanks for checking.  I realise quibbling over an error message
>> seems counter-productive.
>>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
> Thanks, I don't mind changing the message to "Guests without a device 
> model cannot use cd-insert", I just felt that adding HVM would make it 
> clearer, but it might not. If you or the committer prefer to change the 
> message that's fine for me (and I can send a new version if requested).

In principle, PV guests could have a device model providing emulated CD
support.

FWIW, I prefer the "device model" wording.

~Andrew

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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[George Dunlap]

On Fri, Apr 8, 2016 at 3:59 PM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 08/04/16 15:51, Roger Pau Monné wrote:
>> On Fri, 8 Apr 2016, Wei Liu wrote:
>>> On Fri, Apr 08, 2016 at 04:35:22PM +0200, Roger Pau Monné wrote:
>>>> On Fri, 8 Apr 2016, Wei Liu wrote:
>>>>> On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote:
>>>>>> On Fri, 8 Apr 2016, Wei Liu wrote:
>>>>>>> On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
>>>>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>>>>>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>>>>>>>> Cc: Wei Liu <wei.liu2@citrix.com>
>>>>>>>> ---
>>>>>>>>  tools/libxl/libxl.c | 6 ++++++
>>>>>>>>  1 file changed, 6 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>>>>>>>> index 9d785a4..232e2c1 100644
>>>>>>>> --- a/tools/libxl/libxl.c
>>>>>>>> +++ b/tools/libxl/libxl.c
>>>>>>>> @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
>>>>>>>>          goto out;
>>>>>>>>      }
>>>>>>>>
>>>>>>>> +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
>>>>>>>> +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
>>>>>>> It's not specific to HVM, right?
>>>>>> It's specific to guests with a device model (those are the only ones that
>>>>>> can have an emulated CDROM device), and the only guests that can have a
>>>>>> device model are HVM (although not all of them).
>>>>>>
>>>>> Remind me again: how does PV guest CDROM work? ISTR it will also go
>>>>> through this function? My memory is vague though...
>>>> No, AFAICT PV guests use block-attach/detach in order to manage CDROMs,
>>>> the usage of the cd-eject/insert commands is already limited to HVM
>>>> guests, my check is only further limiting it to HVM guests with a device
>>>> model (so it's not used for PVH guests).
>>>>
>>>> A little above of the check that I've added:
>>>>
>>>>     libxl_domain_type type = libxl__domain_type(gc, domid);
>>>>     if (type == LIBXL_DOMAIN_TYPE_INVALID) {
>>>>         rc = ERROR_FAIL;
>>>>         goto out;
>>>>     }
>>>>     if (type != LIBXL_DOMAIN_TYPE_HVM) {
>>>>         LOG(ERROR, "cdrom-insert requires an HVM domain");
>>>>         rc = ERROR_INVAL;
>>>>         goto out;
>>>>     }
>>> Right. Thanks for checking.  I realise quibbling over an error message
>>> seems counter-productive.
>>>
>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>> Thanks, I don't mind changing the message to "Guests without a device
>> model cannot use cd-insert", I just felt that adding HVM would make it
>> clearer, but it might not. If you or the committer prefer to change the
>> message that's fine for me (and I can send a new version if requested).
>
> In principle, PV guests could have a device model providing emulated CD
> support.
>
> FWIW, I prefer the "device model" wording.

Since we're painting bike sheds, I'll toss in my preferred color.

This message is for the user, not for developers, and since in other
areas we talk about "PV" domains or "HVM" domains (e.g., builder="hvm"
in the config file), I think "requires an HVM domain" is probably
going to convey the most meaning.

I don't think we support PV guests having emulated devices yet; when
we do then the message will be inaccurate, but so will the check, so
they can both go away at once. :-)

 -George

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

Re: [PATCH 4/4] libxl: remove code added to use the 'phy' backend with CDROM devices

[Ian Jackson]

Roger Pau Monne writes ("[PATCH 4/4] libxl: remove code added to use the 'phy' backend with CDROM devices"):
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

This is, AFAICT, a partial revert.  The commit message ought to say
which commit (by commit hash and by title) is being reverted.

Also, like any commit, it should explain why the change is being
made...

Thanks,
Ian.

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

Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Wei Liu]

On Fri, Apr 08, 2016 at 04:51:05PM +0200, Roger Pau Monné wrote:
> On Fri, 8 Apr 2016, Wei Liu wrote:
> > On Fri, Apr 08, 2016 at 04:35:22PM +0200, Roger Pau Monné wrote:
> > > On Fri, 8 Apr 2016, Wei Liu wrote:
> > > > On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote:
> > > > > On Fri, 8 Apr 2016, Wei Liu wrote:
> > > > > > On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
> > > > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > > > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > > > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > > > > > ---
> > > > > > >  tools/libxl/libxl.c | 6 ++++++
> > > > > > >  1 file changed, 6 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > > > > index 9d785a4..232e2c1 100644
> > > > > > > --- a/tools/libxl/libxl.c
> > > > > > > +++ b/tools/libxl/libxl.c
> > > > > > > @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
> > > > > > >          goto out;
> > > > > > >      }
> > > > > > >  
> > > > > > > +    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> > > > > > > +        LOG(ERROR, "HVM guests without a device model cannot use cd-insert");
> > > > > > 
> > > > > > It's not specific to HVM, right?
> > > > > 
> > > > > It's specific to guests with a device model (those are the only ones that 
> > > > > can have an emulated CDROM device), and the only guests that can have a 
> > > > > device model are HVM (although not all of them).
> > > > > 
> > > > 
> > > > Remind me again: how does PV guest CDROM work? ISTR it will also go
> > > > through this function? My memory is vague though...
> > > 
> > > No, AFAICT PV guests use block-attach/detach in order to manage CDROMs, 
> > > the usage of the cd-eject/insert commands is already limited to HVM 
> > > guests, my check is only further limiting it to HVM guests with a device 
> > > model (so it's not used for PVH guests).
> > > 
> > > A little above of the check that I've added:
> > > 
> > >     libxl_domain_type type = libxl__domain_type(gc, domid);
> > >     if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> > >         rc = ERROR_FAIL;
> > >         goto out;
> > >     }
> > >     if (type != LIBXL_DOMAIN_TYPE_HVM) {
> > >         LOG(ERROR, "cdrom-insert requires an HVM domain");
> > >         rc = ERROR_INVAL;
> > >         goto out;
> > >     }
> > 
> > Right. Thanks for checking.  I realise quibbling over an error message
> > seems counter-productive.
> > 
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Thanks, I don't mind changing the message to "Guests without a device 
> model cannot use cd-insert", I just felt that adding HVM would make it 
> clearer, but it might not. If you or the committer prefer to change the 
> message that's fine for me (and I can send a new version if requested).
> 

I would be probably easier for you to get a branch for Ian to commit.

Wei.

> Roger.


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