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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list