From f1de8d3beb892049d08593cb358586c9323cbe55 Mon Sep 17 00:00:00 2001
From: Laine Stump <laine@laine.org>
Date: Fri, 19 Oct 2012 12:13:49 -0400
Subject: [PATCH] network: free/null newDef if network fails to start

https://bugzilla.redhat.com/show_bug.cgi?id=866364

pointed out a crash due to virNetworkObjAssignDef free'ing
network->newDef without NULLing it afterward. A fix for this is in
upstream commit b7e9202401ebaa039b8f05acdefda8c24081537a. While the
NULLing of newDef was a legitimate fix, newDef should have already
been empty (NULL) anyway (as indicated in the comment that was deleted
by that commit).

The reason that newDef had a non-NULL value (i.e. the root cause) was
that networkStartNetwork() had failed after populating
network->newDef, but then neglected to free/NULL newDef in the
cleanup.

(A bit of background here: network->newDef should contain the
persistent config of a network when a network is active (and of course
only when it is persisten), and NULL at all other times. There is also
a network->def which should contain the persistent definition of the
network when it is inactive, and the current live state at all other
times. The idea is that you can make changes to network->newDef which
will take effect the next time the network is restarted, but won't
mess with the current state of the network (virDomainObj has a similar
pair of virDomainDefs that behave in the same fashion). Personally I
think there should be a network->live and network->config, and the
location of the persistent config should *always* be in
network->config, but that's for a later cleanup).

Since I love things to be symmetric, I created a new function called
virNetworkObjUnsetDefTransient(), which reverses the effects of
virNetworkObjSetDefTransient(). I don't really like the name of the
new function, but then I also didn't really like the name of the old
one either (it's just named that way to match a similar function in
the domain conf code).
(cherry picked from commit 78fab2770b19097fe5e92ec339a9dd2d62d04bdb)
---
 src/conf/network_conf.c     | 14 ++++++++++++++
 src/conf/network_conf.h     |  1 +
 src/libvirt_private.syms    |  1 +
 src/network/bridge_driver.c | 12 ++++--------
 4 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ebc8061a2e..82251184cb 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -352,6 +352,20 @@ virNetworkObjSetDefTransient(virNetworkObjPtr network, bool live)
     return network->newDef ? 0 : -1;
 }
 
+/* virNetworkObjUnsetDefTransient:
+ *
+ * This *undoes* what virNetworkObjSetDefTransient did.
+ */
+void
+virNetworkObjUnsetDefTransient(virNetworkObjPtr network)
+{
+    if (network->def) {
+        virNetworkDefFree(network->def);
+        network->def = network->newDef;
+        network->newDef = NULL;
+    }
+}
+
 /*
  * virNetworkObjGetPersistentDef:
  * @network: network object pointer
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 55502fb2fc..4c0c8c1fbf 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -253,6 +253,7 @@ int virNetworkObjAssignDef(virNetworkObjPtr network,
                            const virNetworkDefPtr def,
                            bool live);
 int virNetworkObjSetDefTransient(virNetworkObjPtr network, bool live);
+void virNetworkObjUnsetDefTransient(virNetworkObjPtr network);
 virNetworkDefPtr virNetworkObjGetPersistentDef(virNetworkObjPtr network);
 int virNetworkObjReplacePersistentDef(virNetworkObjPtr network,
                                       virNetworkDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index be24d88f79..850b92dedd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -864,6 +864,7 @@ virNetworkObjLock;
 virNetworkObjReplacePersistentDef;
 virNetworkObjSetDefTransient;
 virNetworkObjUnlock;
+virNetworkObjUnsetDefTransient;
 virNetworkObjUpdate;
 virNetworkRemoveInactive;
 virNetworkSaveConfig;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e1846ee6d3..4b37a58041 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2327,8 +2327,10 @@ networkStartNetwork(struct network_driver *driver,
         break;
     }
 
-    if (ret < 0)
+    if (ret < 0) {
+        virNetworkObjUnsetDefTransient(network);
         return ret;
+    }
 
     /* Persist the live configuration now that anything autogenerated
      * is setup.
@@ -2388,13 +2390,7 @@ static int networkShutdownNetwork(struct network_driver *driver,
     }
 
     network->active = 0;
-
-    if (network->newDef) {
-        virNetworkDefFree(network->def);
-        network->def = network->newDef;
-        network->newDef = NULL;
-    }
-
+    virNetworkObjUnsetDefTransient(network);
     return ret;
 }