On 01/10/2012 04:33 PM, Eric Blake wrote: > On 12/14/2011 03:50 AM, Shradha Shah wrote: >> This element will help the user to just specify the SR-IOV physical >> function in order to access all the Virtual functions attached to it. >> --- >> docs/schemas/network.rng | 7 ++++ >> src/conf/network_conf.c | 69 +++++++++++++++++++++++++++++++++++++++++++-- >> src/conf/network_conf.h | 3 ++ >> 3 files changed, 75 insertions(+), 4 deletions(-) > > In addition to Daniel's comments, > >> @@ -965,10 +971,52 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) >> >> /* 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... -- Eric Blake eblake@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list