network: restrict usage of port management APIs

The port allocation APIs are currently called unconditionally for all
types of NIC, but (mostly) only do anything for NICs with type=network.

The exception is the port allocate API which does some validation even
for NICs with type!=network. Relying on this validation is flawed,
however, since the network driver may not even be installed. IOW virt
drivers must not delegate validation to the network driver for NICs
with type != network.

This change allows us to report errors when the virtual network driver
is not registered.

Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 17e8975be3999e496200de983a66b2b223ddbfdf..25207284930a583155a79434772e311a9adcb568 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30307,13 +30307,11 @@ int
 virDomainNetAllocateActualDevice(virDomainDefPtr dom,
                                  virDomainNetDefPtr iface)
 {
-    /* Just silently ignore if network driver isn't present. If something
-     * has tried to use a NIC with type=network, other code will already
-     * cause an error. This ensures type=bridge doesn't break when
-     * network driver is compiled out.
-     */
-    if (!netAllocate)
-        return 0;
+    if (!netAllocate) {
+        virReportError(VIR_ERR_NO_SUPPORT, "%s",
+                       _("Virtual networking driver is not available"));
+        return -1;
+    }
 
     return netAllocate(dom, iface);
 }
@@ -30343,8 +30341,11 @@ bool
 virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface,
                                    virNetDevBandwidthPtr newBandwidth)
 {
-    if (!netBandwidthChangeAllowed)
-        return 0;
+    if (!netBandwidthChangeAllowed) {
+        virReportError(VIR_ERR_NO_SUPPORT, "%s",
+                       _("Virtual networking driver is not available"));
+        return -1;
+    }
 
     return netBandwidthChangeAllowed(iface, newBandwidth);
 }
@@ -30353,8 +30354,11 @@ int
 virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
                             virNetDevBandwidthPtr newBandwidth)
 {
-    if (!netBandwidthUpdate)
-        return 0;
+    if (!netBandwidthUpdate) {
+        virReportError(VIR_ERR_NO_SUPPORT, "%s",
+                       _("Virtual networking driver is not available"));
+        return -1;
+    }
 
     return netBandwidthUpdate(iface, newBandwidth);
 }

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 6e4bf68921f344ad4c6ba6e21e4cbe32250818da..adc6d6d1047d646cbab7de28fecd61482eb4c18a 100644 (file)
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -904,7 +904,8 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
 
             /* cleanup actual device */
             virDomainNetRemoveHostdev(vm->def, net);
-            virDomainNetReleaseActualDevice(vm->def, net);
+            if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+                virDomainNetReleaseActualDevice(vm->def, net);
         }
     }
 
@@ -1061,7 +1062,8 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
          * network's pool of devices, or resolve bridge device name
          * to the one defined in the network definition.
          */
-        if (virDomainNetAllocateActualDevice(def, net) < 0)
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+            virDomainNetAllocateActualDevice(def, net) < 0)
             return -1;
 
         actualType = virDomainNetGetActualType(net);

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a9edc8211d9a660c96c5a7d6f3a39c514c853feb..d5cd3fc8344263d3b0f2158adc91ed9c1e05c9df 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3390,7 +3390,8 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
      * network's pool of devices, or resolve bridge device name
      * to the one defined in the network definition.
      */
-    if (virDomainNetAllocateActualDevice(vm->def, net) < 0)
+    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+        virDomainNetAllocateActualDevice(vm->def, net) < 0)
         goto cleanup;
 
     actualType = virDomainNetGetActualType(net);
@@ -3440,7 +3441,8 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
         vm->def->nets[vm->def->nnets++] = net;
     } else {
         virDomainNetRemoveHostdev(vm->def, net);
-        virDomainNetReleaseActualDevice(vm->def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, net);
     }
     virObjectUnref(cfg);
     return ret;
@@ -3863,7 +3865,8 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
  cleanup:
     libxl_device_nic_dispose(&nic);
     if (!ret) {
-        virDomainNetReleaseActualDevice(vm->def, detach);
+        if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, detach);
         virDomainNetRemove(vm->def, detachidx);
     }
     virObjectUnref(cfg);

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e981f8e9010a6bccb81d65ead30d135c52a8e3a1..027c4fd990b16d73043b2c767390fc85de898c5f 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3834,7 +3834,8 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
      * network's pool of devices, or resolve bridge device name
      * to the one defined in the network definition.
      */
