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

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

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]