On Tue, Mar 19, 2013 at 09:40:54AM +0100, anonym wrote:
Can you use a real name instead of an anonymous psuedonym for patches.
This adds 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' is appended to the '-device
usb-storage' parameter sent to qemu when adding a USB disk via
'-disk'. For versions of qemu only supporting '-usbdevice disk:' for
adding USB disks this feature always remains 'off' since there's no
support for passing such an option.
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 | 35 ++++++++++++++++++--
src/conf/domain_conf.h | 9 +++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 6 ++++
.../qemuxml2argv-disk-usb-device-removable.args | 8 +++++
.../qemuxml2argv-disk-usb-device-removable.xml | 27 +++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
9 files changed, 99 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml
@@ -12915,10 +12940,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_DISK_REMOVABLE_OFF) {
This means that if the user explicitly added removeable='off' to their
XML, we'll be dropping it.
+ virBufferAsprintf(buf, " removable='%s'",
+ virDomainDiskRemovableTypeToString(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 96f11ba..0f4f0d7 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -518,6 +518,13 @@ enum virDomainDiskTray {
VIR_DOMAIN_DISK_TRAY_LAST
};
+enum virDomainDiskRemovable {
If you add in
VIR_DOMAIN_DISK_REMOVABLE_DEFAULT
then you can distinguish explicit on/off settings from the
default setting to address my earlier comment.
+ VIR_DOMAIN_DISK_REMOVABLE_ON,
+ VIR_DOMAIN_DISK_REMOVABLE_OFF,
+
+ VIR_DOMAIN_DISK_REMOVABLE_LAST
+};
+
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5cad990..0d1a9d6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -156,6 +156,7 @@ virDomainDiskIoTypeToString;
virDomainDiskPathByName;
virDomainDiskProtocolTransportTypeFromString;
virDomainDiskProtocolTransportTypeToString;
+virDomainDiskRemovableTypeToString;
The VIR_ENUM macro generates 2 methods, so also add in
virDomainDiskRemovableTypeFromString;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4891b65..c04cecf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3219,6 +3219,11 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
if (disk->product)
virBufferAsprintf(&opt, ",product=%s", disk->product);
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_USB &&
+ disk->removable != VIR_DOMAIN_DISK_REMOVABLE_OFF) {
+ virBufferAsprintf(&opt, ",removable=%s",
+ virDomainDiskRemovableTypeToString(disk->removable));
+ }
We should should not on the QEMU default setting - so make sure
you explicitly set both removeable=on or removeable=off.
Also, not all versions of QEMU support this property, so you'll
need to add a new capability flag in qemu_capabilities.h and
then update qemu_capabilities.c to detect whether it exists or
not.
Then in this code, if you find a QEMU which does not support the
flag, you should do a virRaiseError(VIR_ERR_CONFIG_UNSUPPORTED, ....)
to tell the user we can't do what they asked
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 :|