[libvirt] [PATCHv2 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 https://www.redhat.com/archives/libvir-list/2013-August/msg00581.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 | 24 +++++++++++++++ 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, 155 insertions(+), 12 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 | 7 +++++++ tests/qemuhelptest.c | 18 ++++++++++++------ tests/qemuxml2argvtest.c | 3 ++- 5 files changed, 24 insertions(+), 7 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 b5ac15a..395b24a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4325,6 +4325,13 @@ 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) 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 3a3c304..5dbf941 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 Fri, Aug 23, 2013 at 12:38:10PM +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 | 7 +++++++ tests/qemuhelptest.c | 18 ++++++++++++------ tests/qemuxml2argvtest.c | 3 ++- 5 files changed, 24 insertions(+), 7 deletions(-)
ACK Danielx -- |: 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 :|
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 9d12293..ed24919 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1776,9 +1776,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 dfcd61c..fdf45fd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1285,6 +1285,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 8a187a6..30ec5f4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4756,6 +4756,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; @@ -4912,6 +4913,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 */ @@ -5346,6 +5348,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, @@ -5551,6 +5571,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); + VIR_FREE(removable); VIR_FREE(trans); while (nhosts > 0) { virDomainDiskHostDefFree(&hosts[nhosts - 1]); @@ -14380,10 +14401,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 500a5be..6964fbf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -691,6 +691,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 395b24a..aa7981e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4368,6 +4368,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; @@ -11277,6 +11293,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 5dbf941..1447588 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
On Fri, Aug 23, 2013 at 12:38:11PM +0200, Fred A. Kemp wrote:
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
ACK 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 :|
On 08/23/13 12:38, Fred A. Kemp wrote:
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 https://www.redhat.com/archives/libvir-list/2013-August/msg00581.html
Fred A. Kemp (2): qemu: Add capability flag for usb-storage qemu: Support setting the 'removable' flag for USB disks
This patchset unfortunately breaks the recently added qemuhotplugtest: 14) hotplug-base ATTACH disk-usb ... libvirt: QEMU Driver error : unsupported configuration: This QEMU doesn't support '-device usb-storage' FAILED 15) hotplug-base DETACH disk-usb ... libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED 16) hotplug-base ATTACH disk-usb ... libvirt: QEMU Driver error : unsupported configuration: This QEMU doesn't support '-device usb-storage' FAILED 17) hotplug-base DETACH disk-usb ... domain XML should not match the expected result libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED 18) hotplug-base DETACH disk-usb ... libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED Peter
27/08/13 14:40, Peter Krempa wrote:
On 08/23/13 12:38, Fred A. Kemp wrote:
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 https://www.redhat.com/archives/libvir-list/2013-August/msg00581.html
Fred A. Kemp (2): qemu: Add capability flag for usb-storage qemu: Support setting the 'removable' flag for USB disks
This patchset unfortunately breaks the recently added qemuhotplugtest:
14) hotplug-base ATTACH disk-usb ... libvirt: QEMU Driver error : unsupported configuration: This QEMU doesn't support '-device usb-storage' FAILED 15) hotplug-base DETACH disk-usb ... libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED 16) hotplug-base ATTACH disk-usb ... libvirt: QEMU Driver error : unsupported configuration: This QEMU doesn't support '-device usb-storage' FAILED 17) hotplug-base DETACH disk-usb ... domain XML should not match the expected result libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED 18) hotplug-base DETACH disk-usb ... libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED
After a two minute investigation of this new test, I threw in the following fix which *seemingly* does the trick (i.e. I see no test failures any more): --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -78,6 +78,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, /* for attach & detach qemu must support -device */ virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DRIVE); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_NET_NAME); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VIRTIO_SCSI); if (event) I'm very time constrained at the moment so I didn't have time to read the sources in detail, so the above fix is based on "pattern matching" only. If the fix looks good any way, it should be fixup'ed into my patch #1. I'm a bit confused with the process now, as my previous patches were ACKed but not pushed. Should send a new patchset? Cheers!
On 08/28/13 15:34, anonym wrote:
27/08/13 14:40, Peter Krempa wrote:
On 08/23/13 12:38, Fred A. Kemp wrote:
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 https://www.redhat.com/archives/libvir-list/2013-August/msg00581.html
Fred A. Kemp (2): qemu: Add capability flag for usb-storage qemu: Support setting the 'removable' flag for USB disks
This patchset unfortunately breaks the recently added qemuhotplugtest:
14) hotplug-base ATTACH disk-usb ... libvirt: QEMU Driver error : unsupported configuration: This QEMU doesn't support '-device usb-storage' FAILED 15) hotplug-base DETACH disk-usb ... libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED 16) hotplug-base ATTACH disk-usb ... libvirt: QEMU Driver error : unsupported configuration: This QEMU doesn't support '-device usb-storage' FAILED 17) hotplug-base DETACH disk-usb ... domain XML should not match the expected result libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED 18) hotplug-base DETACH disk-usb ... libvirt: QEMU Driver error : operation failed: disk sdq not found FAILED
After a two minute investigation of this new test, I threw in the following fix which *seemingly* does the trick (i.e. I see no test failures any more):
--- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -78,6 +78,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, /* for attach & detach qemu must support -device */ virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DRIVE); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_NET_NAME); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VIRTIO_SCSI); if (event)
I'm very time constrained at the moment so I didn't have time to read the sources in detail, so the above fix is based on "pattern matching" only. If the fix looks good any way, it should be fixup'ed into my patch #1.
I'm a bit confused with the process now, as my previous patches were ACKed but not pushed. Should send a new patchset?
Ususaly the reviewer is responsible for checking and pushing patches from non-maintainers. If the fix is indeed to enable the one capability bit I'll amend those patches and push them later today.
Cheers!
Peter
On 08/28/13 15:41, Peter Krempa wrote:
On 08/28/13 15:34, anonym wrote:
27/08/13 14:40, Peter Krempa wrote:
On 08/23/13 12:38, Fred A. Kemp wrote:
From: "Fred A. Kemp" <anonym@riseup.net>
...
I'm very time constrained at the moment so I didn't have time to read the sources in detail, so the above fix is based on "pattern matching" only. If the fix looks good any way, it should be fixup'ed into my patch #1.
I'm a bit confused with the process now, as my previous patches were ACKed but not pushed. Should send a new patchset?
Ususaly the reviewer is responsible for checking and pushing patches from non-maintainers. If the fix is indeed to enable the one capability bit I'll amend those patches and push them later today.
I rebased and pushed the patches now that the release is done.
Cheers!
Peter
participants (4)
-
anonym -
Daniel P. Berrange -
Fred A. Kemp
-
Peter Krempa