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 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?

Reviewed-by: Laine Stump <laine@xxxxxxxxx>


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;


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