-    if (virDomainNetAllocateActualDevice(vm->def, net) < 0)
+    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+        virDomainNetAllocateActualDevice(vm->def, net) < 0)
         return -1;
 
     actualType = virDomainNetGetActualType(net);
@@ -4388,7 +4389,8 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
     ret = 0;
  cleanup:
     if (!ret) {
-        virDomainNetReleaseActualDevice(vm->def, detach);
+        if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, detach);
         virDomainNetRemove(vm->def, detachidx);
         virDomainNetDefFree(detach);
     }

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index e0729a24bf076a035c224e67be0aa9bbe02c7c06..7849aaf5b974303670d72fce1d605397c4b05637 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -224,7 +224,8 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
                                 iface->ifname));
             ignore_value(virNetDevVethDelete(iface->ifname));
         }
-        virDomainNetReleaseActualDevice(vm->def, iface);
+        if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, iface);
     }
 
     virDomainConfVMNWFilterTeardown(vm);
@@ -558,7 +559,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
         if (virLXCProcessValidateInterface(net) < 0)
             goto cleanup;
 
-        if (virDomainNetAllocateActualDevice(def, net) < 0)
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+            virDomainNetAllocateActualDevice(def, net) < 0)
             goto cleanup;
 
         type = virDomainNetGetActualType(net);
@@ -637,7 +639,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
                 ignore_value(virNetDevOpenvswitchRemovePort(
                                 virDomainNetGetActualBridgeName(iface),
                                 iface->ifname));
-            virDomainNetReleaseActualDevice(def, iface);
+            if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+                virDomainNetReleaseActualDevice(def, iface);
         }
     }
     return ret;

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a091ed7181c00abcbc62881523ceb8b303ecee49..cf1abe6bb2c7ba86db09f58b7f52c90e9c2833e3 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4391,8 +4391,11 @@ networkAllocateActualDevice(virDomainDefPtr dom,
     size_t i;
     int ret = -1;
 
-    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
-        goto validate;
+    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Expected an interface for a virtual network"));
+        goto error;
+    }
 
     virDomainActualNetDefFree(iface->data.network.actual);
     iface->data.network.actual = NULL;
