From 3e1dd6eacd48107e7646c8346bca73e280692542 Mon Sep 17 00:00:00 2001

Message-Id: <3e1dd6eacd48107e7646c8346bca73e280692542@dist-git>

From: Michal Privoznik <mprivozn@redhat.com>

Date: Fri, 14 Aug 2015 16:06:31 +0200

Subject: [PATCH] bridge_driver: Introduce networkBandwidthChangeAllowed

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

When a domain vNIC's bandwidth is to be changed (at runtime) it is

possible that guaranteed minimal bandwidth (@floor) will change too.

Well, so far it is, because we still don't have an implementation that

allows setting it dynamically, so it's effectively erased on:

    #virsh domiftune $dom vnet0 --inbound 0

However, that's slightly unfortunate. We do some checks on domain

startup to see if @floor can be guaranteed. We ought do the same if

QoS is changed at runtime.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

(cherry picked from commit 41a1531de54476310572f694c0b9cb9c63d4191d)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/network/bridge_driver.c | 78 ++++++++++++++++++++++++++++++++++++++++++---

 src/network/bridge_driver.h | 12 +++++++

 2 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c

index 17fc430..04602ed 100644

--- a/src/network/bridge_driver.c

+++ b/src/network/bridge_driver.c

@@ -4686,9 +4686,18 @@ networkGetNetworkAddress(const char *netname, char **netaddr)

  * networkCheckBandwidth:

  * @net: network QoS

  * @ifaceBand: interface QoS (may be NULL if no QoS)

+ * @oldBandwidth: new interface QoS (may be NULL if no QoS)

  * @ifaceMac: interface MAC (used in error messages for identification)

  * @new_rate: new rate for non guaranteed class

  *

+ * Function checks if @ifaceBand can be satisfied on @net. However, sometimes it

+ * may happen that the interface that @ifaceBand corresponds to is already

+ * plugged into the @net and the bandwidth is to be updated. In that case we

+ * need to check if new bandwidth can be satisfied. If that's the case

+ * @ifaceBand should point to new bandwidth settings and @oldBandwidth to

+ * current ones. If you want to suppress this functionality just pass

+ * @oldBandwidth == NULL.

+ *

  * Returns: -1 if plugging would overcommit network QoS

  *           0 if plugging is safe (@new_rate updated)

  *           1 if no QoS is set (@new_rate untouched)

@@ -4696,6 +4705,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr)

 static int

 networkCheckBandwidth(virNetworkObjPtr net,

                       virNetDevBandwidthPtr ifaceBand,

+                      virNetDevBandwidthPtr oldBandwidth,

                       virMacAddr ifaceMac,

                       unsigned long long *new_rate)

 {

@@ -4716,14 +4726,18 @@ networkCheckBandwidth(virNetworkObjPtr net,

         return -1;

     }

-    if (!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor ||

+    if (((!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor) &&

+         (!oldBandwidth || !oldBandwidth->in || !oldBandwidth->in->floor)) ||

         !netBand || !netBand->in) {

         /* no QoS required, claim success */

         return 1;

     }

     tmp_new_rate = netBand->in->average;

-    tmp_floor_sum += ifaceBand->in->floor;

+    if (oldBandwidth && oldBandwidth->in)

+        tmp_floor_sum -= oldBandwidth->in->floor;

+    if (ifaceBand && ifaceBand->in)

+        tmp_floor_sum += ifaceBand->in->floor;

     /* check against peak */

     if (netBand->in->peak) {

@@ -4749,7 +4763,8 @@ networkCheckBandwidth(virNetworkObjPtr net,

         goto cleanup;

     }

-    *new_rate = tmp_new_rate;

+    if (new_rate)

+        *new_rate = tmp_new_rate;

     ret = 0;

  cleanup:

@@ -4791,7 +4806,7 @@ networkPlugBandwidth(virNetworkObjPtr net,

     char ifmac[VIR_MAC_STRING_BUFLEN];

     virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);

-    if ((plug_ret = networkCheckBandwidth(net, ifaceBand,

+    if ((plug_ret = networkCheckBandwidth(net, ifaceBand, NULL,

                                           iface->mac, &new_rate)) < 0) {

         /* helper reported error */

         goto cleanup;

@@ -4917,3 +4932,58 @@ networkNetworkObjTaint(virNetworkObjPtr net,

                  virNetworkTaintTypeToString(taint));

     }

 }

+

+

+static bool

+networkBandwidthGenericChecks(virDomainNetDefPtr iface,

+                              virNetDevBandwidthPtr newBandwidth)

+{

+    virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);

+    unsigned long long old_floor, new_floor;

+

+    if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_NETWORK) {

+        /* This is not an interface that's plugged into a network.

+         * We don't care. Thus from our POV bandwidth change is allowed. */

+        return false;

+    }

+

+    old_floor = new_floor = 0;

+

+    if (ifaceBand && ifaceBand->in)

+        old_floor = ifaceBand->in->floor;

+    if (newBandwidth && newBandwidth->in)

+        new_floor = newBandwidth->in->floor;

+

+    return new_floor != old_floor;

+}

+

+

+bool

+networkBandwidthChangeAllowed(virDomainNetDefPtr iface,

+                              virNetDevBandwidthPtr newBandwidth)

+{

+    virNetworkDriverStatePtr driver = networkGetDriver();

+    virNetworkObjPtr network = NULL;

+    virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);

+    bool ret = false;

+

+    if (!networkBandwidthGenericChecks(iface, newBandwidth))

+        return true;

+

+    network = virNetworkObjFindByName(driver->networks, iface->data.network.name);

+    if (!network) {

+        virReportError(VIR_ERR_NO_NETWORK,

+                       _("no network with matching name '%s'"),

+                       iface->data.network.name);

+        return false;

+    }

+

+    if (networkCheckBandwidth(network, newBandwidth, ifaceBand, iface->mac, NULL) < 0)

+        goto cleanup;

+

+    ret = true;

+

+ cleanup:

+    virNetworkObjEndAPI(&network);

+    return ret;

+}

diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h

index 513ccf7..cce9237 100644

--- a/src/network/bridge_driver.h

+++ b/src/network/bridge_driver.h

@@ -52,6 +52,11 @@ int networkDnsmasqConfContents(virNetworkObjPtr network,

                         char **configstr,

                         dnsmasqContext *dctx,

                         dnsmasqCapsPtr caps);

+

+bool networkBandwidthChangeAllowed(virDomainNetDefPtr iface,

+                                   virNetDevBandwidthPtr newBandwidth)

+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

+

 # else

 /* Define no-op replacements that don't drag in any link dependencies.  */

 #  define networkAllocateActualDevice(dom, iface) 0

@@ -73,6 +78,13 @@ networkReleaseActualDevice(virDomainDefPtr dom ATTRIBUTE_UNUSED,

     return 0;

 }

+static inline bool

+networkBandwidthChangeAllowed(virDomainNetDefPtr iface ATTRIBUTE_UNUSED,

+                              virNetDevBandwidthPtr newBandwidth ATTRIBUTE_UNUSED)

+{

+    return true;

+}

+

 # endif

 typedef char *(*networkDnsmasqLeaseFileNameFunc)(const char *netname);

-- 

2.5.0