Re: [PATCH v3 17/36] conf: add APIs to convert virDomainNetDef to virNetworkPortDef

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

 



On Fri, Mar 22, 2019 at 01:11:34PM -0400, Laine Stump wrote:
> On 3/19/19 8:46 AM, Daniel P. Berrangé wrote:
> > Helper APIs are needed to
> > 
> >   - Populate basic virNetworkPortDef from virDomainNetDef
> >   - Set a virDomainActualNetDef from virNetworkPortDef
> >   - Populate a full virNetworkPortDef from virDomainActualNetDef
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > ---
> >   src/conf/domain_conf.c   | 272 +++++++++++++++++++++++++++++++++++++++
> >   src/conf/domain_conf.h   |  17 +++
> >   src/libvirt_private.syms |   3 +
> >   3 files changed, 292 insertions(+)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index d283feaca6..ee4d586d77 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -39,6 +39,7 @@
> >   #include "virbuffer.h"
> >   #include "virlog.h"
> >   #include "nwfilter_conf.h"
> > +#include "virnetworkportdef.h"
> >   #include "storage_conf.h"
> >   #include "virstoragefile.h"
> >   #include "virfile.h"
> > @@ -30161,6 +30162,277 @@ virDomainNetTypeSharesHostView(const virDomainNetDef *net)
> >       return false;
> >   }
> > +virNetworkPortDefPtr
> > +virDomainNetDefToNetworkPort(virDomainDefPtr dom,
> > +                             virDomainNetDefPtr iface)
> > +{
> > +    virNetworkPortDefPtr port;
> > +
> > +    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                       _("Expected an interface of type 'network' not '%s'"),
> > +                       virDomainNetTypeToString(iface->type));
> > +        return NULL;
> > +    }
> > +
> > +    if (VIR_ALLOC(port) < 0)
> > +        return NULL;
> > +
> > +    virUUIDGenerate(port->uuid);
> > +
> > +    memcpy(port->owneruuid, dom->uuid, VIR_UUID_BUFLEN);
> > +    if (VIR_STRDUP(port->ownername, dom->name) < 0)
> > +        goto error;
> 
> 
> It's not important, but  was there any reason you put the above two items
> out of order wrt the virNetworkPortDef struct? Having them in order would
> make it simpler to verify nothing had been missed.
> 
> 
> > +
> > +    if (VIR_STRDUP(port->group, iface->data.network.portgroup) < 0)
> > +        goto error;
> > +
> > +    memcpy(&port->mac, &iface->mac, VIR_MAC_BUFLEN);
> > +
> > +    if (virNetDevVPortProfileCopy(&port->virtPortProfile, iface->virtPortProfile) < 0)
> > +        goto error;
> > +
> > +    if (virNetDevBandwidthCopy(&port->bandwidth, iface->bandwidth) < 0)
> > +        goto error;
> > +
> > +    if (virNetDevVlanCopy(&port->vlan, &iface->vlan) < 0)
> > +        goto error;
> > +
> > +    port->trustGuestRxFilters = iface->trustGuestRxFilters;
> > +
> > +    return port;
> > +
> > + error:
> > +    virNetworkPortDefFree(port);
> > +    return NULL;
> > +}
> > +
> > +int
> > +virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface,
> > +                                     virNetworkPortDefPtr port)
> > +{
> > +    virDomainActualNetDefPtr actual = NULL;
> > +
> > +    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                       _("Expected an interface of type 'network' not '%s'"),
> > +                       virDomainNetTypeToString(iface->type));
> > +        return -1;
> > +    }
> > +
> > +    if (VIR_ALLOC(actual) < 0)
> > +        return -1;
> > +
> > +    switch ((virNetworkPortPlugType)port->plugtype) {
> > +    case VIR_NETWORK_PORT_PLUG_TYPE_NONE:
> > +        break;
> > +
> > +    case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE:
> > +        if (VIR_STRDUP(actual->data.bridge.brname,
> > +                       port->plug.bridge.brname) < 0)
> > +            goto error;
> > +        actual->data.bridge.macTableManager = port->plug.bridge.macTableManager;
> > +        break;
> 
> 
> none of the cases set actual->type.
> 
> 
> > +
> > +    case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT:
> > +        if (VIR_STRDUP(actual->data.direct.linkdev,
> > +                       port->plug.direct.linkdev) < 0)
> > +            goto error;
> > +        actual->data.direct.mode = port->plug.direct.mode;
> > +        break;
> > +
> > +    case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI:
> > +        actual->data.hostdev.def.parent = iface;
> > +        actual->data.hostdev.def.info = &iface->info;
> 
> 
> Again, it would be simpler to verify if the assignments were in the same
> order as the attributes are in the definition of virDomainHostdevDef. (It
> appears there are some that aren't being set (startupPolicy, missing,
> readonly, shareable, origStates), but that's just because they're never used
> in this context anyway, (as proven by the fact that they don't have a
> counterpart in virNetworkPort.
> 
> 
> > +        actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
> > +        actual->data.hostdev.def.managed = port->plug.hostdevpci.managed;
> > +        actual->data.hostdev.def.source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
> > +        actual->data.hostdev.def.source.subsys.u.pci.addr = port->plug.hostdevpci.addr;
> > +        switch ((virNetworkForwardDriverNameType)port->plug.hostdevpci.driver) {
> > +        case VIR_NETWORK_FORWARD_DRIVER_NAME_DEFAULT:
> > +            actual->data.hostdev.def.source.subsys.u.pci.backend =
> > +                VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT;
> > +            break;
> > +
> > +        case VIR_NETWORK_FORWARD_DRIVER_NAME_KVM:
> > +            actual->data.hostdev.def.source.subsys.u.pci.backend =
> > +                VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM;
> > +            break;
> 
> 
> Sigh. More legacy KVM pci device assignment stuff.
> 
> 
> > +
> > +        case VIR_NETWORK_FORWARD_DRIVER_NAME_VFIO:
> > +            actual->data.hostdev.def.source.subsys.u.pci.backend =
> > +                VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
> > +            break;
> > +
> > +        case VIR_NETWORK_FORWARD_DRIVER_NAME_LAST:
> > +        default:
> > +            virReportEnumRangeError(virNetworkForwardDriverNameType,
> > +                                    port->plug.hostdevpci.driver);
> > +            goto error;
> > +        }
> > +
> > +        break;
> > +
> > +    case VIR_NETWORK_PORT_PLUG_TYPE_LAST:
> > +    default:
> > +        virReportEnumRangeError(virNetworkPortPlugType, port->plugtype);
> > +        goto error;
> > +    }
> > +
> > +    if (virNetDevVPortProfileCopy(&actual->virtPortProfile, port->virtPortProfile) < 0)
> > +        goto error;
> > +
> > +    if (virNetDevBandwidthCopy(&actual->bandwidth, port->bandwidth) < 0)
> > +        goto error;
> > +
> > +    if (virNetDevVlanCopy(&actual->vlan, &port->vlan) < 0)
> > +        goto error;
> > +
> > +    actual->class_id = port->class_id;
> > +    actual->trustGuestRxFilters = port->trustGuestRxFilters;
> > +
> > +    virDomainActualNetDefFree(iface->data.network.actual);
> > +    iface->data.network.actual = actual;
> > +
> > +    return 0;
> > +
> > + error:
> > +    virDomainActualNetDefFree(actual);
> > +    return -1;
> > +}
> > +
> > +virNetworkPortDefPtr
> > +virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
> > +                                   virDomainNetDefPtr iface)
> > +{
> > +    virDomainActualNetDefPtr actual;
> > +    virNetworkPortDefPtr port;
> > +
> > +    if (!iface->data.network.actual) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                       _("Missing actual data for interface '%s'"),
> > +                       iface->ifname);
> > +        return NULL;
> > +    }
> > +
> > +    actual = iface->data.network.actual;
> > +
> > +    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                       _("Expected an interface of type 'network' not '%s'"),
> > +                       virDomainNetTypeToString(iface->type));
> > +        return NULL;
> > +    }
> > +
> > +    if (VIR_ALLOC(port) < 0)
> > +        return NULL;
> > +
> > +    /* Bad - we need to preserve original port uuid */
> 
> 
> So is this acceptable as-is because of the limited ways you end up using 
> virDomainNetDefActualToNetworkPort()? Or is it something that needs to be
> fixed?

Hm, I swear I fixed that, but perhapos I've mistakenly squashed into
a later patch in this series.


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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux