Re: [libvirt] [PATCH 3/9] bridge_driver: Introduce networkBandwidthChangeAllowed

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(a)redhat.com&gt; --- src/network/bridge_driver.c | 72 +++++++++++++++++++++++++++++++++++++++++++-- src/network/bridge_driver.h | 12 ++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 17fc430..fa60ba4 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) { @@ -4723,6 +4733,8 @@ networkCheckBandwidth(virNetworkObjPtr net, } tmp_new_rate = netBand->in->average; + if (oldBandwidth && oldBandwidth->in) + tmp_floor_sum -= oldBandwidth->in->floor; tmp_floor_sum += ifaceBand->in->floor; /* check against peak */ @@ -4749,7 +4761,8 @@ networkCheckBandwidth(virNetworkObjPtr net, goto cleanup; } - *new_rate = tmp_new_rate; + if (new_rate) + *new_rate = tmp_new_rate; ret = 0; cleanup: @@ -4791,7 +4804,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 +4930,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);