network: utility functions for updating network config

These new functions are highly inspired by those in domain_conf.c (but
not identical), and are intended to make it simpler to update the
various combinations of live/persistent network configs.

The network driver wasn't previously as careful about the separation
between the live "status" in network->def and the persistent "config"
in network->newDef (or sometimes in network->def). This series
attempts to remedy some of that, but probably doesn't go all the way
(enough to get these functions working and enable continued work on
virNetworkUpdate though).

bridge_driver.c and test_driver.c were updated in a few places to take
advantage of the new functions and/or account for changes in argument
lists.

5 files changed, 262 insertions, 21 deletions

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 88e1492f5..a48eb9eec 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -228,20 +228,74 @@ void virNetworkObjListFree(virNetworkObjListPtr nets)
     nets->count = 0;
 }
 
-virNetworkObjPtr virNetworkAssignDef(virNetworkObjListPtr nets,
-                                     const virNetworkDefPtr def)
+/*
+ * virNetworkObjAssignDef:
+ * @network: the network object to update
+ * @def: the new NetworkDef (will be consumed by this function iff successful)
+ * @live: is this new def the "live" version, or the "persistent" version
+ *
+ * Replace the appropriate copy of the given network's NetworkDef
+ * with def. Use "live" and current state of the network to determine
+ * which to replace.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+int
+virNetworkObjAssignDef(virNetworkObjPtr network,
+                       const virNetworkDefPtr def,
+                       bool live)
 {
-    virNetworkObjPtr network;
-
-    if ((network = virNetworkFindByName(nets, def->name))) {
-        if (!virNetworkObjIsActive(network)) {
+    if (virNetworkObjIsActive(network)) {
+        if (live) {
             virNetworkDefFree(network->def);
             network->def = def;
-        } else {
+        } else if (network->persistent) {
+            /* save current configuration to be restored on network shutdown */
             virNetworkDefFree(network->newDef);
             network->newDef = def;
+        } else {
+            virReportError(VIR_ERR_OPERATION_INVALID,
+                           _("cannot save persistent config of transient "
+                             "network '%s'"), network->def->name);
+            return -1;
         }
