Re: [libvirt] [PATCH 6/4] qemu: address: Remove QEMU_CAPS_DEVICE usage
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