From e3be969238c715142642a1429630f2f0bcb5b293 Mon Sep 17 00:00:00 2001 Message-Id: From: Laine Stump Date: Wed, 6 Nov 2013 04:47:59 -0700 Subject: [PATCH] network: fix connections count in case of allocate failure This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1020135 If networkAllocateActualDevice() had failed due to a pool of hostdev or direct devices being depleted, the calling function could still call networkReleaseActualDevice() as part of its cleanup, and that function would then unconditionally decrement the connections count for the network, even though it hadn't been incremented (due to failure of allocate). This *was* necessary because the .actual member of the netdef was allocated with a "lazy" algorithm, only being created if there was a need to store data there (e.g. if a device was allocated from a pool, or bandwidth was allocated for the device), so there was no simple way for networkReleaseActualDevice() to tell if something really had been allocated (i.e. if "connections++" had been executed). This patch changes networkAllocateDevice() to *always* allocate an actual device for any netdef of type='network', even if it isn't needed for any other reason. This has no ill effects anywhere else in the code (except for using a small amount of memory), and networkReleaseActualDevice() can then determine if there was a previous successful allocate by checking for .actual != NULL (if not, it skips the "connections--"). (cherry picked from commit b4e0299d4ff059c8707a760b7ec2063ccd57cc21) Signed-off-by: Jiri Denemark --- src/network/bridge_driver.c | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ea09663..c900478 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3841,6 +3841,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } netdef = network->def; + if (VIR_ALLOC(iface->data.network.actual) < 0) + goto error; + /* portgroup can be present for any type of network, in particular * for bandwidth information, so we need to check for that and * fill it in appropriately for all forward types. @@ -3857,14 +3860,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) else if (portgroup && portgroup->bandwidth) bandwidth = portgroup->bandwidth; - if (bandwidth) { - if (!iface->data.network.actual && - VIR_ALLOC(iface->data.network.actual) < 0) - goto error; - if (virNetDevBandwidthCopy(&iface->data.network.actual->bandwidth, - bandwidth) < 0) - goto error; - } + if (bandwidth && virNetDevBandwidthCopy(&iface->data.network.actual->bandwidth, + bandwidth) < 0) + goto error; /* copy appropriate vlan info to actualNet */ if (iface->vlan.nTags > 0) @@ -3874,13 +3872,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) else if (netdef->vlan.nTags > 0) vlan = &netdef->vlan; - if (vlan) { - if (!iface->data.network.actual && - VIR_ALLOC(iface->data.network.actual) < 0) - goto error; - if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0) - goto error; - } + if (vlan && virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0) + goto error; if ((netdef->forward.type == VIR_NETWORK_FORWARD_NONE) || (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) || @@ -3889,8 +3882,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) *NETWORK; we just keep the info from the portgroup in * iface->data.network.actual */ - if (iface->data.network.actual) - iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK; + iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK; if (networkPlugBandwidth(network, iface) < 0) goto error; @@ -3902,10 +3894,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) * is VIR_DOMAIN_NET_TYPE_BRIDGE */ - if (!iface->data.network.actual && - VIR_ALLOC(iface->data.network.actual) < 0) - goto error; - iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_BRIDGE; if (VIR_STRDUP(iface->data.network.actual->data.bridge.brname, netdef->bridge) < 0) @@ -3938,10 +3926,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) virDomainHostdevSubsysPciBackendType backend; - if (!iface->data.network.actual && - VIR_ALLOC(iface->data.network.actual) < 0) - goto error; - iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_HOSTDEV; if (netdef->forward.npfs > 0 && netdef->forward.nifs <= 0 && networkCreateInterfacePool(netdef) < 0) { @@ -4025,10 +4009,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) * VIR_DOMAIN_NET_TYPE_DIRECT. */ - if (!iface->data.network.actual && - VIR_ALLOC(iface->data.network.actual) < 0) - goto error; - /* Set type=direct and appropriate */ iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_DIRECT; switch (netdef->forward.type) { @@ -4503,7 +4483,8 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) } success: - netdef->connections--; + if (iface->data.network.actual) + netdef->connections--; VIR_DEBUG("Releasing network %s, %d connections", netdef->name, netdef->connections); ret = 0; -- 1.8.4.2