On 08/14/2012 01:15 AM, Laine Stump wrote: > The following config elements now support a <vlan> subelements: > > within a domain: <interface>, and the <actual> subelement of <interface> > within a network: the toplevel, as well as any <portgroup> > > Each vlan element must have one or more <tag id='n'/> subelements. If > there is more than one tag, it is assumed that vlan trunking is being > requested. If trunking is required with only a single tag, the > attribute "trunk='yes'" should be added to the toplevel <vlan> > element. > > > IMPORTANT NOTE: As of this patch there is no backend support for the > vlan element for *any* network device type. When support is added in > later patches, it will only be for those select network types that > support setting up a vlan on the host side, without the guest's > involvement. (For example, it will be possible to configure a vlan for > a guest connected to an openvswitch bridge, but it won't be possible > + <element name="tag"> > + <attribute name="id"> > + <data type="unsignedInt"> > + <param name="maxInclusive">4095</param> > + </data> > + </attribute> I would also add <empty/> here, since the user should always be using <tag/> rather than <tag><sub/></tag> or <tag>data</tag>. > +int > +virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr def) > +{ > + int ret = -1; > + xmlNodePtr save = ctxt->node; > + const char *trunk; > + xmlNodePtr *tagNodes = NULL; > + int nTags; > + > + ctxt->node = node; > + > + nTags = virXPathNodeSet("./tag", ctxt, &tagNodes); > + if (nTags < 0) > + goto error; > + > + if (nTags == 0) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("missing tag id - each <vlan> must have " > + "at least one <tag id='n'/> subelement")); > + goto error; > + } > + > + if (nTags > 0) { This 'if' is spurious, given the above two statements always short-circuiting to error. Cosmetic, though. > + /* now that we know how many tags there are, look for an explicit > + * trunk setting. > + */ > + if (nTags > 1) > + def->trunk = true; > + > + ctxt->node = node; > + if ((trunk = virXPathString("string(./@trunk)", ctxt)) != NULL) { > + def->trunk = STRCASEEQ(trunk, "yes"); > + if (nTags > 1 && !def->trunk) { It took me a couple reads to see - this says the user can pass trunk='garbage' for a single <tag> (it won't get past the RNG validation, but our C code doesn't care), if they passed trunk at all, it MUST be trunk='yes' for multiple <tag>s. Okay. ACK with nits addressed. -- 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