On Tue, Jan 08, 2019 at 05:15:49PM +0100, Michal Privoznik wrote:
> On 12/24/18 3:58 PM, Daniel P. Berrangé wrote:
> > Introduce a virNetworkPortDefPtr struct to represent the data associated
> > with a virtual network port. Add APIs for parsing/formatting XML docs
> > with the data.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
[snip]
> > +static virNetworkPortDefPtr
> > +virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
> > +{
> > + virNetworkPortDefPtr def;
> > + char *uuid = NULL;
> > + xmlNodePtr virtPortNode;
> > + xmlNodePtr vlanNode;
> > + xmlNodePtr bandwidthNode;
> > + xmlNodePtr addressNode;
> > + char *trustGuestRxFilters = NULL;
> > + char *mac = NULL;
> > + char *macmgr = NULL;
> > + char *mode = NULL;
> > + char *plugtype = NULL;
> > + char *managed = NULL;
> > + char *driver = NULL;
> > +
> > + if (VIR_ALLOC(def) < 0)
> > + return NULL;
> > +
> > + uuid = virXPathString("string(./uuid)", ctxt);
> > + if (!uuid) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + "%s", _("network port has no uuid"));
> > + goto error;
> > + }
> > + if (virUUIDParse(uuid, def->uuid) < 0) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("Unable to parse UUID '%s'"), uuid);
> > + goto error;
> > + }
> > +
> > + def->ownername = virXPathString("string(./owner/name)", ctxt);
> > + if (!def->ownername) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + "%s", _("network port has no owner name"));
> > + goto error;
> > + }
> > +
> > + VIR_FREE(uuid);
> > + uuid = virXPathString("string(./owner/uuid)", ctxt);
> > + if (!uuid) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + "%s", _("network port has no owner UUID"));
> > + goto error;
> > + }
> > +
> > + if (virUUIDParse(uuid, def->owneruuid) < 0) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("Unable to parse UUID '%s'"), uuid);
> > + goto error;
> > + }
> > +
> > + def->group = virXPathString("string(./group)", ctxt);
> > +
> > + virtPortNode = virXPathNode("./virtualport", ctxt);
> > + if (virtPortNode &&
> > + (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode, 0)))) {
> > + goto error;
> > + }
> > +
> > + mac = virXPathString("string(./mac/@address)", ctxt);
> > + if (!mac) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + "%s", _("network port has no mac"));
> > + goto error;
> > + }
> > + if (virMacAddrParse(mac, &def->mac) < 0) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("Unable to parse MAC '%s'"), mac);
> > + goto error;
> > + }
> > +
> > + bandwidthNode = virXPathNode("./bandwidth", ctxt);
> > + if (bandwidthNode &&
> > + virNetDevBandwidthParse(&def->bandwidth, bandwidthNode, -1) < 0)
>
> This must not be -1. The bandwidth corresponds to the interface not the
> bridge. If this is -1 then 'floor' is disallowed, which is not what we
> want. Maybe we need to change the @net_type argument of
> virNetDevBandwidthParse() so that it is bool which allows/denies 'floor'.
Yes, I'll introduce a prerequisite patch turning that parameter into
a 'bool allowFloor'. When parsing network port XML, we'll have to
pass true unconditionally. At the time the port is actually wired up
we'll have a runtime check as to whether floor can actually be used
or not for a given port.
> > + if (def->trustGuestRxFilters)
> > + virBufferAsprintf(buf, "<rxfilters trustGuest='%s'/>",
>
> Add "\n" here.
Yep, this shows some missing coverage in the test XML files which
I'll address too.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
