[libvirt] [PATCH] qemu: Support setting the 'removable' flag for USB disks

[Peter Krempa]

On 03/19/13 12:59, Daniel P. Berrange wrote:
> 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
>>

...

>>       /*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

Or use

VIR_DOMAIN_FEATURE_STATE_[DEFAULT|ON|OFF|LAST] that I introduced 
specifically to avoid adding enums like this for every feature.

>> +};
>> +
>
>

Peter