4:48 a.m.
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.
---
src/conf/network_conf.c | 234 ++++++++++++++++++++++++++++++++++++++++++--
src/conf/network_conf.h | 16 ++-
src/libvirt_private.syms | 10 +-
src/network/bridge_driver.c | 14 ++-
src/test/test_driver.c | 9 +-
5 files changed, 262 insertions(+), 21 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 88e1492..a48eb9e 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 5dbf50d..0d37a8b 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 db08c09..33763fc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -829,6 +829,8 @@ virNetDevVPortTypeToString;
# network_conf.h
virNetworkAssignDef;
virNetworkConfigFile;
+virNetworkConfigChangeSetup;
+virNetworkDefCopy;
virNetworkDefFormat;
virNetworkDefFree;
virNetworkDefGetIpByIndex;
@@ -842,15 +844,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 4faad5d..8dbb050 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 e15833b..d635f3d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -576,7 +576,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;
}
@@ -945,8 +945,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;
}
@@ -3109,7 +3108,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;
@@ -3134,7 +3133,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;
--
1.7.11.4