@@ -4711,7 +4714,6 @@ networkAllocateActualDevice(virDomainDefPtr dom,
     if (virNetDevVPortProfileCheckComplete(virtport, true) < 0)
         goto error;
 
- validate:
     /* 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.
@@ -4730,19 +4732,11 @@ networkAllocateActualDevice(virDomainDefPtr dom,
               (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE &&
                virtport && virtport->virtPortType
                == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) {
-            if (netdef) {
-                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);
-            } else {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("an interface of type '%s' "
-                                 "is requesting a vlan tag, but that is not "
-                                 "supported for this type of connection"),
-                               virDomainNetTypeToString(iface->type));
-            }
+            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);
             goto error;
         }
     }
@@ -4758,22 +4752,20 @@ networkAllocateActualDevice(virDomainDefPtr dom,
         }
     }
 
-    if (netdef) {
-        netdef->connections++;
+    netdef->connections++;
+    if (dev)
+        dev->connections++;
+    /* finally we can call the 'plugged' hook script if any */
+    if (networkRunHook(obj, dom, iface,
+                       VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,
+                       VIR_HOOK_SUBOP_BEGIN) < 0) {
+        /* adjust for failure */
+        netdef->connections--;
         if (dev)
-            dev->connections++;
-        /* finally we can call the 'plugged' hook script if any */
-        if (networkRunHook(obj, dom, iface,
-                           VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,
-                           VIR_HOOK_SUBOP_BEGIN) < 0) {
-            /* adjust for failure */
-            netdef->connections--;
-            if (dev)
-                dev->connections--;
-            goto error;
-        }
-        networkLogAllocation(netdef, actualType, dev, iface, true);
+            dev->connections--;
+        goto error;
     }
+    networkLogAllocation(netdef, actualType, dev, iface, true);
 
     ret = 0;
 
@@ -4814,8 +4806,11 @@ networkNotifyActualDevice(virDomainDefPtr dom,
     size_t i;
     char *master = NULL;
 
-    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
-        return;
+    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Expected an interface for a virtual network"));
+        goto error;
+    }
 
     obj = virNetworkObjFindByName(driver->networks, iface->data.network.name);
     if (!obj) {
@@ -5047,8 +5042,11 @@ networkReleaseActualDevice(virDomainDefPtr dom,
     size_t i;
     int ret = -1;
 
-    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
-        return 0;
+    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Expected an interface for a virtual network"));
+        goto error;
+    }
 
     obj = virNetworkObjFindByName(driver->networks, iface->data.network.name);
     if (!obj) {
@@ -5539,6 +5537,12 @@ networkBandwidthUpdate(virDomainNetDefPtr iface,
     int plug_ret;
     int ret = -1;
 
+    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Expected an interface for a virtual network"));
+        return -1;
+    }
+
     if (!networkBandwidthGenericChecks(iface, newBandwidth))
         return 0;
 

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4510b0ce6093923db304cf1de15980bf193bd03c..811762f3074e64382f1201460125cec8994693b4 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11623,12 +11623,14 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
                    sizeof(*newBandwidth->out));
         }
 
-        if (!virDomainNetBandwidthChangeAllowed(net, newBandwidth))
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+            !virDomainNetBandwidthChangeAllowed(net, newBandwidth))
             goto endjob;
 
         if (virNetDevBandwidthSet(net->ifname, newBandwidth, false,
                                   !virDomainNetTypeSharesHostView(net)) < 0 ||
-            virDomainNetBandwidthUpdate(net, newBandwidth) < 0) {
+            (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+             virDomainNetBandwidthUpdate(net, newBandwidth) < 0)) {
             ignore_value(virNetDevBandwidthSet(net->ifname,
                                                net->bandwidth,
                                                false,

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ef14b1977cea4cc2fb2f27bf318daac3db30cbeb..48489d3cdcead88ed39f797ecb7129e08c021812 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1385,7 +1385,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
      * network's pool of devices, or resolve bridge device name
      * to the one defined in the network definition.
      */
-    if (virDomainNetAllocateActualDevice(vm->def, net) < 0)
+    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+        virDomainNetAllocateActualDevice(vm->def, net) < 0)
         goto cleanup;
 
     actualType = virDomainNetGetActualType(net);
@@ -1689,7 +1690,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 
         virDomainNetRemoveHostdev(vm->def, net);
 
-        virDomainNetReleaseActualDevice(vm->def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, net);
     }
 
     VIR_FREE(nicstr);
@@ -4126,7 +4128,8 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
 
         /* this function doesn't work with HOSTDEV networks yet, thus
          * no need to change the pointer in the hostdev structure */
-        virDomainNetReleaseActualDevice(vm->def, olddev);
+        if (olddev->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, olddev);
         virDomainNetDefFree(olddev);
         /* move newdev into the nets list, and NULL it out from the
          * virDomainDeviceDef that we were given so that the caller
@@ -4157,7 +4160,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
      * that the changes were minor enough that we didn't need to
      * replace the entire device object.
      */
-    if (newdev)
+    if (newdev && newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK)
         virDomainNetReleaseActualDevice(vm->def, newdev);
 
     return ret;
@@ -4750,7 +4753,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
     virDomainHostdevDefFree(hostdev);
 
     if (net) {
-        virDomainNetReleaseActualDevice(vm->def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, net);
         virDomainNetDefFree(net);
     }
 
@@ -4852,7 +4856,8 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
 
     qemuDomainNetDeviceVportRemove(net);
 
-    virDomainNetReleaseActualDevice(vm->def, net);
+    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+        virDomainNetReleaseActualDevice(vm->def, net);
     virDomainNetDefFree(net);
     ret = 0;
 

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 55f4074ea146baeca415abda290b833011fd1946..8b0ae901e99b54636028e2618716b920939be9a1 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3288,7 +3288,8 @@ qemuProcessNotifyNets(virDomainDefPtr def)
         if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)
            ignore_value(virNetDevMacVLanReserveName(net->ifname, false));
 
-        virDomainNetNotifyActualDevice(def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetNotifyActualDevice(def, net);
     }
 }
 
@@ -5474,7 +5475,8 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def)
          * network's pool of devices, or resolve bridge device name
          * to the one defined in the network definition.
          */
-        if (virDomainNetAllocateActualDevice(def, net) < 0)
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+            virDomainNetAllocateActualDevice(def, net) < 0)
             goto cleanup;
 
         actualType = virDomainNetGetActualType(net);
@@ -7321,7 +7323,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
         /* kick the device out of the hostdev list too */
         virDomainNetRemoveHostdev(def, net);
-        virDomainNetReleaseActualDevice(vm->def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, net);
     }
 
  retry: