[libvirt] [PATCH 6/4] qemu: address: Remove QEMU_CAPS_DEVICE usage

Cole Robinson crobinso at redhat.com
Wed May 18 11:53:58 UTC 2016


On 05/18/2016 06:27 AM, Andrea Bolognani wrote:
> On Sat, 2016-05-14 at 17:39 -0400, Cole Robinson wrote:
>> All qemu versions we support have QEMU_CAPS_DEVICE, so checking
>> for it is redundant. Remove the usage.
>>  
>> The code diff isn't clear, but all that code is just inindented
>> with no other change.
> 
> 'git show -w' is your friend ;)
> 
> This information, however, should probably be moved out of
> the commit message and after the '---' separator.
> 
>> Test cases that hit qemuDomainAssignAddresses but don't have
>> infrastructure for specifying qemuCaps values see lots of
>> churn, since now PCI addresses are in the XML output.
> 
> So, I want to make sure I'm getting this right: the addresses
> should have been there in the first place, and would be if we
> were processing the input files in the real world, outside of
> the test suite; however, since the addresses being there
> depend on QEMU_CAPS_DEVICE, and some test cases run with an
> empty virQEMUCaps, they never appeared until we got rid of
> the check on QEMU_CAPS_DEVICE.
> 
> ACK if the above makes sense.

Kind of. Prior to patch #2, the test suite output was correct (no addresses),
it's what we were returning via domxml-from-native. After patch #2, the test
suite output was wrong for all real world usage; it didn't change because it
was only hitting a !QEMU_CAPS_DEVICE code path

So the potentially contentious bit is that patch #2 changes domxml-from-native
output to contain addresses, however that's exactly the same result that will
happen when the XML would eventually be defined anyways, so it's effectively
the same result as pre-patch #2 anyways. If we think of domxml-from-native as
telling the user 'this is exactly what libvirt thinks that command line is'
then we are now giving more accurate results

Let me know if that still warrants the ACK

Thanks,
Cole




More information about the libvir-list mailing list