From f36309d6885b4dc830ce6338b3c9613d9b8e7746 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Fri, 14 Sep 2012 11:35:35 -0400 Subject: [PATCH] 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. --- src/conf/network_conf.c | 238 ++++++++++++++++++++++++++++++++++-- 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, 264 insertions(+), 23 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 88e1492f57..a48eb9eec9 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) +{ + if (virNetworkObjIsActive(network)) { + if (live) { + virNetworkDefFree(network->def); + network->def = def; + } 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 (!virNetworkObjIsActive(network)) { - virNetworkDefFree(network->def); - network->def = def; - } else { - virNetworkDefFree(network->newDef); - network->newDef = def; + 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 5dbf50d052..0d37a8b6b0 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 e8f3fa5b49..39e06e47ce 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 4faad5d87e..8dbb050073 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 fbd8ed0eec..1bd0d61515 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;