[libvirt] [PATCH] network: fill in bandwidth from portgroup for all forward modes

[Laine Stump]

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).
---
 src/conf/domain_conf.c      |    6 +++-
 src/network/bridge_driver.c |   60 ++++++++++++++++++++++++++++--------------
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6fb1888..ab1249b 100644
--- 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 51023a3..445c3cb 100644
--- 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).
          */
-- 
1.7.3.4