On 01/10/2012 05:41 PM, Eric Blake wrote:
/* all of these modes can use a pool of physical interfaces */ nForwardIfs = virXPathNodeSet("./interface", ctxt, &forwardIfNodes); - if (nForwardIfs < 0) + if (nForwardIfs <= 0) { + virNetworkReportError(VIR_ERR_XML_ERROR, + _("No interface pool given, checking for SRIOV pf")); + nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes); + + if (nForwardPfs <= 0) { + virNetworkReportError(VIR_ERR_XML_ERROR, + _("No interface pool or SRIOV physical device given"));
This has to be a check for '< 0', not '<= 0', or else you get LOTS of 'make check' failures.
Also, your patch touched the rng, but failed to add documentation for the new XML, nor tests that prove we can parse it. And in adding those tests, I found that your rng additions don't match your code (which wants pf before, not after, interface). I'm working on fixing that, but it's taking me longer than I planned, since I also decided to add tests of the parsing, and in those tests, it appears that pf did not get parsed as expected. I don't know if it's a flaw in the original patch or in my touchups...
I think I understand the problem now. You aren't parsing nForwardPfs unless there are exactly 0 nForwardIfs; but if you end up reparsing existing live XML, you don't want to lose the pf information. That is, the parsing routine should always parse pf information; and perhaps even be taught to honor flags (just like the format routine), so that it skips parsing <interface> elements if doing an INACTIVE parse and a pf is present. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org