diff mbox series

[v2,2/3] libxl: Allow Phy backend for CDROM devices

Message ID 20240201214004.238858-3-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
A Linux HVM domain ignores PV block devices with type cdrom.  The
Windows PV drivers also ignore device-type != "disk".  Therefore QEMU's
emulated CD-ROM support is used.  This allows ejection and other CD-ROM
features to work.

With a stubdom, QEMU is running in the stubdom.  A PV disk is still
connected into the stubdom, and then QEMU can emulate the CD-ROM into
the guest.  Phy support has been enhanced to provide a placeholder file
forempty disks, so it is usable as a CDROM backend as well.  Allow Phy
to pass the check as well.

(Bypassing just for a linux-based stubdom doesn't work because
libxl__device_disk_setdefault() gets called early in domain creation
before xenstore is populated with relevant information for the stubdom
type.  The build information isn't readily available and won't exist in
some call trees, so it isn't usable either.)

Let disk_try_backend() allow format empty for Phy cdrom drives.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
v2:
Different approach to pass QDISK requirement check.
---
 tools/libs/light/libxl_device.c | 12 ++++++++----
 tools/libs/light/libxl_disk.c   | 11 +++++++----
 2 files changed, 15 insertions(+), 8 deletions(-)

Comments

Anthony PERARD Feb. 13, 2024, 11:42 a.m. UTC | #1
On Thu, Feb 01, 2024 at 04:40:03PM -0500, Jason Andryuk wrote:
> A Linux HVM domain ignores PV block devices with type cdrom.  The
> Windows PV drivers also ignore device-type != "disk".  Therefore QEMU's
> emulated CD-ROM support is used.  This allows ejection and other CD-ROM
> features to work.
> 
> With a stubdom, QEMU is running in the stubdom.  A PV disk is still
> connected into the stubdom, and then QEMU can emulate the CD-ROM into
> the guest.  Phy support has been enhanced to provide a placeholder file
> forempty disks, so it is usable as a CDROM backend as well.  Allow Phy
> to pass the check as well.
> 
> (Bypassing just for a linux-based stubdom doesn't work because
> libxl__device_disk_setdefault() gets called early in domain creation
> before xenstore is populated with relevant information for the stubdom
> type.  The build information isn't readily available and won't exist in
> some call trees, so it isn't usable either.)
> 
> Let disk_try_backend() allow format empty for Phy cdrom drives.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,
diff mbox series

Patch

diff --git a/tools/libs/light/libxl_device.c b/tools/libs/light/libxl_device.c
index 09d85928d7..f73c6705d4 100644
--- a/tools/libs/light/libxl_device.c
+++ b/tools/libs/light/libxl_device.c
@@ -301,13 +301,17 @@  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) {
-            goto bad_format;
-        }
-
         if (libxl_defbool_val(a->disk->colo_enable))
             goto bad_colo;
 
+        if (a->disk->is_cdrom && a->disk->format == LIBXL_DISK_FORMAT_EMPTY) {
+            LOG(DEBUG, "Disk vdev=%s is an empty cdrom", a->disk->vdev);
+            return backend;
+        }
+
+        if (a->disk->format != LIBXL_DISK_FORMAT_RAW)
+            goto bad_format;
+
         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);
diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
index c48e1de659..09082ffb58 100644
--- a/tools/libs/light/libxl_disk.c
+++ b/tools/libs/light/libxl_disk.c
@@ -188,15 +188,18 @@  static int libxl__device_disk_setdefault(libxl__gc *gc, uint32_t domid,
         return ERROR_FAIL;
     }
 
-    /* Force Qdisk backend for CDROM devices of guests with a device model. */
+    /* Only allow Qdisk or Phy for CDROM devices. */
     if (disk->is_cdrom != 0 &&
         libxl__domain_type(gc, domid) == LIBXL_DOMAIN_TYPE_HVM) {
+        if (disk->backend == LIBXL_DISK_BACKEND_UNKNOWN)
+            disk->backend = LIBXL_DISK_BACKEND_QDISK;
+
         if (!(disk->backend == LIBXL_DISK_BACKEND_QDISK ||
-              disk->backend == LIBXL_DISK_BACKEND_UNKNOWN)) {
-            LOGD(ERROR, domid, "Backend for CD devices on HVM guests must be Qdisk");
+              disk->backend == LIBXL_DISK_BACKEND_PHY)) {
+            LOGD(ERROR, domid,
+                 "Backend for CD devices on HVM guests must be Qdisk or Phy");
             return ERROR_FAIL;
         }
-        disk->backend = LIBXL_DISK_BACKEND_QDISK;
     }
 
     if (disk->is_cdrom &&