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

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



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

Attachment: signature.asc
Description: OpenPGP digital signature


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