[libvirt] [PATCH] network: unplug bandwidth and call networkRunHook only when appropriate
According to commit b4e0299dm if networkAllocateActualDevice() was successful, it will *always* allocate an iface->data.network.actual, so we can use this during networkReleaseActualDevice() to know if there is really anything to undo. We were properly using this information to only decrement the network connections counter if it had previously been incremented, but we were unconditionally unplugging bandwidth and calling the "unplugged" network hook for *all* interfaces (during qemuProcessStop()) whether they had been previously plugged or not. This caused problems if a domain failed to start at some time prior to all interfaces being allocated. (I encountered this when an interface had a bandwidth floor set but no inbound QoS). This patch changes both the call to networkUnplugBandwidth() and the call to networkRunHook() to only be called if there was a previous call to "plug" for the same interface. --- src/network/bridge_driver.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3fb5ad3..c797f8f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3936,7 +3936,8 @@ networkReleaseActualDevice(virDomainDefPtr dom, } netdef = network->def; - if ((netdef->forward.type == VIR_NETWORK_FORWARD_NONE || + if (iface->data.network.actual && + (netdef->forward.type == VIR_NETWORK_FORWARD_NONE || netdef->forward.type == VIR_NETWORK_FORWARD_NAT || netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE) && networkUnplugBandwidth(network, iface) < 0) @@ -4029,15 +4030,15 @@ networkReleaseActualDevice(virDomainDefPtr dom, } success: - if (iface->data.network.actual) + if (iface->data.network.actual) { netdef->connections--; + VIR_DEBUG("Releasing network %s, %d connections", + netdef->name, netdef->connections); - /* finally we can call the 'unplugged' hook script if any */ - networkRunHook(network, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, - VIR_HOOK_SUBOP_BEGIN); - - VIR_DEBUG("Releasing network %s, %d connections", - netdef->name, netdef->connections); + /* finally we can call the 'unplugged' hook script if any */ + networkRunHook(network, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, + VIR_HOOK_SUBOP_BEGIN); + } ret = 0; cleanup: if (network) -- 1.8.5.3
On 02/25/2014 08:47 AM, Laine Stump wrote:
According to commit b4e0299dm if networkAllocateActualDevice() was successful, it will *always* allocate an iface->data.network.actual, so we can use this during networkReleaseActualDevice() to know if there is really anything to undo. We were properly using this information to only decrement the network connections counter if it had previously been incremented, but we were unconditionally unplugging bandwidth and calling the "unplugged" network hook for *all* interfaces (during qemuProcessStop()) whether they had been previously plugged or not. This caused problems if a domain failed to start at some time prior to all interfaces being allocated. (I encountered this when an interface had a bandwidth floor set but no inbound QoS).
This patch changes both the call to networkUnplugBandwidth() and the call to networkRunHook() to only be called if there was a previous call to "plug" for the same interface. --- src/network/bridge_driver.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 25.02.2014 16:47, Laine Stump wrote:
According to commit b4e0299dm if networkAllocateActualDevice() was
The 'm' at the end of has is a typo. s/dm/d,/
successful, it will *always* allocate an iface->data.network.actual, so we can use this during networkReleaseActualDevice() to know if there is really anything to undo. We were properly using this information to only decrement the network connections counter if it had previously been incremented, but we were unconditionally unplugging bandwidth and calling the "unplugged" network hook for *all* interfaces (during qemuProcessStop()) whether they had been previously plugged or not. This caused problems if a domain failed to start at some time prior to all interfaces being allocated. (I encountered this when an interface had a bandwidth floor set but no inbound QoS).
This patch changes both the call to networkUnplugBandwidth() and the call to networkRunHook() to only be called if there was a previous call to "plug" for the same interface. --- src/network/bridge_driver.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
ACK Michal
participants (3)
-
Eric Blake -
Laine Stump -
Michal Privoznik