[libvirt] [PATCH v2] Perform disk config validity checking for attach-device config

[John Ferlan]

On 08/14/2014 11:53 AM, Ján Tomko wrote:
> On 08/07/2014 04:53 PM, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1078126
>>
>> Using 'virsh attach-device --config' (or --persistent) to attach a
>> file backed lun device will succeed; however, subsequent domain restarts
>> will result in failure because the configuration of a file backed lun
>> is not supported.
>>
>> Although allowing 'illegal configurations' is something that can be
>> allowed, it may not be practical in this case. Generally, when attaching
>> a device to a domain means the domain must be running. A way around
>> this is using the --config (or --persistent) option. When an attach
>> is done to a running domain, a temporary configuration is modified
>> first followed by the live update. The live update will make a number
>> of disk validity checks when building the qemu command to attach the
>> disk. If any fail, then change is rejected.
>>
>> Rather than allow a potentially illegal combination, adjust the code
>> in the configuration path to make the same checks as the running path
>> will make with respect to disk validity checks. This way we avoid
>> having the potential for some subsequent start/reboot to fail because
>> an illegal combination was allowed.
>>
>> NB: The live path still checks the configuration since it is possible
>> to just do --live guest modification...
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>
>> Changes since V1:
>> http://www.redhat.com/archives/libvir-list/2014-August/msg00045.html
>>
>> Based on #virt IRC discussion this morning 
>> - move the qemu_caps checking to only occur during the live check
>> - slightly change the name of the called routine
>>
>>  src/qemu/qemu_command.c | 127 +++++++++++++++++++++++++++---------------------
>>  src/qemu/qemu_command.h |   2 +
>>  src/qemu/qemu_driver.c  |   2 +
>>  3 files changed, 76 insertions(+), 55 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 033a5a8..c1791d9 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3230,6 +3230,73 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
>>  }
>>  
>>  
>> +/* Perform disk definition config validity checks */
>> +int
>> +qemuBuildCheckDiskConfig(virDomainDiskDefPtr disk)
> 
> Just a nitpick:
> 'Build' seems redundant here, how about just 'qemuCheckDiskConfig'?
> 
> ACK either way.
> 
> Jan
> 
> 

Adjusted function name and pushed.

Tks,

John