[libvirt] [PATCH 0/2] Support setting the 'removable' flag for USB disks
From: "Fred A. Kemp" <anonym@riseup.net> The commit message of patch #2 explains the purpose of this patch set. A review would be greatly appreciated! Note that I've only added the new capability for usb-storage.removable to the qemu help tests of qemu(-kvm) version 1.2.0, since that's what I had easily available to get the output of `-device usb-storage,?` from. I hope that's not an issue, otherwise, is there a way to obtain these outputs without having to hunt down and install all supported versions? Previous submissions of this patch set to this list: http://www.redhat.com/archives/libvir-list/2013-March/msg01051.html http://www.redhat.com/archives/libvir-list/2013-May/msg02039.html https://www.redhat.com/archives/libvir-list/2013-July/msg01635.html Fred A. Kemp (2): qemu: Add capability flag for usb-storage qemu: Support setting the 'removable' flag for USB disks docs/formatdomain.html.in | 8 +++-- docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 31 ++++++++++++++++++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 10 +++++++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 23 +++++++++++++-- tests/qemuhelpdata/qemu-1.2.0-device | 11 +++++++ tests/qemuhelpdata/qemu-kvm-1.2.0-device | 11 +++++++ tests/qemuhelptest.c | 20 +++++++++---- .../qemuxml2argv-disk-usb-device-removable.args | 8 +++++ .../qemuxml2argv-disk-usb-device-removable.xml | 27 +++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++- 13 files changed, 151 insertions(+), 15 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml -- 1.7.10.4
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); diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index d6cc04b..cbabe12 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -514,7 +514,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_SERIAL, QEMU_CAPS_DEVICE_USB_NET, QEMU_CAPS_DEVICE_PCI_BRIDGE, - QEMU_CAPS_DEVICE_SCSI_GENERIC); + QEMU_CAPS_DEVICE_SCSI_GENERIC, + QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -653,7 +654,8 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_DEVICE_PCI_BRIDGE); + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST("qemu-1.0", 1000000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -736,7 +738,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_SERIAL, QEMU_CAPS_DEVICE_USB_NET, QEMU_CAPS_DEVICE_SCSI_GENERIC, - QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX); + QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, + QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST("qemu-1.1.0", 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -831,7 +834,8 @@ mymain(void) QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, - QEMU_CAPS_VNC_SHARE_POLICY); + QEMU_CAPS_VNC_SHARE_POLICY, + QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST("qemu-1.2.0", 1002000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -937,7 +941,8 @@ mymain(void) QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, - QEMU_CAPS_VNC_SHARE_POLICY); + QEMU_CAPS_VNC_SHARE_POLICY, + QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST("qemu-kvm-1.2.0", 1002000, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -1048,7 +1053,8 @@ mymain(void) QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, - QEMU_CAPS_VNC_SHARE_POLICY); + QEMU_CAPS_VNC_SHARE_POLICY, + QEMU_CAPS_DEVICE_USB_STORAGE); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 679124e..2f993ce 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -540,7 +540,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_BOOTINDEX); DO_TEST("disk-usb", NONE); DO_TEST("disk-usb-device", - QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_USB_STORAGE, + QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-scsi-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); -- 1.7.10.4
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 :|
13/08/13 16:15, Daniel P. Berrange wrote:
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.
I fail to see why this is not good. I thought '-usbdevice' was the correct way do handle USB disks if 'usb-storage' is missing. Whether we have '-device' or seems beside the point; if we have 'usb-storage', then it's implied we have '-device', and if we don't, '-device' is worthless and '-usbdevice' is the way to go. Letting QEMU fail and die when we have '-device' but not 'usb-storage' seems worse than just falling back to '-usbdevice'. What am I missing?
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
This doesn't make sense to me either, but I guess it will after clearing up my previous confusion. Cheers!
On Thu, Aug 15, 2013 at 12:55:26PM +0200, anonym wrote:
13/08/13 16:15, Daniel P. Berrange wrote:
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.
I fail to see why this is not good. I thought '-usbdevice' was the correct way do handle USB disks if 'usb-storage' is missing. Whether we have '-device' or seems beside the point; if we have 'usb-storage', then it's implied we have '-device', and if we don't, '-device' is worthless and '-usbdevice' is the way to go. Letting QEMU fail and die when we have '-device' but not 'usb-storage' seems worse than just falling back to '-usbdevice'.
What am I missing?
If -device exists, we must *never* use the -usbdevice syntax. This is a legacy syntax that is only there for compatibility with apps that predate the introduction of -device syntax. If the 'usb-storage' device does not exist, then '-usbdevice disk' is not going to work either since it uses the same code internally.
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
This doesn't make sense to me either, but I guess it will after clearing up my previous confusion.
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 :|
15/08/13 12:58, Daniel P. Berrange wrote:
On Thu, Aug 15, 2013 at 12:55:26PM +0200, anonym wrote:
13/08/13 16:15, Daniel P. Berrange wrote:
On Tue, Aug 13, 2013 at 01:52:33PM +0200, Fred A. Kemp wrote:
From: "Fred A. Kemp" <anonym@riseup.net>
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.
I fail to see why this is not good. I thought '-usbdevice' was the correct way do handle USB disks if 'usb-storage' is missing. Whether we have '-device' or seems beside the point; if we have 'usb-storage', then it's implied we have '-device', and if we don't, '-device' is worthless and '-usbdevice' is the way to go. Letting QEMU fail and die when we have '-device' but not 'usb-storage' seems worse than just falling back to '-usbdevice'.
What am I missing?
If -device exists, we must *never* use the -usbdevice syntax. This is a legacy syntax that is only there for compatibility with apps that predate the introduction of -device syntax.
Without knowing the exact development history of qemu, I assumed it could be the case the we have '-device' but 'usb-storage' hadn't been implemented yet (so '-usbdevice' was still the way to go).
If the 'usb-storage' device does not exist, then '-usbdevice disk' is not going to work either since it uses the same code internally.
Note that the QEMU_CAPS_DEVICE_USB_STORAGE capability I add only checks if '-device usb-storage' is supported (by parsing 'qemu -device ?' like other, similar caps) and has nothing to do with '-usbdevice'.
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
So, what if I drop the above chunk, and do the following instead: --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8046,18 +8046,26 @@ qemuBuildCommandLine(virConnectPtr conn, /* Unless we have -device, then USB disks need special handling */ - if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src); + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support '-device " + "usb-storage'")); + goto error; + } } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported usb disk type for '%s'"), - disk->src); - goto error; + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArgFormat(cmd, "disk:%s", disk->src); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported usb disk type for '%s'"), + disk->src); + goto error; + } + continue; } - continue; } switch (disk->device) { Alternatively, I could do the following instead, which is more concise, but doesn't happen in the same if() and thus spread the capability checking over a larger code area: --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4311,13 +4311,6 @@ qemuBuildDriveDevStr(virDomainDefPtr def, goto error; 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'")); + goto error; + + } virBufferAddLit(&opt, "usb-storage"); if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0) Once I have an opinion (or further explanation why I'm still confused :)) I'll re-submit a new patchset with the preferred fix, rebased on the then-current master. Cheers!
On 08/19/2013 08:00 AM, anonym wrote:
Without knowing the exact development history of qemu, I assumed it could be the case the we have '-device' but 'usb-storage' hadn't been implemented yet (so '-usbdevice' was still the way to go).
No, if we have -device, the we MUST use it. Fallback to older code is possible only if -device is missing.
So, what if I drop the above chunk, and do the following instead:
--- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8046,18 +8046,26 @@ qemuBuildCommandLine(virConnectPtr conn,
/* Unless we have -device, then USB disks need special handling */ - if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src); + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support '-device " + "usb-storage'")); + goto error; + }
This looks okay...
} else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported usb disk type for '%s'"), - disk->src); - goto error; + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArgFormat(cmd, "disk:%s", disk->src);
...but this still looks fishy (-device is so old that we are probably safer rejecting attempts to use a usb on a qemu that lacked -device than we are trying to figure out if -usbdevice worked for something that old - no one targetting a qemu that old will be trying to use new features anyway).
Alternatively, I could do the following instead, which is more concise, but doesn't happen in the same if() and thus spread the capability checking over a larger code area:
--- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4311,13 +4311,6 @@ qemuBuildDriveDevStr(virDomainDefPtr def, goto error; 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'")); + goto error; + + }
That actually looks a bit nicer, even if it does spread the capability check across a wider set of code.
virBufferAddLit(&opt, "usb-storage");
if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0)
Once I have an opinion (or further explanation why I'm still confused :)) I'll re-submit a new patchset with the preferred fix, rebased on the then-current master.
When resubmitting, send as a top-level patch (no in-reply-to) so it isn't buried, and use 'git send-email --subject-prefix=PATCHv2' or similar to make it obvious in the title that it is a rebased series. Good luck. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Fred A. Kemp" <anonym@riseup.net> Add an attribute named 'removable' to the 'target' element of disks, which controls the removable flag. For instance, on a Linux guest it controls the value of /sys/block/$dev/removable. This option is only valid for USB disks (i.e. bus='usb'), and its default value is 'off', which is the same behaviour as before. To achieve this, 'removable=on' (or 'off') is appended to the '-device usb-storage' parameter sent to qemu when adding a USB disk via '-disk'. A capability flag QEMU_CAPS_USB_STORAGE_REMOVABLE was added to keep track if this option is supported by the qemu version used. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=922495 --- docs/formatdomain.html.in | 8 +++-- docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 31 ++++++++++++++++++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 8 +++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 17 +++++++++++ tests/qemuhelpdata/qemu-1.2.0-device | 11 +++++++ tests/qemuhelpdata/qemu-kvm-1.2.0-device | 11 +++++++ tests/qemuhelptest.c | 6 ++-- .../qemuxml2argv-disk-usb-device-removable.args | 8 +++++ .../qemuxml2argv-disk-usb-device-removable.xml | 27 +++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 13 files changed, 133 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dd22b6d..6fbc418 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1671,9 +1671,13 @@ removable disks (i.e. CDROM or Floppy disk), the value can be either "open" or "closed", defaults to "closed". NB, the value of <code>tray</code> could be updated while the domain is running. - <span class="since">Since 0.0.3; <code>bus</code> attribute since 0.4.3; + The optional attribute <code>removable</code> sets the + removable flag for USB disks, and its value can be either "on" + or "off", defaulting to "off". <span class="since">Since + 0.0.3; <code>bus</code> attribute since 0.4.3; <code>tray</code> attribute since 0.9.11; "usb" attribute value since - after 0.4.4; "sata" attribute value since 0.9.7</span> + after 0.4.4; "sata" attribute value since 0.9.7; "removable" attribute + value since X.Y.Z</span> </dd> <dt><code>iotune</code></dt> <dd>The optional <code>iotune</code> element provides the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ac807e6..aabc592 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1284,6 +1284,14 @@ </choice> </attribute> </optional> + <optional> + <attribute name="removable"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> </element> </define> <define name="geometry"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7309877..efa19af 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4748,6 +4748,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, char *authUUID = NULL; char *usageType = NULL; char *tray = NULL; + char *removable = NULL; char *logical_block_size = NULL; char *physical_block_size = NULL; char *wwn = NULL; @@ -4904,6 +4905,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, target = virXMLPropString(cur, "dev"); bus = virXMLPropString(cur, "bus"); tray = virXMLPropString(cur, "tray"); + removable = virXMLPropString(cur, "removable"); /* HACK: Work around for compat with Xen * driver in previous libvirt releases */ @@ -5337,6 +5339,24 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, def->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED; } + if (removable) { + if ((def->removable = virDomainFeatureStateTypeFromString(removable)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk removable status '%s'"), removable); + goto error; + } + + if (def->bus != VIR_DOMAIN_DISK_BUS_USB) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("removable is only valid for usb disks")); + goto error; + } + } else { + if (def->bus == VIR_DOMAIN_DISK_BUS_USB) { + def->removable = VIR_DOMAIN_FEATURE_STATE_DEFAULT; + } + } + if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && def->bus != VIR_DOMAIN_DISK_BUS_FDC) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -5542,6 +5562,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); + VIR_FREE(removable); VIR_FREE(trans); while (nhosts > 0) { virDomainDiskHostDefFree(&hosts[nhosts - 1]); @@ -14360,10 +14381,14 @@ virDomainDiskDefFormat(virBufferPtr buf, if ((def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && def->tray_status != VIR_DOMAIN_DISK_TRAY_CLOSED) - virBufferAsprintf(buf, " tray='%s'/>\n", + virBufferAsprintf(buf, " tray='%s'", virDomainDiskTrayTypeToString(def->tray_status)); - else - virBufferAddLit(buf, "/>\n"); + if (def->bus == VIR_DOMAIN_DISK_BUS_USB && + def->removable != VIR_DOMAIN_FEATURE_STATE_DEFAULT) { + virBufferAsprintf(buf, " removable='%s'", + virDomainFeatureStateTypeToString(def->removable)); + } + virBufferAddLit(buf, "/>\n"); /*disk I/O throttling*/ if (def->blkdeviotune.total_bytes_sec || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3e118d6..bc7442a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -690,6 +690,7 @@ struct _virDomainDiskDef { char *src; char *dst; int tray_status; + int removable; int protocol; size_t nhosts; virDomainDiskHostDefPtr hosts; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5c566c1..7f69fed 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -236,6 +236,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "device-del-event", "dmi-to-pci-bridge", "usb-storage", + "usb-storage.removable", ); struct _virQEMUCaps { @@ -1438,6 +1439,10 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsScsiGeneric[] = { { "bootindex", QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX }, }; +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUsbStorage[] = { + { "removable", QEMU_CAPS_USB_STORAGE_REMOVABLE }, +}; + struct virQEMUCapsObjectTypeProps { const char *type; struct virQEMUCapsStringFlags *props; @@ -1475,6 +1480,8 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { ARRAY_CARDINALITY(virQEMUCapsObjectPropsUsbHost) }, { "scsi-generic", virQEMUCapsObjectPropsScsiGeneric, ARRAY_CARDINALITY(virQEMUCapsObjectPropsScsiGeneric) }, + { "usb-storage", virQEMUCapsObjectPropsUsbStorage, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsUsbStorage) }, }; @@ -1665,6 +1672,7 @@ virQEMUCapsExtractDeviceStr(const char *qemu, "-device", "ide-drive,?", "-device", "usb-host,?", "-device", "scsi-generic,?", + "-device", "usb-storage,?", NULL); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ virCommandSetErrorBuffer(cmd, &output); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4a8b14b..b587e0e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -192,6 +192,7 @@ enum virQEMUCapsFlags { 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_USB_STORAGE_REMOVABLE = 154, /* usb-storage.removable */ 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 03fb798..c50210b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4347,6 +4347,22 @@ qemuBuildDriveDevStr(virDomainDefPtr def, if (disk->product) virBufferAsprintf(&opt, ",product=%s", disk->product); + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_STORAGE_REMOVABLE)) { + if (disk->removable == VIR_DOMAIN_FEATURE_STATE_ON) + virBufferAsprintf(&opt, ",removable=on"); + else + virBufferAsprintf(&opt, ",removable=off"); + } else { + if (disk->removable != VIR_DOMAIN_FEATURE_STATE_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support setting the " + "removable flag of USB storage devices")); + goto error; + } + } + } + if (virBufferError(&opt)) { virReportOOMError(); goto error; @@ -11239,6 +11255,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, disk->type = VIR_DOMAIN_DISK_TYPE_FILE; disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; disk->bus = VIR_DOMAIN_DISK_BUS_USB; + disk->removable = VIR_DOMAIN_FEATURE_STATE_DEFAULT; if (VIR_STRDUP(disk->dst, "sda") < 0) goto error; if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) diff --git a/tests/qemuhelpdata/qemu-1.2.0-device b/tests/qemuhelpdata/qemu-1.2.0-device index 40845e4..bfc3a4d 100644 --- a/tests/qemuhelpdata/qemu-1.2.0-device +++ b/tests/qemuhelpdata/qemu-1.2.0-device @@ -213,3 +213,14 @@ scsi-generic.bootindex=int32 scsi-generic.channel=uint32 scsi-generic.scsi-id=uint32 scsi-generic.lun=uint32 +usb-storage.drive=drive +usb-storage.logical_block_size=blocksize +usb-storage.physical_block_size=blocksize +usb-storage.min_io_size=uint16 +usb-storage.opt_io_size=uint32 +usb-storage.bootindex=int32 +usb-storage.discard_granularity=uint32 +usb-storage.serial=string +usb-storage.removable=on/off +usb-storage.port=string +usb-storage.full-path=on/off diff --git a/tests/qemuhelpdata/qemu-kvm-1.2.0-device b/tests/qemuhelpdata/qemu-kvm-1.2.0-device index 09e3ef7..f4bfd68 100644 --- a/tests/qemuhelpdata/qemu-kvm-1.2.0-device +++ b/tests/qemuhelpdata/qemu-kvm-1.2.0-device @@ -225,3 +225,14 @@ scsi-generic.bootindex=int32 scsi-generic.channel=uint32 scsi-generic.scsi-id=uint32 scsi-generic.lun=uint32 +usb-storage.drive=drive +usb-storage.logical_block_size=blocksize +usb-storage.physical_block_size=blocksize +usb-storage.min_io_size=uint16 +usb-storage.opt_io_size=uint32 +usb-storage.bootindex=int32 +usb-storage.discard_granularity=uint32 +usb-storage.serial=string +usb-storage.removable=on/off +usb-storage.port=string +usb-storage.full-path=on/off diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index cbabe12..a1cf568 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -942,7 +942,8 @@ mymain(void) QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, QEMU_CAPS_VNC_SHARE_POLICY, - QEMU_CAPS_DEVICE_USB_STORAGE); + QEMU_CAPS_DEVICE_USB_STORAGE, + QEMU_CAPS_USB_STORAGE_REMOVABLE); DO_TEST("qemu-kvm-1.2.0", 1002000, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -1054,7 +1055,8 @@ mymain(void) QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, QEMU_CAPS_VNC_SHARE_POLICY, - QEMU_CAPS_DEVICE_USB_STORAGE); + QEMU_CAPS_DEVICE_USB_STORAGE, + QEMU_CAPS_USB_STORAGE_REMOVABLE); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args new file mode 100644 index 0000000..36be080 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 -device ide-drive,\ +bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=/tmp/usbdisk.img,\ +if=none,id=drive-usb-disk0 -device usb-storage,drive=drive-usb-disk0,\ +id=usb-disk0,removable=on -device virtio-balloon-pci,id=balloon0,bus=pci.0,\ +addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml new file mode 100644 index 0000000..6ee3e9b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='file' device='disk'> + <source file='/tmp/usbdisk.img'/> + <target dev='sda' bus='usb' removable='on'/> + </disk> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2f993ce..cf11976 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -542,6 +542,9 @@ mymain(void) DO_TEST("disk-usb-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("disk-usb-device-removable", + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_USB_STORAGE, + QEMU_CAPS_USB_STORAGE_REMOVABLE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-scsi-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); -- 1.7.10.4
participants (4)
-
anonym -
Daniel P. Berrange -
Eric Blake -
Fred A. Kemp