[libvirt] [PATCH v3 17/36] conf: add APIs to convert virDomainNetDef to virNetworkPortDef
Laine Stump
laine at laine.org
Fri Mar 22 17:11:34 UTC 2019
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 at redhat.com>
> ---
> 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?
Reviewed-by: Laine Stump <laine at laine.org>
assuming that you fix the missing assignment of actual->type in the
first function!
> + virUUIDGenerate(port->uuid);
> +
> + memcpy(port->owneruuid, dom->uuid, VIR_UUID_BUFLEN);
> + if (VIR_STRDUP(port->ownername, dom->name) < 0)
> + goto error;
> +
> + if (VIR_STRDUP(port->group, iface->data.network.portgroup) < 0)
> + goto error;
> +
> + memcpy(&port->mac, &iface->mac, VIR_MAC_BUFLEN);
> +
> + switch (virDomainNetGetActualType(iface)) {
> + case VIR_DOMAIN_NET_TYPE_BRIDGE:
> + port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE;
> + if (VIR_STRDUP(port->plug.bridge.brname,
> + actual->data.bridge.brname) < 0)
> + goto error;
> + port->plug.bridge.macTableManager = actual->data.bridge.macTableManager;
> + break;
> +
> + case VIR_DOMAIN_NET_TYPE_DIRECT:
> + port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_DIRECT;
> + if (VIR_STRDUP(port->plug.direct.linkdev,
> + actual->data.direct.linkdev) < 0)
> + goto error;
> + port->plug.direct.mode = actual->data.direct.mode;
> + break;
> +
> + case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> + port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI;
> + if (actual->data.hostdev.def.mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
> + actual->data.hostdev.def.source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Actual interface '%s' hostdev was not a PCI device"),
> + iface->ifname);
> + goto error;
> + }
> + port->plug.hostdevpci.managed = actual->data.hostdev.def.managed;
> + port->plug.hostdevpci.addr = actual->data.hostdev.def.source.subsys.u.pci.addr;
> + switch ((virDomainHostdevSubsysPCIBackendType)actual->data.hostdev.def.source.subsys.u.pci.backend) {
> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
> + port->plug.hostdevpci.driver = VIR_NETWORK_FORWARD_DRIVER_NAME_DEFAULT;
> + break;
> +
> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
> + port->plug.hostdevpci.driver = VIR_NETWORK_FORWARD_DRIVER_NAME_KVM;
> + break;
> +
> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO:
> + port->plug.hostdevpci.driver = VIR_NETWORK_FORWARD_DRIVER_NAME_VFIO;
> + break;
> +
> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Unexpected PCI backend 'xen'"));
> + break;
> +
> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST:
> + default:
> + virReportEnumRangeError(virDomainHostdevSubsysPCIBackendType,
> + actual->data.hostdev.def.source.subsys.u.pci.backend);
> + goto error;
> + }
> +
> + break;
> +
> + case VIR_DOMAIN_NET_TYPE_CLIENT:
> + case VIR_DOMAIN_NET_TYPE_ETHERNET:
> + case VIR_DOMAIN_NET_TYPE_INTERNAL:
> + case VIR_DOMAIN_NET_TYPE_MCAST:
> + case VIR_DOMAIN_NET_TYPE_NETWORK:
> + case VIR_DOMAIN_NET_TYPE_SERVER:
> + case VIR_DOMAIN_NET_TYPE_UDP:
> + case VIR_DOMAIN_NET_TYPE_USER:
> + case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Unexpected network port type %s"),
> + virDomainNetTypeToString(virDomainNetGetActualType(iface)));
> + goto error;
> +
> + case VIR_DOMAIN_NET_TYPE_LAST:
> + default:
> + virReportEnumRangeError(virNetworkPortPlugType, port->plugtype);
> + goto error;
> + }
> +
> + if (virNetDevVPortProfileCopy(&port->virtPortProfile, actual->virtPortProfile) < 0)
> + goto error;
> +
> + if (virNetDevBandwidthCopy(&port->bandwidth, actual->bandwidth) < 0)
> + goto error;
> +
> + if (virNetDevVlanCopy(&port->vlan, &actual->vlan) < 0)
> + goto error;
> +
> + port->class_id = actual->class_id;
> + port->trustGuestRxFilters = actual->trustGuestRxFilters;
> +
> + return port;
> +
> + error:
> + virNetworkPortDefFree(port);
> + return NULL;
> +}
> +
> static virDomainNetAllocateActualDeviceImpl netAllocate;
> static virDomainNetNotifyActualDeviceImpl netNotify;
> static virDomainNetReleaseActualDeviceImpl netRelease;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 30aa985344..546ee181b1 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3510,6 +3510,23 @@ bool
> virDomainDefLifecycleActionAllowed(virDomainLifecycle type,
> virDomainLifecycleAction action);
>
> +// Forward decl to avoid pulling in virnetworkportdef.h because
> +// that pulls in virhostdev.h which pulls in domain_conf.h (evil)
> +typedef struct _virNetworkPortDef virNetworkPortDef;
> +typedef virNetworkPortDef *virNetworkPortDefPtr;
> +
> +virNetworkPortDefPtr
> +virDomainNetDefToNetworkPort(virDomainDefPtr dom,
> + virDomainNetDefPtr iface);
> +
> +int
> +virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface,
> + virNetworkPortDefPtr port);
> +
> +virNetworkPortDefPtr
> +virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
> + virDomainNetDefPtr iface);
> +
> typedef int
> (*virDomainNetAllocateActualDeviceImpl)(virNetworkPtr net,
> virDomainDefPtr dom,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 1e405cbe5f..b23d3c9891 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -457,9 +457,12 @@ virDomainNetAllocateActualDevice;
> virDomainNetAppendIPAddress;
> virDomainNetBandwidthChangeAllowed;
> virDomainNetBandwidthUpdate;
> +virDomainNetDefActualFromNetworkPort;
> +virDomainNetDefActualToNetworkPort;
> virDomainNetDefClear;
> virDomainNetDefFormat;
> virDomainNetDefFree;
> +virDomainNetDefToNetworkPort;
> virDomainNetFind;
> virDomainNetFindByName;
> virDomainNetFindIdx;
More information about the libvir-list
mailing list