On Tue, Aug 13, 2013 at 01:52:33PM +0200, Fred A. Kemp wrote:
From: "Fred A. Kemp" <anonym@riseup.net>
Allow use of the usb-storage device only if the new capability flag QEMU_CAPS_DEVICE_USB_STORAGE is set, which it is for qemu(-kvm) versions >= 0.12.1.2-rhel62-beta. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 6 +++--- tests/qemuhelptest.c | 18 ++++++++++++------ tests/qemuxml2argvtest.c | 3 ++- 5 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 47cc07a..5c566c1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -235,6 +235,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vnc-share-policy", /* 150 */ "device-del-event", "dmi-to-pci-bridge", + "usb-storage", );
struct _virQEMUCaps { @@ -1383,6 +1384,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "vfio-pci", QEMU_CAPS_DEVICE_VFIO_PCI }, { "scsi-generic", QEMU_CAPS_DEVICE_SCSI_GENERIC }, { "i82801b11-bridge", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE }, + { "usb-storage", QEMU_CAPS_DEVICE_USB_STORAGE }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 074e55d..4a8b14b 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -191,6 +191,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */ QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */ + QEMU_CAPS_DEVICE_USB_STORAGE = 153, /* -device usb-storage */
QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b811e1d..03fb798 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8044,10 +8044,10 @@ qemuBuildCommandLine(virConnectPtr conn, bool withDeviceArg = false; bool deviceFlagMasked = false;
- /* Unless we have -device, then USB disks need special - handling */ + /* Unless we have `-device usb-storage`, then USB disks + need special handling */ if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { virCommandAddArg(cmd, "-usbdevice"); virCommandAddArgFormat(cmd, "disk:%s", disk->src);
Hmm, I'm not sure this logic change is right. Previously if you have -device support, but 'usb-storage' was not available, libvirt would try to start the guest with -device & then QEMU would report an error. With this change, if you have -device support, but 'usb-storage' was not available, then libvirt is going to fallback to using the legacy '-usbdevice' support. This is not good. Adding an explicit check for 'usb-storage' is a fine goal, but we should be doing that check in the branch of this if() that handles '-device usb-storage', so we don't exercise the -usbdevice branch Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|