From f02e21cb3379a41cd42f2d8116f2d10dabace83b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Tue, 26 Feb 2019 14:49:35 +0000 Subject: [PATCH] network: remove the virDomainNetBandwidthChangeAllowed callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current qemu driver code for changing bandwidth on a NIC first asks the network driver if the change is supported, then changes the bandwidth on the VIF, and then tells the network driver to update the bandwidth on the bridge. This is potentially racing if a parallel API call causes the network driver to allocate bandwidth on the bridge between the check and the update phases. Change the code to just try to apply the network bridge update immediately and rollback at the end if something failed. Reviewed-by: Laine Stump Signed-off-by: Daniel P. Berrangé --- src/conf/domain_conf.c | 15 -------- src/conf/domain_conf.h | 10 ----- src/libvirt_private.syms | 1 - src/network/bridge_driver.c | 77 ++++++++----------------------------- src/qemu/qemu_driver.c | 8 ++-- 5 files changed, 21 insertions(+), 90 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6f704f4e5d..85b374572c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30670,7 +30670,6 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom, static virDomainNetAllocateActualDeviceImpl netAllocate; static virDomainNetNotifyActualDeviceImpl netNotify; static virDomainNetReleaseActualDeviceImpl netRelease; -static virDomainNetBandwidthChangeAllowedImpl netBandwidthChangeAllowed; static virDomainNetBandwidthUpdateImpl netBandwidthUpdate; @@ -30678,13 +30677,11 @@ void virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, virDomainNetNotifyActualDeviceImpl notify, virDomainNetReleaseActualDeviceImpl release, - virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed, virDomainNetBandwidthUpdateImpl bandwidthUpdate) { netAllocate = allocate; netNotify = notify; netRelease = release; - netBandwidthChangeAllowed = bandwidthChangeAllowed; netBandwidthUpdate = bandwidthUpdate; } @@ -30767,18 +30764,6 @@ virDomainNetReleaseActualDevice(virConnectPtr conn, return ret; } -bool -virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth) -{ - if (!netBandwidthChangeAllowed) { - virReportError(VIR_ERR_NO_SUPPORT, "%s", - _("Virtual networking driver is not available")); - return -1; - } - - return netBandwidthChangeAllowed(iface, newBandwidth); -} int virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d88c29fad5..6ba0643cd1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3584,10 +3584,6 @@ typedef int virDomainDefPtr dom, virDomainNetDefPtr iface); -typedef bool -(*virDomainNetBandwidthChangeAllowedImpl)(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth); - typedef int (*virDomainNetBandwidthUpdateImpl)(virDomainNetDefPtr iface, virNetDevBandwidthPtr newBandwidth); @@ -3597,7 +3593,6 @@ void virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, virDomainNetNotifyActualDeviceImpl notify, virDomainNetReleaseActualDeviceImpl release, - virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed, virDomainNetBandwidthUpdateImpl bandwidthUpdate); int @@ -3618,11 +3613,6 @@ virDomainNetReleaseActualDevice(virConnectPtr conn, virDomainNetDefPtr iface) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -bool -virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - int virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, virNetDevBandwidthPtr newBandwidth) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6b030e3de3..c4bc60fb24 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -456,7 +456,6 @@ virDomainMemorySourceTypeFromString; virDomainMemorySourceTypeToString; virDomainNetAllocateActualDevice; virDomainNetAppendIPAddress; -virDomainNetBandwidthChangeAllowed; virDomainNetBandwidthUpdate; virDomainNetDefActualFromNetworkPort; virDomainNetDefActualToNetworkPort; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 95b7a7cce9..fe00a4fd41 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -5349,64 +5349,6 @@ networkNetworkObjTaint(virNetworkObjPtr obj, } -static bool -networkBandwidthGenericChecks(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth) -{ - virNetDevBandwidthPtr ifaceBand; - unsigned long long old_floor, new_floor; - - if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_NETWORK && - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_BRIDGE || - iface->data.network.actual->data.bridge.brname == NULL)) { - /* 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; - } - - ifaceBand = virDomainNetGetActualBandwidth(iface); - 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; -} - - -static bool -networkBandwidthChangeAllowed(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth) -{ - virNetworkDriverStatePtr driver = networkGetDriver(); - virNetworkObjPtr obj = NULL; - virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); - bool ret = false; - - if (!networkBandwidthGenericChecks(iface, newBandwidth)) - return true; - - obj = virNetworkObjFindByName(driver->networks, iface->data.network.name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - iface->data.network.name); - return false; - } - - if (networkCheckBandwidth(obj, newBandwidth, ifaceBand, &iface->mac, NULL) < 0) - goto cleanup; - - ret = true; - - cleanup: - virNetworkObjEndAPI(&obj); - return ret; -} - - static int networkBandwidthUpdate(virDomainNetDefPtr iface, virNetDevBandwidthPtr newBandwidth) @@ -5417,6 +5359,7 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, unsigned long long tmp_floor_sum; virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); unsigned long long new_rate = 0; + unsigned long long old_floor, new_floor; int plug_ret; int ret = -1; @@ -5426,7 +5369,22 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, return -1; } - if (!networkBandwidthGenericChecks(iface, newBandwidth)) + if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_NETWORK && + (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_BRIDGE || + iface->data.network.actual->data.bridge.brname != NULL)) { + /* This is not an interface that's plugged into a bridge. + * We don't care. Thus from our POV bandwidth change is allowed. */ + return 0; + } + + old_floor = new_floor = 0; + + if (ifaceBand && ifaceBand->in) + old_floor = ifaceBand->in->floor; + if (newBandwidth && newBandwidth->in) + new_floor = newBandwidth->in->floor; + + if (new_floor == old_floor) return 0; obj = virNetworkObjFindByName(driver->networks, iface->data.network.name); @@ -5572,7 +5530,6 @@ networkRegister(void) networkAllocateActualDevice, networkNotifyActualDevice, networkReleaseActualDevice, - networkBandwidthChangeAllowed, networkBandwidthUpdate); return 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f623eaa422..b6ff038635 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11625,17 +11625,17 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, } if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && - !virDomainNetBandwidthChangeAllowed(net, newBandwidth)) + virDomainNetBandwidthUpdate(net, newBandwidth) < 0) goto endjob; if (virNetDevBandwidthSet(net->ifname, newBandwidth, false, - !virDomainNetTypeSharesHostView(net)) < 0 || - (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && - virDomainNetBandwidthUpdate(net, newBandwidth) < 0)) { + !virDomainNetTypeSharesHostView(net)) < 0) { ignore_value(virNetDevBandwidthSet(net->ifname, net->bandwidth, false, !virDomainNetTypeSharesHostView(net))); + ignore_value(virDomainNetBandwidthUpdate(net, + net->bandwidth)); goto endjob; } -- GitLab