On Mon, Jun 23, 2025 at 21:59:18 +0200, Peter Krempa wrote:
From: Akihiko Odaki <akihiko.odaki@daynix.com>
usb-storage is a compound device that automatically creates a USB mass storage device and a SCSI device as its backend. Unfortunately it lacks some configuration options that are usually present with a SCSI device, and cannot represent CD-ROM in particular.
Replace usb-storage with usb-bot, which can be combined with a manually created SCSI device. libvirt will configure the SCSI device in a way identical with how QEMU does for usb-storage except that now it respects a configuration option to represent CD-ROM.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/368 Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_alias.c | 20 ++++- src/qemu/qemu_capabilities.c | 3 +- src/qemu/qemu_command.c | 86 ++++++++++++++++--- src/qemu/qemu_command.h | 5 ++ src/qemu/qemu_hotplug.c | 18 ++++ src/qemu/qemu_validate.c | 38 ++++++-- tests/qemuhotplugtest.c | 4 +- ...om-usb-empty.x86_64-latest.abi-update.args | 3 +- .../disk-usb-device-model.x86_64-latest.args | 6 +- ...k-usb-device.x86_64-latest.abi-update.args | 12 ++- 10 files changed, 167 insertions(+), 28 deletions(-) ... diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e1ed8181e3..4f6a3d3414 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c ... @@ -811,6 +820,12 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, } }
+ if (rc == 0) { + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB && + disk->model == VIR_DOMAIN_DISK_MODEL_USB_BOT) + rc = qemuMonitorSetUSBDiskAttached(priv->mon, disk->info.alias); + }
Why not just if (rc == 0 && disk->bus == ... && disk->model == ...)
+ qemuDomainObjExitMonitor(vm);
if (rc < 0) @@ -822,6 +837,9 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0) return -1;
+ if (busAdded) + ignore_value(qemuMonitorDelDevice(priv->mon, disk->info.alias)); + if (extensionDeviceAttached) ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &disk->info));
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7e0e4db516..4b8d4fc692 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3183,16 +3183,40 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, break;
case VIR_DOMAIN_DISK_BUS_USB: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support '-device usb-storage'")); + switch (disk->model) { + case VIR_DOMAIN_DISK_MODEL_DEFAULT: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("USB disk model was not selected by selection code"));
This should only be possible if neither usb-storage nor usb-bot is supported by QEMU, shouldn't it? In that case, shouldn't the selection code itself report the error and actually be explicit about the failure?
return -1; - }
- if (disk->model == VIR_DOMAIN_DISK_MODEL_USB_STORAGE && - virStorageSourceIsEmpty(disk->src)) { + case VIR_DOMAIN_DISK_MODEL_USB_STORAGE: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support '-device usb-storage'")); + return -1; + } + + if (virStorageSourceIsEmpty(disk->src)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'usb' disk must not be empty")); + return -1; + } + break; + + case VIR_DOMAIN_DISK_MODEL_USB_BOT: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_BOT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support '-device usb-bot'")); + return -1; + } + break; + + case VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL: + case VIR_DOMAIN_DISK_MODEL_VIRTIO: + case VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL: + case VIR_DOMAIN_DISK_MODEL_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'usb' disk must not be empty")); + _("USB disk supports only models: 'usb-storage', 'usb-bot''"));
s/''/'/ And the error message looks weird to me, how about s/models/the following models/ ?
return -1; }
... Reviewed-by: Jiri Denemark <jdenemar@redhat.com>