network: fill in bandwidth from portgroup for all forward modes

This patch is a fix for:

  https://bugzilla.redhat.com/show_bug.cgi?id=743176

which was discovered by Dan Berrange while making bandwidth
configuration work for LXC guests.

Background: Although virtportprofile data from a network portgroup is
only applicable for direct mode interfaces, the code that copies
bandwidth data from the portgroup was also only being executed in the
case of direct mode interfaces. The result was that interfaces using
traditional virtual networks (forward mode='nat|route|none'), and
those using a host bridge for forwarding, would not pick up bandwidth
data from a portgroup defined in the network.

This patch moves that code outside the conditional, so that bandwidth
information is *alway* copied from the appropriate portgroup (unless
the <interface> definition itself already has bandwidth information,
which would take precedence over what's in the portgroup anyway).

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6fb188878efe3656fdaa910fb8cf6d727410c99e..ab1249b704eb82d9383bfbb275b3f48de914f002 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2934,7 +2934,8 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
         goto error;
     }
     if (actual->type != VIR_DOMAIN_NET_TYPE_BRIDGE &&
-        actual->type != VIR_DOMAIN_NET_TYPE_DIRECT) {
+        actual->type != VIR_DOMAIN_NET_TYPE_DIRECT &&
+        actual->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
         virDomainReportError(VIR_ERR_INTERNAL_ERROR,
                              _("unsupported type '%s' in interface's <actual> element"),
                              type);
@@ -9406,7 +9407,8 @@ virDomainActualNetDefFormat(virBufferPtr buf,
     }
 
     if (def->type != VIR_DOMAIN_NET_TYPE_BRIDGE &&
-        def->type != VIR_DOMAIN_NET_TYPE_DIRECT) {
+        def->type != VIR_DOMAIN_NET_TYPE_DIRECT &&
+        def->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
         virDomainReportError(VIR_ERR_INTERNAL_ERROR,
                              _("unexpected net type %s"), type);
         goto error;

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 51023a324780cb891d1d48029d5ae1a295c6f0dd..445c3cbb67427a95073e60ad11dbd0651122e489 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2753,6 +2753,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
     struct network_driver *driver = driverState;
     virNetworkObjPtr network;
     virNetworkDefPtr netdef;
+    virPortGroupDefPtr portgroup;
     int ret = -1;
 
     if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
@@ -2772,14 +2773,48 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
     }
 
     netdef = network->def;
-    if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) &&
-        netdef->bridge) {
+
+    /* portgroup can be present for any type of network, in particular
+     * for bandwidth information, so we need to check for that and
+     * fill it in appropriately for all forward types.
+    */
+    portgroup = virPortGroupFindByName(netdef, iface->data.network.portgroup);
+
+    /* If there is already interface-specific bandwidth, just use that
+     * (already in NetDef). Otherwise, if there is bandwidth info in
+     * the portgroup, fill that into the ActualDef.
+     */
+    if (portgroup && !iface->bandwidth) {
+        if (!iface->data.network.actual
+            && (VIR_ALLOC(iface->data.network.actual) < 0)) {
+            virReportOOMError();
+            goto cleanup;
+        }
+
+        if (virBandwidthCopy(&iface->data.network.actual->bandwidth,
+                             portgroup->bandwidth) < 0) {
+            goto cleanup;
+        }
+    }
+
+    if ((netdef->forwardType == VIR_NETWORK_FORWARD_NONE) ||
+        (netdef->forwardType == VIR_NETWORK_FORWARD_NAT) ||
+        (netdef->forwardType == VIR_NETWORK_FORWARD_ROUTE)) {
+        /* for these forward types, the actual net type really *is*
+         *NETWORK; we just keep the info from the portgroup in
+         * iface->data.network.actual
+        */
+        if (iface->data.network.actual)
+            iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK;
+    } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) &&
+               netdef->bridge) {
 
         /* <forward type='bridge'/> <bridge name='xxx'/>
          * is VIR_DOMAIN_NET_TYPE_BRIDGE
          */
 
-        if (VIR_ALLOC(iface->data.network.actual) < 0) {
+        if (!iface->data.network.actual
+            && (VIR_ALLOC(iface->data.network.actual) < 0)) {
             virReportOOMError();
             goto cleanup;
         }
@@ -2796,13 +2831,13 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
                (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) ||
                (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
         virVirtualPortProfileParamsPtr virtport = NULL;
-        virPortGroupDefPtr portgroup = NULL;
 
         /* <forward type='bridge|private|vepa|passthrough'> are all
          * VIR_DOMAIN_NET_TYPE_DIRECT.
          */
 
-        if (VIR_ALLOC(iface->data.network.actual) < 0) {
+        if (!iface->data.network.actual
+            && (VIR_ALLOC(iface->data.network.actual) < 0)) {
             virReportOOMError();
             goto cleanup;
         }
@@ -2825,7 +2860,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
         }
 
         /* Find the most specific virtportprofile and copy it */
-        portgroup = virPortGroupFindByName(netdef, iface->data.network.portgroup);
         if (iface->data.network.virtPortProfile) {
             virtport = iface->data.network.virtPortProfile;
         } else {
@@ -2845,20 +2879,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
             *iface->data.network.actual->data.direct.virtPortProfile = *virtport;
         }
 
-        /* Find the most specific bandwidth config and copy it */
-        if (iface->bandwidth) {
-            if (virBandwidthCopy(&iface->data.network.actual->bandwidth,
-                                 iface->bandwidth) < 0) {
-                goto cleanup;
-            }
-        } else {
-            if (portgroup &&
-                virBandwidthCopy(&iface->data.network.actual->bandwidth,
-                                 portgroup->bandwidth) < 0) {
-                goto cleanup;
-            }
-        }
-
         /* If there is only a single device, just return it (caller will detect
          * any error if exclusive use is required but could not be acquired).
          */