Re: [PATCH v2 1/3] libxl: Create empty file for Phy cdrom

2024-02-13 Thread Anthony PERARD
On Thu, Feb 01, 2024 at 04:40:02PM -0500, Jason Andryuk wrote:
> With a device model stubdom, dom0 exports a PV disk to the stubdom.
> Inside the stubdom, QEMU emulates a cdrom to the guest with a
> host_device pointing at the PV frontend (/dev/xvdc)
> 
> An empty cdrom drive causes problems booting the stubdom.  The PV disk
> protocol isn't designed to support no media.  That can be partially
> hacked around, but the stubdom kernel waits for all block devices to
> transition to Connected.  Since the backend never connects empty media,
> stubdom launch times out and it is destroyed.
> 
> Empty media and the PV disks not connecting is fine at runtime since the
> stubdom keeps running irrespective of the disk state.
> 
> Empty media can be worked around my providing an empty file to the
> stubdom for the PV disk source.  This works as the disk is exposed as a
> zero-size disk.  Dynamically create the empty file as needed and remove
> in the stubdom cleanup.
> 
> libxl__device_disk_set_backend() needs to allow through these "empty"
> disks with a pdev_path.
> 
> Fixup the params writing since scripts have trouble with an empty params
> field.
> 
> This works for non-stubdom HVMs as well.
> 
> Signed-off-by: Jason Andryuk 

Reviewed-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD



[PATCH v2 1/3] libxl: Create empty file for Phy cdrom

2024-02-01 Thread Jason Andryuk
With a device model stubdom, dom0 exports a PV disk to the stubdom.
Inside the stubdom, QEMU emulates a cdrom to the guest with a
host_device pointing at the PV frontend (/dev/xvdc)

An empty cdrom drive causes problems booting the stubdom.  The PV disk
protocol isn't designed to support no media.  That can be partially
hacked around, but the stubdom kernel waits for all block devices to
transition to Connected.  Since the backend never connects empty media,
stubdom launch times out and it is destroyed.

Empty media and the PV disks not connecting is fine at runtime since the
stubdom keeps running irrespective of the disk state.

Empty media can be worked around my providing an empty file to the
stubdom for the PV disk source.  This works as the disk is exposed as a
zero-size disk.  Dynamically create the empty file as needed and remove
in the stubdom cleanup.

libxl__device_disk_set_backend() needs to allow through these "empty"
disks with a pdev_path.

Fixup the params writing since scripts have trouble with an empty params
field.

This works for non-stubdom HVMs as well.

Signed-off-by: Jason Andryuk 
---
v2:
New to support "empty" cdroms
---
 tools/libs/light/libxl_device.c   |  5 -
 tools/libs/light/libxl_disk.c | 36 +++
 tools/libs/light/libxl_domain.c   |  4 
 tools/libs/light/libxl_internal.h |  1 +
 4 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/tools/libs/light/libxl_device.c b/tools/libs/light/libxl_device.c
index 13da6e0573..09d85928d7 100644
--- a/tools/libs/light/libxl_device.c
+++ b/tools/libs/light/libxl_device.c
@@ -421,7 +421,10 @@ int libxl__device_disk_set_backend(libxl__gc *gc, 
libxl_device_disk *disk) {
 LOG(ERROR, "Disk vdev=%s is empty but not cdrom", disk->vdev);
 return ERROR_INVAL;
 }
-if (disk->pdev_path != NULL && strcmp(disk->pdev_path, "")) {
+if (disk->pdev_path != NULL &&
+(strcmp(disk->pdev_path, "") &&
+ strncmp(disk->pdev_path, LIBXL_STUBDOM_EMPTY_CDROM,
+ strlen(LIBXL_STUBDOM_EMPTY_CDROM {
 LOG(ERROR,
 "Disk vdev=%s is empty but an image has been provided: %s",
 disk->vdev, disk->pdev_path);
diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
index ea3623dd6f..c48e1de659 100644
--- a/tools/libs/light/libxl_disk.c
+++ b/tools/libs/light/libxl_disk.c
@@ -199,6 +199,32 @@ static int libxl__device_disk_setdefault(libxl__gc *gc, 
uint32_t domid,
 disk->backend = LIBXL_DISK_BACKEND_QDISK;
 }
 
+if (disk->is_cdrom &&
+disk->format == LIBXL_DISK_FORMAT_EMPTY &&
+disk->backend == LIBXL_DISK_BACKEND_PHY &&
+disk->backend_domid == LIBXL_TOOLSTACK_DOMID) {
+uint32_t target_domid;
+int fd;
+
+if (libxl_is_stubdom(CTX, domid, _domid)) {
+LOGED(DEBUG, domid, "Using target_domid %u", target_domid);
+} else {
+target_domid = domid;
+}
+free(disk->pdev_path);
+disk->pdev_path =
+libxl__sprintf(NOGC, LIBXL_STUBDOM_EMPTY_CDROM ".%u",
+   target_domid);
+fd = creat(disk->pdev_path, 0400);
+if (fd < 0) {
+LOGED(ERROR, domid, "Failed to create empty cdrom \"%s\"",
+  disk->pdev_path);
+return ERROR_FAIL;
+}
+
+close(fd);
+}
+
 rc = libxl__device_disk_set_backend(gc, disk);
 return rc;
 }
@@ -988,7 +1014,7 @@ static void cdrom_insert_ejected(libxl__egc *egc,
 empty = flexarray_make(gc, 4, 1);
 flexarray_append_pair(empty, "type",
   libxl__device_disk_string_of_backend(disk->backend));
-flexarray_append_pair(empty, "params", "");
+flexarray_append_pair(empty, "params", disk->pdev_path ?: "");
 
 for (;;) {
 rc = libxl__xs_transaction_start(gc, );
@@ -1164,13 +1190,15 @@ static void cdrom_insert_inserted(libxl__egc *egc,
 insert = flexarray_make(gc, 4, 1);
 flexarray_append_pair(insert, "type",
   libxl__device_disk_string_of_backend(disk->backend));
-if (disk->format != LIBXL_DISK_FORMAT_EMPTY)
+if (disk->backend == LIBXL_DISK_BACKEND_QDISK &&
+disk->format != LIBXL_DISK_FORMAT_EMPTY) {
 flexarray_append_pair(insert, "params",
 GCSPRINTF("%s:%s",
 libxl__device_disk_string_of_format(disk->format),
 disk->pdev_path));
-else
-flexarray_append_pair(insert, "params", "");
+} else {
+flexarray_append_pair(insert, "params", disk->pdev_path ?: "");
+}
 
 for (;;) {
 rc = libxl__xs_transaction_start(gc, );
diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 5ee1544d9c..6751fc785f 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -1525,6 +1525,10 @@ static void