Re: [PATCH v2 4/6] tests: utils: Add PreFormat callback for CompareXML2XML helper

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

 



On Tue, Feb 09, 2016 at 03:28:59PM -0500, Cole Robinson wrote:
On 02/09/2016 03:13 PM, Laine Stump wrote:
On 02/09/2016 10:59 AM, Cole Robinson wrote:
This allows individual driver tests to hook in their own code before
the def is formatted and compared.

We will eventually use this in the qemuxml2xml
---

Oops. I ACKed 5/6 before this one (after looking ahead to see how this would
be used).

This looks okay, although I'm wondering why you call just the PCI address
assignment rather than a full postparse callback. Other things can happen in
there, e.g. auto-adding controllers. We may as well test the whole thing.


PostParse is automatically called by DomainDefParse (I said DomainDefFormat in
a previously mail, that's incorrect), so we don't need to call it explicitly.
The qemu specific controller additions are triggered via that.

The generic domain_conf AddImplicitControllers though isn't tested here, but I
have previously posted patches that move that call into the generic PostParse
function, so it will be tested after that. I'll be reposting those as part of
another series after this is merged.

The other series also puts AssignAddresses into qemu's PostParse callback, so
the immediate need for this patch will be gone. But there are other bits like
AssignAliases we should be testing here as well


Yes, there are still more things that need to be done, especially if
we're going to check active XML.  However the problem of our codebase is
that we don't have dedicated places to modify:

a) persistent definition before starting the domain (adding defaults,
   controllers, whatever) -- this could be added to PortParse later
   since we now pass qemuCaps into that function.

b) live definition before the domain is started -- there are various
   places where such information is changed and because some of them
   are in BuildCommandLine and some are not, and our tests only call
   that function, we don't have a check for those modified in
   ProcessLaunch, for example.

I don't know whether there's an ongoing effort to do this, but I
overheard some chatter about that.  Not a problem of this patch, of
course, just needed to let the steam out somewhere ;)

Still this is a good start at that, so ACK.



Thanks, pushed now

- Cole

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]