[libvirt] [PATCH 6/6] net: Add support for changing persistent networks to transient

[Laine Stump]

On 10/25/2012 11:18 AM, Peter Krempa wrote:
> Until now, the network undefine API was able to undefine only inactive
> networks. The restriction doesn't make sense any more so this patch
> implements changing networks to transient.
> ---
>  src/network/bridge_driver.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index e90444d..95aaea9 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2856,6 +2856,7 @@ networkUndefine(virNetworkPtr net) {
>      struct network_driver *driver = net->conn->networkPrivateData;
>      virNetworkObjPtr network;
>      int ret = -1;
> +    bool active = false;
>
>      networkDriverLock(driver);
>
> @@ -2866,24 +2867,25 @@ networkUndefine(virNetworkPtr net) {
>          goto cleanup;
>      }
>
> -    if (virNetworkObjIsActive(network)) {
> -        virReportError(VIR_ERR_OPERATION_INVALID,
> -                       "%s", _("network is still active"));
> -        goto cleanup;
> -    }
> +    if (virNetworkObjIsActive(network))
> +        active = true;
>
>      if (virNetworkDeleteConfig(driver->networkConfigDir,
>                                 driver->networkAutostartDir,
>                                 network) < 0)
>          goto cleanup;
>
> +    network->persistent = 0;
> +
>      VIR_INFO("Undefining network '%s'", network->def->name);
> -    if (networkRemoveInactive(driver, network) < 0) {
> +    if (!active) {
> +        if (networkRemoveInactive(driver, network) < 0) {
> +            network = NULL;
> +            goto cleanup;
> +        }
>          network = NULL;
> -        goto cleanup;
>      }

If the network *is* active, you also need to free network->newDef and
NULL it out. newDef should only be non-NULL for persistent networks

(I *really* dislike the whole "def, newDef" way of doing things. I think
there should be a "stateDef" and "configDef" instead, and they should be
moved back and forth all the time - that just makes life confusing;
configDef should always contain the persistent config (if it's a
persistent network, or 0 otherwise) and stateDef should always contain
the live state of the network (or 0 if its an inactive persistent network).

>
> -    network = NULL;
>      ret = 0;
>
>  cleanup:

ACK once you clear out newDef (that other change is for later :-)