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@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