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 ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
