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--").

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3ce31302a26c8912c5712bfad0149170abf3120e..41edb97eebebf6a8d3d026c17351b582c0625b77 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3151,6 +3151,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.
@@ -3167,14 +3170,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)
@@ -3184,13 +3182,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) ||
@@ -3199,8 +3192,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;
@@ -3212,10 +3204,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)
@@ -3248,10 +3236,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) {
@@ -3335,10 +3319,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) {
@@ -3813,7 +3793,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;