aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorLaine Stump <laine@laine.org>2012-09-21 14:48:17 -0400
committerLaine Stump <laine@laine.org>2012-09-21 20:10:43 -0400
commitf59e25e012001911f565b1fb60d5353cd72d78e4 (patch)
treef33123378c42e032c88fe535c9927c03e26a8557 /src
parentnetwork: don't "refresh" iptables rules on rule-less networks (diff)
downloadlibvirt-f59e25e012001911f565b1fb60d5353cd72d78e4.tar.gz
libvirt-f59e25e012001911f565b1fb60d5353cd72d78e4.tar.bz2
libvirt-f59e25e012001911f565b1fb60d5353cd72d78e4.zip
network: make virNetworkObjUpdate error detection/recovery better
1) virNetworkObjUpdate should be an all or none operation, but in the case that we want to update both the live state and persistent config versions of the network, it was committing the update to the live state before starting to update the persistent config. If update of the persistent config failed, we would leave with things in an inconsistent state - the live state would be updated (even though an error was returned), but persistent config unchanged. This patch changed virNetworkObjUpdate to use a separate pointer for each copy of the virNetworkDef, and not commit either of them in the virNetworkObj until both live and config parts of the update have successfully completed. 2) The parsers for various pieces of the virNetworkDef have all sorts of subtle limitations on them that may not be known by the Update[section] function, making it possible for one of these functions to make a modification directly to the object that may not pass the scrutiny of a subsequent parse. But normally another parse wouldn't be done on the data until the *next* time the object was updated (which could leave the network definition in an unusable state). Rather than fighting the losing battle of trying to duplicate all the checks from the parsers into the update functions as well, the more foolproof solution to this is to simply do an extra virNetworkDefCopy() operation on the updated networkdef - virNetworkDefCopy() does a virNetworkFormat() followed by a virNetworkParseString(), so it will do all the checks we need. If this fails, then we don't commit the changed def.
Diffstat (limited to 'src')
-rw-r--r--src/conf/network_conf.c47
1 files changed, 34 insertions, 13 deletions
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 34487dd4c..17b964335 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2840,45 +2840,66 @@ virNetworkObjUpdate(virNetworkObjPtr network,
unsigned int flags) /* virNetworkUpdateFlags */
{
int ret = -1;
- virNetworkDefPtr def = NULL;
+ virNetworkDefPtr livedef = NULL, configdef = NULL;
/* normalize config data, and check for common invalid requests. */
if (virNetworkConfigChangeSetup(network, flags) < 0)
goto cleanup;
if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE) {
+ virNetworkDefPtr checkdef;
+
/* work on a copy of the def */
- if (!(def = virNetworkDefCopy(network->def, 0)))
+ if (!(livedef = virNetworkDefCopy(network->def, 0)))
goto cleanup;
- if (virNetworkDefUpdateSection(def, command, section,
+ if (virNetworkDefUpdateSection(livedef, command, section,
parentIndex, xml, flags) < 0) {
goto cleanup;
}
- /* successfully modified copy, now replace original */
- virNetworkDefFree(network->def);
- network->def = def;
- def = NULL;
+ /* run a final format/parse cycle to make sure we didn't
+ * add anything illegal to the def
+ */
+ if (!(checkdef = virNetworkDefCopy(livedef, 0)))
+ goto cleanup;
+ virNetworkDefFree(checkdef);
}
if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) {
+ virNetworkDefPtr checkdef;
+
/* work on a copy of the def */
- if (!(def = virNetworkDefCopy(virNetworkObjGetPersistentDef(network),
- VIR_NETWORK_XML_INACTIVE))) {
+ if (!(configdef = virNetworkDefCopy(virNetworkObjGetPersistentDef(network),
+ VIR_NETWORK_XML_INACTIVE))) {
goto cleanup;
}
- if (virNetworkDefUpdateSection(def, command, section,
+ if (virNetworkDefUpdateSection(configdef, command, section,
parentIndex, xml, flags) < 0) {
goto cleanup;
}
+ if (!(checkdef = virNetworkDefCopy(configdef,
+ VIR_NETWORK_XML_INACTIVE))) {
+ goto cleanup;
+ }
+ virNetworkDefFree(checkdef);
+ }
+
+ if (configdef) {
/* successfully modified copy, now replace original */
- if (virNetworkObjReplacePersistentDef(network, def) < 0)
+ if (virNetworkObjReplacePersistentDef(network, configdef) < 0)
goto cleanup;
- def = NULL;
+ configdef = NULL;
+ }
+ if (livedef) {
+ /* successfully modified copy, now replace original */
+ virNetworkDefFree(network->def);
+ network->def = livedef;
+ livedef = NULL;
}
ret = 0;
cleanup:
- virNetworkDefFree(def);
+ virNetworkDefFree(livedef);
+ virNetworkDefFree(configdef);
return ret;
}