From e3be969238c715142642a1429630f2f0bcb5b293 Mon Sep 17 00:00:00 2001

Message-Id: <e3be969238c715142642a1429630f2f0bcb5b293.1383922565.git.jdenemar@redhat.com>

From: Laine Stump <laine@laine.org>

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 <jdenemar@redhat.com>

---

 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 <source mode='xxx'/> */

         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