[libvirt] [PATCH 3/5 v3] Adding the element pf to network xml.

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120111/8c20c656/attachment-0001.sig>