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