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

Re: [libvirt] [PATCH 10/12] qemu: caps: blacklist QEMU_CAPS_CHARDEV



On 07/07/2017 09:52 AM, Andrea Bolognani wrote:
> On Mon, 2017-06-26 at 14:01 -0400, Cole Robinson wrote:
>> @@ -5555,12 +5551,9 @@ virQEMUCapsCacheFree(virQEMUCapsCachePtr cache)
>>   
>>   bool
>>   virQEMUCapsSupportsChardev(const virDomainDef *def,
>> -                           virQEMUCapsPtr qemuCaps,
>> +                           virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
> 
> I know you're going to get rid of @qemuCaps when you rename
> the function later on, but I think you should go ahead and
> get rid of it in this commit instead, at the same time as
> you remove its only use.
>

Thanks for your review! I see your argument but to do this fully the result
would be a lot of churn that distracts from the goal of the patch. Plus when
you pull on this thread a lot comes out:

- remove qemuCaps from all SupportsChardev callers...
-- which requires removing the now unused qemuCaps in ProcessLookupPTYs
--- which requires removing the now unused qemuCaps in
qemuProcessFindCharDevicePTYsMonitor
--- which requires removing the now unused qemuCaps in qemuProcessWaitForMonitor

I'll leave this as is here and send an additional patch to handle
ProcessLookupPTYs

>> @@ -1403,14 +1397,14 @@ mymain(void)
>>                       QEMU_CAPS_PCI_OHCI,
>>                       QEMU_CAPS_PIIX3_USB_UHCI);
>>       DO_TEST("usb-controller-xhci",
>> -            QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PIIX3_USB_UHCI,
>> +            QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PIIX3_USB_UHCI,
> 
> Here, and in a few other instances, since you're going to
> touch the line in any case you can take advantage of the
> opportunity and make sure each of the remaining capabilities
> is on its own line so that future tweaking will result in
> smaller diffs.

I'll try to fix all instances of this

Thanks,
Cole


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