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

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



On Wed, 2016-05-18 at 09:58 -0400, Laine Stump wrote:
> On 05/18/2016 07:53 AM, Cole Robinson wrote:
> > 
> > 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
> 
> My opinion is that this is a change for the better. While it is true 
> that this could lead to XML that potentially shows different PCI 
> addresses than what would have been auto-assigned by qemu when presented 
> with the original commandline, it *is* showing the addresses that would 
> be auto-allocated by libvirt if you had fed the "un-addressified" XML to 
> libvirt - so at least the user will get a warning rather than being 
> surprised at runtime. And it's not as if the output of 
> domxml-from-native has ever produced anything even close to the correct 
> XML in any real world situation. (dreadkopp in public #virt pastebin'ed 
> an example last night - 90% of it was translated directly into 
> <qemu:arg> elements).

After reading Cole's explanation, I tend to agree.

So I guess we can go ahead and merge this, as well as 5/4 :)

-- 
Andrea Bolognani
Software Engineer - Virtualization Team


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