[libvirt] [PATCH] network: Don't remove transient network if creating of config file fails

On the off-chance that creation of persistent configuration file would fail when defining a network that is already started as transient, the code would remove the transient data structure and thus the network. This patch changes the code so that in such case, the network is again marked as transient and left behind. --- src/network/bridge_driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 27dd230..64c71af 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3160,8 +3160,13 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { freeDef = false; if (virNetworkSaveConfig(driver->networkConfigDir, def) < 0) { - virNetworkRemoveInactive(&driver->networks, network); - network = NULL; + if (!virNetworkObjIsActive(network)) { + virNetworkRemoveInactive(&driver->networks, network); + network = NULL; + goto cleanup; + } + network->persistent = 0; + virNetworkDefFree(network->newDef); goto cleanup; } -- 1.8.2.1

On 04/22/13 11:32, Peter Krempa wrote:
On the off-chance that creation of persistent configuration file would fail when defining a network that is already started as transient, the code would remove the transient data structure and thus the network.
This patch changes the code so that in such case, the network is again marked as transient and left behind. --- src/network/bridge_driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Could somebody please have a look on this patch? Thanks. Peter

On 04/22/2013 05:32 AM, Peter Krempa wrote:
On the off-chance that creation of persistent configuration file would fail when defining a network that is already started as transient, the code would remove the transient data structure and thus the network.
This patch changes the code so that in such case, the network is again marked as transient and left behind. --- src/network/bridge_driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 27dd230..64c71af 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3160,8 +3160,13 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { freeDef = false;
if (virNetworkSaveConfig(driver->networkConfigDir, def) < 0) { - virNetworkRemoveInactive(&driver->networks, network); - network = NULL; + if (!virNetworkObjIsActive(network)) { + virNetworkRemoveInactive(&driver->networks, network); + network = NULL; + goto cleanup; + } + network->persistent = 0; + virNetworkDefFree(network->newDef);
You also need to set network->newDef = NULL here. (It took some perusal of the code to figure out that this was the correct way to "reset" to the pre-networkDefine() state. The reason it works is that up above we've called virNetworkObjAssignDef(network, def, false); in that case, virNetworkObjAssignDef() just frees any existing network->newDef and sets network->newDef = def. ACK with that nit fixed. This should go in before 1.0.5.

On 04/29/13 18:07, Laine Stump wrote:
On 04/22/2013 05:32 AM, Peter Krempa wrote:
On the off-chance that creation of persistent configuration file would fail when defining a network that is already started as transient, the code would remove the transient data structure and thus the network.
This patch changes the code so that in such case, the network is again marked as transient and left behind. --- src/network/bridge_driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 27dd230..64c71af 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3160,8 +3160,13 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { freeDef = false;
if (virNetworkSaveConfig(driver->networkConfigDir, def) < 0) { - virNetworkRemoveInactive(&driver->networks, network); - network = NULL; + if (!virNetworkObjIsActive(network)) { + virNetworkRemoveInactive(&driver->networks, network); + network = NULL; + goto cleanup; + } + network->persistent = 0; + virNetworkDefFree(network->newDef);
You also need to set network->newDef = NULL here.
(It took some perusal of the code to figure out that this was the correct way to "reset" to the pre-networkDefine() state. The reason it works is that up above we've called virNetworkObjAssignDef(network, def, false); in that case, virNetworkObjAssignDef() just frees any existing network->newDef and sets network->newDef = def.
ACK with that nit fixed. This should go in before 1.0.5.
Thanks, I added clearing of the pointer and pushed. Peter
participants (2)
-
Laine Stump
-
Peter Krempa