[PATCH 2/3] qemu: Replace usb-storage with usb-bot

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(a)daynix.com&gt; --- src/qemu/qemu_alias.c | 3 +- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 55 ++++++++++++++++++++-- src/qemu/qemu_command.h | 5 ++ src/qemu/qemu_hotplug.c | 34 ++++++++++--- src/qemu/qemu_validate.c | 4 +- tests/qemuhotplugtest.c | 8 +++- .../qemuxmlconfdata/disk-cache.x86_64-latest.args | 3 +- .../disk-cdrom-bus-other.x86_64-latest.args | 3 +- .../disk-device-removable.x86_64-latest.args | 3 +- .../disk-usb-device.x86_64-latest.args | 3 +- 11 files changed, 102 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 3e6bced4a88538a040dd8a65f40dc9f7f56b8ec9..64986368d0588f7778c8e3a09ae3169c1961b456 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -267,8 +267,7 @@ qemuAssignDeviceDiskAlias(virDomainDef *def, break; case VIR_DOMAIN_DISK_BUS_USB: - diskPriv->qomName = g_strdup_printf("/machine/peripheral/%s/%s.0/legacy[0]", - disk->info.alias, disk->info.alias); + diskPriv->qomName = g_strconcat("scsi", disk->info.alias, NULL); break; case VIR_DOMAIN_DISK_BUS_XEN: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2c7f9cfe994c45dd3bbecc9cd9fff51889adfb0f..b253cb407e8334320cc8ecd49d28d2481d82e9b4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -6477,7 +6477,7 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCaps *qemuCaps, VIR_DOMAIN_DISK_BUS_VIRTIO, /* VIR_DOMAIN_DISK_BUS_SD */); - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_BOT)) VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_USB); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0ad73af335974a236341fa85d02c83f14af08934..29db916e70804ec482392c078a280904a992eaf3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1611,6 +1611,31 @@ qemuBuildIothreadMappingProps(GSList *iothreads) return g_steal_pointer(&ret); } +int +qemuBuildDiskBusProps(const virDomainDef *def, + const virDomainDiskDef *disk, + virJSONValue **propsRet) +{ + g_autoptr(virJSONValue) props = NULL; + + *propsRet = NULL; + + if (disk->bus != VIR_DOMAIN_DISK_BUS_USB) + return 0; + + if (virJSONValueObjectAdd(&props, + "s:driver", "usb-bot", + "s:id", disk->info.alias, + NULL) < 0) + return -1; + + if (qemuBuildDeviceAddressProps(props, def, &disk->info) < 0) + return -1; + + *propsRet = g_steal_pointer(&props); + + return 0; +} virJSONValue * qemuBuildDiskDeviceProps(const virDomainDef *def, @@ -1619,6 +1644,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, { g_autoptr(virJSONValue) props = NULL; const char *driver = NULL; + g_autofree char *usbBusId = NULL; g_autofree char *scsiVPDDeviceId = NULL; virTristateSwitch shareRW = VIR_TRISTATE_SWITCH_ABSENT; g_autofree char *chardev = NULL; @@ -1637,6 +1663,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, const char *rpolicy = NULL; const char *model = NULL; const char *product = NULL; + const char *id = disk->info.alias; switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_IDE: @@ -1650,6 +1677,17 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, break; + case VIR_DOMAIN_DISK_BUS_USB: + usbBusId = g_strconcat(disk->info.alias, ".0", NULL); + if (virJSONValueObjectAdd(&props, + "s:bus", usbBusId, + "u:scsi-id", 0, + "u:lun", 0, + NULL) < 0) + return NULL; + + id = QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName; + G_GNUC_FALLTHROUGH; case VIR_DOMAIN_DISK_BUS_SCSI: if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { driver = "scsi-block"; @@ -1719,8 +1757,6 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, } break; - case VIR_DOMAIN_DISK_BUS_USB: - driver = "usb-storage"; if (disk->removable == VIR_TRISTATE_SWITCH_ABSENT) removable = VIR_TRISTATE_SWITCH_OFF; @@ -1755,7 +1791,8 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) disk->info.addr.drive.diskbus = disk->bus; - if (qemuBuildDeviceAddressProps(props, def, &disk->info) < 0) + if (disk->bus != VIR_DOMAIN_DISK_BUS_USB && + qemuBuildDeviceAddressProps(props, def, &disk->info) < 0) return NULL; if (disk->src->shared) @@ -1816,7 +1853,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "T:share-rw", shareRW, "S:drive", drive, "S:chardev", chardev, - "s:id", disk->info.alias, + "s:id", id, "p:bootindex", bootindex, "S:loadparm", bootLoadparm, "p:logical_block_size", logical_block_size, @@ -2110,6 +2147,16 @@ qemuBuildDiskCommandLine(virCommand *cmd, if (qemuCommandAddExtDevice(cmd, &disk->info, def, qemuCaps) < 0) return -1; + if (qemuBuildDiskBusProps(def, disk, &devprops) < 0) + return -1; + + if (devprops) { + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) + return -1; + + g_clear_pointer(&devprops, virJSONValueFree); + } + if (!(devprops = qemuBuildDiskDeviceProps(def, disk, qemuCaps))) return -1; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 76c514b5f744c60e433f0d8d8760a8730c886eed..48b8ed8ac58d005503c8bdb7e255af097a548fd9 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -116,6 +116,11 @@ qemuBlockStorageSourceChainData * qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSource *top, virStorageSource *backingStore); +int +qemuBuildDiskBusProps(const virDomainDef *def, + const virDomainDiskDef *disk, + virJSONValue **propsRet); + virJSONValue * qemuBuildDiskDeviceProps(const virDomainDef *def, virDomainDiskDef *disk, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 28ca321c5c194678d25d67aa02ec82c13175f4f5..cfbaf195bd0a8991c8902795560bda3ee85e42bc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -31,6 +31,7 @@ #include "qemu_command.h" #include "qemu_hostdev.h" #include "qemu_interface.h" +#include "qemu_monitor_json.h" #include "qemu_passt.h" #include "qemu_process.h" #include "qemu_security.h" @@ -709,8 +710,10 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, { g_autoptr(qemuBlockStorageSourceChainData) data = NULL; qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(virJSONValue) busprops = NULL; g_autoptr(virJSONValue) devprops = NULL; bool extensionDeviceAttached = false; + bool busAdded = false; int rc; g_autoptr(qemuSnapshotDiskContext) transientDiskSnapshotCtxt = NULL; bool origReadonly = disk->src->readonly; @@ -766,6 +769,9 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, } } + if (qemuBuildDiskBusProps(vm->def, disk, &busprops) < 0) + goto rollback; + if (!(devprops = qemuBuildDiskDeviceProps(vm->def, disk, priv->qemuCaps))) goto rollback; @@ -775,16 +781,20 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, if ((rc = qemuDomainAttachExtensionDevice(priv->mon, &disk->info)) == 0) extensionDeviceAttached = true; + if (rc == 0 && busprops && + (rc = qemuMonitorAddDeviceProps(priv->mon, &busprops)) == 0) + busAdded = true; + if (rc == 0) rc = qemuMonitorAddDeviceProps(priv->mon, &devprops); - /* Setup throttling of disk via block_set_io_throttle QMP command. This - * is a hack until the 'throttle' blockdev driver will support modification - * of the trhottle group. See also qemuProcessSetupDiskThrottlingBlockdev. - * As there isn't anything sane to do if this fails, let's just return - * success. - */ if (rc == 0) { + /* Setup throttling of disk via block_set_io_throttle QMP command. This + * is a hack until the 'throttle' blockdev driver will support modification + * of the trhottle group. See also qemuProcessSetupDiskThrottlingBlockdev. + * As there isn't anything sane to do if this fails, let's just return + * success. + */ qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); g_autoptr(GHashTable) blockinfo = NULL; @@ -801,6 +811,15 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, qemuProcessRefreshDiskProps(disk, diskinfo); } } + + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + qemuMonitorJSONObjectProperty prop = { + .type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN, + .val.b = true, + }; + + rc = qemuMonitorJSONSetObjectProperty(priv->mon, disk->info.alias, "attached", &prop); + } } qemuDomainObjExitMonitor(vm); @@ -814,6 +833,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 f3ef1be660b1cbbfe8ed5643baf2e9f4a63cf60d..f9ec3a7c076c3755e1c3ddf3c63aeeb7ed576f2b 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3146,9 +3146,9 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, break; case VIR_DOMAIN_DISK_BUS_USB: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_BOT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support '-device usb-storage'")); + _("This QEMU doesn't support '-device usb-bot'")); return -1; } diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d2a1f5acf1c17236f0cc038fda43e4f3052cc28b..96ab222836f2594ce7dbd88236ebfe704c18838b 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -590,7 +590,9 @@ mymain(void) DO_TEST_ATTACH("x86_64", "base-live", "disk-usb", false, true, "blockdev-add", QMP_OK, "device_add", QMP_OK, - "query-block", QMP_EMPTY_ARRAY); + "device_add", QMP_OK, + "query-block", QMP_EMPTY_ARRAY, + "qom-set", QMP_OK); DO_TEST_DETACH("x86_64", "base-live", "disk-usb", true, true, "device_del", QMP_OK); DO_TEST_DETACH("x86_64", "base-live", "disk-usb", false, false, @@ -746,7 +748,9 @@ mymain(void) DO_TEST_ATTACH("x86_64", "base-live", "cdrom-usb", false, true, "blockdev-add", QMP_OK, "device_add", QMP_OK, - "query-block", QMP_EMPTY_ARRAY); + "device_add", QMP_OK, + "query-block", QMP_EMPTY_ARRAY, + "qom-set", QMP_OK); DO_TEST_DETACH("x86_64", "base-live", "cdrom-usb", true, true, "device_del", QMP_OK); DO_TEST_DETACH("x86_64", "base-live", "cdrom-usb", false, false, diff --git a/tests/qemuxmlconfdata/disk-cache.x86_64-latest.args b/tests/qemuxmlconfdata/disk-cache.x86_64-latest.args index 27be6441774b13dc0c491328431aed6ccd3eddaa..206f8ab9ca46008ff1895730159838aa7dd1e018 100644 --- a/tests/qemuxmlconfdata/disk-cache.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-cache.x86_64-latest.args @@ -42,7 +42,8 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-2-format","id":"virtio-disk0","write-cache":"off"}' \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"qcow2","file":"libvirt-1-storage"}' \ --device '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-1-format","id":"usb-disk1","removable":false,"write-cache":"off"}' \ +-device '{"driver":"usb-bot","id":"usb-disk1","bus":"usb.0","port":"1"}' \ +-device '{"bus":"usb-disk1.0","scsi-id":0,"lun":0,"driver":"scsi-hd","device_id":"drive-usb-disk1","drive":"libvirt-1-format","id":"scsiusb-disk1","write-cache":"off"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on diff --git a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args b/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args index e1406af663625bbb1c27203450bc0135af3861e7..8e5c60ac1ec185d1b717b3b22ceb07b721b54846 100644 --- a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args @@ -28,7 +28,8 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -boot strict=on \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ -blockdev '{"driver":"file","filename":"/root/boot.iso","node-name":"libvirt-1-storage","read-only":true}' \ --device '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-1-storage","id":"usb-disk0","removable":false}' \ +-device '{"driver":"usb-bot","id":"usb-disk0","bus":"usb.0","port":"1"}' \ +-device '{"bus":"usb-disk0.0","scsi-id":0,"lun":0,"driver":"scsi-cd","device_id":"drive-usb-disk0","drive":"libvirt-1-storage","id":"scsiusb-disk0"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on diff --git a/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.args b/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.args index e0701f4bd25d79b60dc4f9294a603d0a7cd9f0b1..d9b259bc59cc54b10ede57e696e553b3d5f2ef06 100644 --- a/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.args @@ -31,7 +31,8 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-3-storage","read-only":false}' \ -device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-3-storage","id":"ide0-0-0","bootindex":1}' \ -blockdev '{"driver":"file","filename":"/tmp/usbdisk.img","node-name":"libvirt-2-storage","read-only":false}' \ --device '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-2-storage","id":"usb-disk0","removable":true}' \ +-device '{"driver":"usb-bot","id":"usb-disk0","bus":"usb.0","port":"1"}' \ +-device '{"bus":"usb-disk0.0","scsi-id":0,"lun":0,"driver":"scsi-hd","device_id":"drive-usb-disk0","drive":"libvirt-2-storage","id":"scsiusb-disk0","removable":true}' \ -blockdev '{"driver":"file","filename":"/tmp/scsidisk.img","node-name":"libvirt-1-storage","read-only":false}' \ -device '{"driver":"scsi-hd","bus":"scsi0.0","channel":0,"scsi-id":0,"lun":1,"device_id":"drive-scsi0-0-0-1","drive":"libvirt-1-storage","id":"scsi0-0-0-1","removable":true}' \ -audiodev '{"id":"audio1","driver":"none"}' \ diff --git a/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.args b/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.args index 0fd7e755b1384227b63d6a080bd6af947596ae06..534780faf1a34af1a60dcca4dc71f91943267ea9 100644 --- a/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.args @@ -30,7 +30,8 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-2-storage","read-only":false}' \ -device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-2-storage","id":"ide0-0-0","bootindex":1}' \ -blockdev '{"driver":"file","filename":"/tmp/usbdisk.img","node-name":"libvirt-1-storage","read-only":false}' \ --device '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-1-storage","id":"usb-disk0","removable":false}' \ +-device '{"driver":"usb-bot","id":"usb-disk0","bus":"usb.0","port":"1"}' \ +-device '{"bus":"usb-disk0.0","scsi-id":0,"lun":0,"driver":"scsi-hd","device_id":"drive-usb-disk0","drive":"libvirt-1-storage","id":"scsiusb-disk0"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -- 2.48.1