From 3e1dd6eacd48107e7646c8346bca73e280692542 Mon Sep 17 00:00:00 2001 Message-Id: <3e1dd6eacd48107e7646c8346bca73e280692542@dist-git> From: Michal Privoznik 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 (cherry picked from commit 41a1531de54476310572f694c0b9cb9c63d4191d) Signed-off-by: Michal Privoznik Signed-off-by: Jiri Denemark --- 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