+    } else if (!live) {
+        virNetworkDefFree(network->newDef); /* should be unnecessary */
+        virNetworkDefFree(network->def);
+        network->def = def;
+    } else {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("cannot save live config of inactive "
+                         "network '%s'"), network->def->name);
+        return -1;
+    }
+    return 0;
+}
+
+/*
+ * virNetworkAssignDef:
+ * @nets: list of all networks
+ * @def: the new NetworkDef (will be consumed by this function iff successful)
+ * @live: is this new def the "live" version, or the "persistent" version
+ *
+ * Either replace the appropriate copy of the NetworkDef with name
+ * matching def->name or, if not found, create a new NetworkObj with
+ * def. For an existing network, use "live" and current state of the
+ * network to determine which to replace.
+ *
+ * Returns -1 on failure, 0 on success.
+ */
+virNetworkObjPtr
+virNetworkAssignDef(virNetworkObjListPtr nets,
+                    const virNetworkDefPtr def,
+                    bool live)
+{
+    virNetworkObjPtr network;
 
+    if ((network = virNetworkFindByName(nets, def->name))) {
+        if (virNetworkObjAssignDef(network, def, live) < 0) {
+            return NULL;
+        }
         return network;
     }
 
@@ -270,6 +324,150 @@ virNetworkObjPtr virNetworkAssignDef(virNetworkObjListPtr nets,
 
 }
 
+/*
+ * virNetworkObjSetDefTransient:
+ * @network: network object pointer
+ * @live: if true, run this operation even for an inactive network.
+ *   this allows freely updated network->def with runtime defaults
+ *   before starting the network, which will be discarded on network
+ *   shutdown. Any cleanup paths need to be sure to handle newDef if
+ *   the network is never started.
+ *
+ * Mark the active network config as transient. Ensures live-only update
+ * operations do not persist past network destroy.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virNetworkObjSetDefTransient(virNetworkObjPtr network, bool live)
+{
+    if (!virNetworkObjIsActive(network) && !live)
+        return 0;
+
+    if (!network->persistent || network->newDef)
+        return 0;
+
+    network->newDef = virNetworkDefCopy(network->def, VIR_NETWORK_XML_INACTIVE);
+    return network->newDef ? 0 : -1;
+}
+
+/*
+ * virNetworkObjGetPersistentDef:
+ * @network: network object pointer
+ *
+ * Return the persistent network configuration. If network is transient,
+ * return the running config.
+ *
+ * Returns NULL on error, virNetworkDefPtr on success.
+ */
+virNetworkDefPtr
+virNetworkObjGetPersistentDef(virNetworkObjPtr network)
+{
+    if (network->newDef)
+        return network->newDef;
+    else
+        return network->def;
+}
+
+/*
+ * virNetworkObjReplacePersistentDef:
+ * @network: network object pointer
+ * @def: new virNetworkDef to replace current persistent config
+ *
+ * Replace the "persistent" network configuration with the given new
+ * virNetworkDef. This pays attention to whether or not the network
+ * is active.
+ *
+ * Returns -1 on error, 0 on success
+ */
+int
+virNetworkObjReplacePersistentDef(virNetworkObjPtr network,
+                                  virNetworkDefPtr def)
+{
+    if (virNetworkObjIsActive(network)) {
+        virNetworkDefFree(network->newDef);
+        network->newDef = def;
+    } else {
+        virNetworkDefFree(network->def);
+        network->def = def;
+    }
+    return 0;
+}
+
+/*
+ * virNetworkDefCopy:
+ * @def: NetworkDef to copy
+ * @flags: VIR_NETWORK_XML_INACTIVE if appropriate
+ *
+ * make a deep copy of the given NetworkDef
+ *
+ * Returns a new NetworkDef on success, or NULL on failure.
+ */
+virNetworkDefPtr
+virNetworkDefCopy(virNetworkDefPtr def, unsigned int flags)
+{
+    char *xml = NULL;
+    virNetworkDefPtr newDef = NULL;
+
+    if (!def) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       "%s", _("NULL NetworkDef"));
+        return NULL;
+    }
+
+    /* deep copy with a format/parse cycle */
+    if (!(xml = virNetworkDefFormat(def, flags)))
+        goto cleanup;
+    newDef = virNetworkDefParseString(xml);
+cleanup:
+    VIR_FREE(xml);
+    return newDef;
+}
+
+/*
+ * virNetworkConfigChangeSetup:
+ *
+ * 1) checks whether network state is consistent with the requested
+ *    type of modification.
+ *
+ * 3) make sure there are separate "def" and "newDef" copies of
+ *    networkDef if appropriate.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int
+virNetworkConfigChangeSetup(virNetworkObjPtr network, unsigned int flags)
+{
+    bool isActive;
+    int ret = -1;
+
+    isActive = virNetworkObjIsActive(network);
+
+    if (!isActive && (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE)) {
+        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                       _("network is not running"));
+        goto cleanup;
+    }
+
+    if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) {
+        if (!network->persistent) {
+            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                           _("cannot change persistent config of a "
+                             "transient network"));
+            goto cleanup;
+        }
+        /* this should already have been done by the driver, but do it
+         * anyway just in case.
+         */
+        if (isActive && (virNetworkObjSetDefTransient(network, false) < 0))
+            goto cleanup;
+    }
+
+    ret = 0;
+cleanup:
+    return ret;
+}
+
 void virNetworkRemoveInactive(virNetworkObjListPtr nets,
                               const virNetworkObjPtr net)
 {
@@ -1791,7 +1989,7 @@ int virNetworkSaveConfig(const char *configDir,
     int ret = -1;
     char *xml;
 
-    if (!(xml = virNetworkDefFormat(def, 0)))
+    if (!(xml = virNetworkDefFormat(def, VIR_NETWORK_XML_INACTIVE)))
         goto cleanup;
 
     if (virNetworkSaveXML(configDir, def, xml))
@@ -1804,6 +2002,24 @@ cleanup:
 }
 
 
+int virNetworkSaveStatus(const char *statusDir,
+                         virNetworkObjPtr network)
+{
+    int ret = -1;
+    char *xml;
+
+    if (!(xml = virNetworkDefFormat(network->def, 0)))
+        goto cleanup;
+
+    if (virNetworkSaveXML(statusDir, network->def, xml))
+        goto cleanup;
+
+    ret = 0;
+cleanup:
+    VIR_FREE(xml);
+    return ret;
+}
+
 virNetworkObjPtr virNetworkLoadConfig(virNetworkObjListPtr nets,
                                       const char *configDir,
                                       const char *autostartDir,
@@ -1844,7 +2060,7 @@ virNetworkObjPtr virNetworkLoadConfig(virNetworkObjListPtr nets,
             goto error;
     }
 
-    if (!(net = virNetworkAssignDef(nets, def)))
+    if (!(net = virNetworkAssignDef(nets, def, false)))
         goto error;
 
     net->autostart = autostart;
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 5dbf50d05..0d37a8b6b 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -247,7 +247,18 @@ void virNetworkObjFree(virNetworkObjPtr net);
 void virNetworkObjListFree(virNetworkObjListPtr vms);
 
 virNetworkObjPtr virNetworkAssignDef(virNetworkObjListPtr nets,
-                                     const virNetworkDefPtr def);
+                                     const virNetworkDefPtr def,
+                                     bool live);
+int virNetworkObjAssignDef(virNetworkObjPtr network,
+                           const virNetworkDefPtr def,
+                           bool live);
+int virNetworkObjSetDefTransient(virNetworkObjPtr network, bool live);
+virNetworkDefPtr virNetworkObjGetPersistentDef(virNetworkObjPtr network);
+int virNetworkObjReplacePersistentDef(virNetworkObjPtr network,
+                                      virNetworkDefPtr def);
+virNetworkDefPtr virNetworkDefCopy(virNetworkDefPtr def, unsigned int flags);
+int virNetworkConfigChangeSetup(virNetworkObjPtr dom, unsigned int flags);
+
 void virNetworkRemoveInactive(virNetworkObjListPtr nets,
                               const virNetworkObjPtr net);
 
@@ -283,6 +294,9 @@ int virNetworkSaveXML(const char *configDir,
 int virNetworkSaveConfig(const char *configDir,
                          virNetworkDefPtr def);
 
+int virNetworkSaveStatus(const char *statusDir,
+                         virNetworkObjPtr net) ATTRIBUTE_RETURN_CHECK;
+
 virNetworkObjPtr virNetworkLoadConfig(virNetworkObjListPtr nets,
                                       const char *configDir,
                                       const char *autostartDir,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e8f3fa5b4..39e06e47c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -839,6 +839,8 @@ virNetDevVPortTypeToString;
 # network_conf.h
 virNetworkAssignDef;
 virNetworkConfigFile;
+virNetworkConfigChangeSetup;
+virNetworkDefCopy;
 virNetworkDefFormat;
 virNetworkDefFree;
 virNetworkDefGetIpByIndex;
@@ -852,15 +854,21 @@ virNetworkIpDefNetmask;
 virNetworkIpDefPrefix;
 virNetworkList;
 virNetworkLoadAllConfigs;
+virNetworkObjAssignDef;
+virNetworkObjFree;
+virNetworkObjGetPersistentDef;
 virNetworkObjIsDuplicate;
 virNetworkObjListFree;
 virNetworkObjLock;
+virNetworkObjReplacePersistentDef;
+virNetworkObjSetDefTransient;
 virNetworkObjUnlock;
-virNetworkObjFree;
 virNetworkRemoveInactive;
 virNetworkSaveConfig;
+virNetworkSaveStatus;
 virNetworkSetBridgeMacAddr;
 virNetworkSetBridgeName;
+virNetworkSetDefTransient;
 virPortGroupFindByName;
 
 
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 4faad5d87..8dbb05007 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2023,6 +2023,9 @@ networkStartNetwork(struct network_driver *driver,
         return -1;
     }
 
+    if (virNetworkObjSetDefTransient(network, true) < 0)
+        return -1;
+
     switch (network->def->forwardType) {
 
     case VIR_NETWORK_FORWARD_NONE:
@@ -2046,7 +2049,7 @@ networkStartNetwork(struct network_driver *driver,
     /* Persist the live configuration now that anything autogenerated
      * is setup.
      */
-    if ((ret = virNetworkSaveConfig(NETWORK_STATE_DIR, network->def)) < 0) {
+    if ((ret = virNetworkSaveStatus(NETWORK_STATE_DIR, network)) < 0) {
         goto error;
     }
 
@@ -2389,8 +2392,10 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) {
     if (networkValidate(def) < 0)
        goto cleanup;
 
-    if (!(network = virNetworkAssignDef(&driver->networks,
-                                        def)))
+    /* NB: "live" is false because this transient network hasn't yet
+     * been started
+     */
+    if (!(network = virNetworkAssignDef(&driver->networks, def, false)))
         goto cleanup;
     def = NULL;
 
@@ -2465,8 +2470,7 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
     if (networkValidate(def) < 0)
        goto cleanup;
 
-    if (!(network = virNetworkAssignDef(&driver->networks,
-                                        def)))
+    if (!(network = virNetworkAssignDef(&driver->networks, def, false)))
         goto cleanup;
     freeDef = false;
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index fbd8ed0ee..1bd0d6151 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -579,7 +579,7 @@ static int testOpenDefault(virConnectPtr conn) {
 
     if (!(netdef = virNetworkDefParseString(defaultNetworkXML)))
         goto error;
-    if (!(netobj = virNetworkAssignDef(&privconn->networks, netdef))) {
+    if (!(netobj = virNetworkAssignDef(&privconn->networks, netdef, false))) {
         virNetworkDefFree(netdef);
         goto error;
     }
@@ -948,8 +948,7 @@ static int testOpenFromFile(virConnectPtr conn,
             if ((def = virNetworkDefParseNode(xml, networks[i])) == NULL)
                 goto error;
         }
-        if (!(net = virNetworkAssignDef(&privconn->networks,
-                                        def))) {
+        if (!(net = virNetworkAssignDef(&privconn->networks, def, false))) {
             virNetworkDefFree(def);
             goto error;
         }
@@ -3112,7 +3111,7 @@ static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) {
     if ((def = virNetworkDefParseString(xml)) == NULL)
         goto cleanup;
 
-    if ((net = virNetworkAssignDef(&privconn->networks, def)) == NULL)
+    if (!(net = virNetworkAssignDef(&privconn->networks, def, false)))
         goto cleanup;
     def = NULL;
     net->active = 1;
@@ -3137,7 +3136,7 @@ static virNetworkPtr testNetworkDefine(virConnectPtr conn, const char *xml) {
     if ((def = virNetworkDefParseString(xml)) == NULL)
         goto cleanup;
 
-    if ((net = virNetworkAssignDef(&privconn->networks, def)) == NULL)
+    if (!(net = virNetworkAssignDef(&privconn->networks, def, false)))
         goto cleanup;
     def = NULL;
     net->persistent = 1;