[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: command: Ignore QEMU_CAPS_DEVICE when building drive alias



On 05/16/2016 02:42 AM, Peter Krempa wrote:
> On Sun, May 15, 2016 at 17:03:55 -0400, Cole Robinson wrote:
>> QEMU_CAPS_DEVICE is always set nowadays, so we can drop the
>> non-DEVICE code paths
>> ---
>>  src/qemu/qemu_command.c | 12 ++++--------
>>  src/qemu/qemu_command.h |  3 +--
>>  src/qemu/qemu_hotplug.c |  4 ++--
>>  3 files changed, 7 insertions(+), 12 deletions(-)
> 
> [...]
> 
>> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
>> index d5ad1b2..c777701 100644
>> --- a/src/qemu/qemu_command.h
>> +++ b/src/qemu/qemu_command.h
>> @@ -96,8 +96,7 @@ char *qemuBuildNicDevStr(virDomainDefPtr def,
>>                           size_t vhostfdSize,
>>                           virQEMUCapsPtr qemuCaps);
>>  
>> -char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk,
>> -                               virQEMUCapsPtr qemuCaps);
>> +char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk);
> 
> ACK, although it's weird that this function is only used in the hotplug
> code. 

Yep, and even outside qemu_command.c it's not consistently used. After sending
this I added a BiteSizedTask to look for places to convert:

http://wiki.libvirt.org/page/BiteSizedTasks#More_usage_of_qemuDeviceDriveHostAlias.28.29

This has probably to do with the very weird command line generator
> code for drives which actually clears QEMU_CAPS_DEVICE in certain cases
> and then sets it back once finished.
> 

Yes, I was scratching my head over that code again recently. I had patches for
it at one point but I doubt they apply any longer and I won't get back to them
anytime soon, so if you take a stab at it I'll help review.

http://www.redhat.com/archives/libvir-list/2016-January/msg00995.html

Patch pushed now, thanks

- Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]