From 9d6920bd7de3f92be1894790adeb689060ab25eb Mon Sep 17 00:00:00 2001
From: Laine Stump <laine@redhat.com>
Date: Tue, 1 Oct 2019 14:05:58 -0400
Subject: [PATCH] net/qemu: move vlan/bandwidth validation out of network
 driver

In the past the network driver was (mistakenly) being called for all
interfaces, not just those of type='network', and so it had a chance
to validate all interface configs after the actual type of the
interface was known.

But since the network driver has been more completely/properly
separated from qemu, the network driver isn't called during the
startup of any interfaces except those with type='network', so this
validation no longer takes place for, e.g. <interface type='bridge'>
(or direct, etc). This in turn meant that a config could erroneously
specify a vlan tag, or bandwidth settings, for a type of interface
that didn't support it, and the domain would start without complaint,
just silently ignoring those settings.

This patch moves those validation checks out of the network driver,
and into virDomainActualNetDefValidate() so they will be done for all
interfaces, not just type='network'.

https://bugzilla.redhat.com/1741121
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
---
 src/conf/domain_conf.c      | 38 +++++++++++++++++++++++++++++++++++--
 src/network/bridge_driver.c | 37 ------------------------------------
 2 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1b59d04d34..9580884747 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6135,7 +6135,7 @@ virDomainRedirdevDefValidate(const virDomainDef *def,
 
 
 int
-virDomainActualNetDefValidate(const virDomainNetDef *net G_GNUC_UNUSED)
+virDomainActualNetDefValidate(const virDomainNetDef *net)
 {
     /* Unlike virDomainNetDefValidate(), which is a static function
      * called internally to this file, virDomainActualNetDefValidate()
@@ -6150,9 +6150,43 @@ virDomainActualNetDefValidate(const virDomainNetDef *net G_GNUC_UNUSED)
      * is allowed for a type of interface), but *not*
      * hypervisor-specific things.
      */
+    char macstr[VIR_MAC_STRING_BUFLEN];
+    virDomainNetType actualType = virDomainNetGetActualType(net);
+    const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net);
+    const virNetDevBandwidth *bandwidth = virDomainNetGetActualBandwidth(net);
 
-    return 0;
+    virMacAddrFormat(&net->mac, macstr);
 
+    if (virDomainNetGetActualVlan(net)) {
+        /* vlan configuration via libvirt is only supported for PCI
+         * Passthrough SR-IOV devices (hostdev or macvtap passthru
+         * mode) and openvswitch bridges. Otherwise log an error and
+         * fail
+         */
+        if (!(actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV ||
+              (actualType == VIR_DOMAIN_NET_TYPE_DIRECT &&
+               virDomainNetGetActualDirectMode(net) == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) ||
+              (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE &&
+               vport  && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("interface %s - vlan tag not supported for this connection type"),
+                           macstr);
+            return -1;
+        }
+    }
+
+    /* bandwidth configuration via libvirt is not supported for
+     * hostdev network devices
+     */
+    if (bandwidth && actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("interface %s - bandwidth settings are not supported "
+                         "for hostdev interfaces"),
+                       macstr);
+        return -1;
+    }
+
+    return 0;
 }
 
 
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 68bb916501..07dba8cfe4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4739,43 +4739,6 @@ networkAllocatePort(virNetworkObjPtr obj,
     if (virNetDevVPortProfileCheckComplete(port->virtPortProfile, true) < 0)
         return -1;
 
-    /* make sure that everything now specified for the device is
-     * actually supported on this type of network. NB: network,
-     * netdev, and iface->data.network.actual may all be NULL.
-     */
-    VIR_DEBUG("Sanity check port config");
-
-    if (port->vlan.nTags) {
-        /* vlan configuration via libvirt is only supported for PCI
-         * Passthrough SR-IOV devices (hostdev or macvtap passthru
-         * mode) and openvswitch bridges. Otherwise log an error and
-         * fail
-         */
-        if (!(port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI ||
-              (port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_DIRECT &&
-               port->plug.direct.mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) ||
-              (port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE &&
-               port->virtPortProfile &&
-               port->virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("an interface connecting to network '%s' "
-                             "is requesting a vlan tag, but that is not "
-                             "supported for this type of network"),
-                           netdef->name);
-            return -1;
-        }
-    }
-
-    /* bandwidth configuration via libvirt is not supported for
-     * hostdev network devices
-     */
-    if (port->bandwidth && port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("bandwidth settings are not supported "
-                         "for hostdev interfaces"));
-        return -1;
-    }
-
     netdef->connections++;
     if (dev)
         dev->connections++;
-- 
GitLab