Re: [libvirt] [PATCH 2/4] 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.

--- src/conf/network_conf.c | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 34487dd..17b9643 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;

if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { + virNetworkDefPtr checkdef;