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

Re: [libvirt] [PATCH 09/27] conf: introduce virNetworkPortDefPtr struct and XML support



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 redhat com>

[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 :|


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