[libvirt] [PATCH 2/3] virsh: prevent destroying a in-used network for net-destroy

[Michal Privoznik]

On 02.02.2015 15:08, Lin Ma wrote:
> * add --system flag for net-dumpxml to show information about the
>   attached interfaces of the virtual network.
> 
> * call virNetworkGetXMLDesc in cmdNetworkDestroy to get the live state
>   info to check whether the virtual network is in use.
> 
> * add --force flag for net-destroy to forcibly destroy the virtual
>   network even if it's in use.
> 
> Signed-off-by: Lin Ma <lma at suse.com>
> ---
>  tools/virsh-network.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++----
>  tools/virsh.pod       |  8 +++++--
>  2 files changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/virsh-network.c b/tools/virsh-network.c
> index 5f8743c..17a5710 100644
> --- a/tools/virsh-network.c
> +++ b/tools/virsh-network.c
> @@ -253,6 +253,10 @@ static const vshCmdOptDef opts_network_destroy[] = {
>       .flags = VSH_OFLAG_REQ,
>       .help = N_("network name or uuid")
>      },
> +    {.name = "force",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("Forcefully stop a given network")
> +    },
>      {.name = NULL}
>  };
>  
> @@ -260,20 +264,55 @@ static bool
>  cmdNetworkDestroy(vshControl *ctl, const vshCmd *cmd)
>  {
>      virNetworkPtr network;
> -    bool ret = true;
> +    bool ret = false;
>      const char *name;
> +    char *br_xml = NULL;
> +    xmlDocPtr xml_doc = NULL;
> +    xmlXPathContextPtr ctxt = NULL;
>  
>      if (!(network = vshCommandOptNetwork(ctl, cmd, &name)))
> -        return false;
> +        goto cleanup;
> +
> +#ifdef WITH_NETCF

No. Even if we (the client) were built without netcf support, we may be
talking to a daemon built with the support and therefore we may want
--force.

> +    if (!vshCommandOptBool(cmd, "force")) {
> +        if (virNetworkIsActive(network) <= 0) {
> +            vshError(ctl, "%s", _("network is not active"));
> +            goto cleanup;
> +        }
> +        if (!(br_xml = virNetworkGetXMLDesc(network,
> +                                            VIR_NETWORK_XML_IFACE_ATTACHED)))
> +            goto cleanup;
> +
> +        if (!(xml_doc = virXMLParseStringCtxt(br_xml,
> +                                              _("(bridge interface "
> +                                                "definition)"), &ctxt))) {
> +            vshError(ctl, "%s", _("Failed to parse network configuration"));
> +            goto cleanup;
> +        }
> +
> +        if (virXPathNode("./bridge/interface[1]", ctxt) != NULL) {
> +            vshError(ctl, "%s", _("The network is in use by guests"));
> +            vshPrint(ctl, "%s", _("Use --force flag if you do want to do so"));
> +            goto cleanup;

I must say I'm not a big fan of this. It's a change in behaviour, while
previously users were able to destroy a running network (and cut off
domains), it won't be possible using the same command anymore.
On the other hand, preventing them from shooting in their own foot is
nice thing to do.

> +        }
> +    }
> +#endif
>  
>      if (virNetworkDestroy(network) == 0) {
>          vshPrint(ctl, _("Network %s destroyed\n"), name);
>      } else {
>          vshError(ctl, _("Failed to destroy network %s"), name);
> -        ret = false;
> +        goto cleanup;
>      }
>  
> -    virNetworkFree(network);
> +    ret = true;
> + cleanup:
> +    if(network)
> +        virNetworkFree(network);
> +    VIR_FREE(br_xml);
> +    xmlXPathFreeContext(ctxt);
> +    xmlFreeDoc(xml_doc);
> +
>      return ret;
>  }
>  
> @@ -300,6 +339,10 @@ static const vshCmdOptDef opts_network_dumpxml[] = {
>       .type = VSH_OT_BOOL,
>       .help = N_("network information of an inactive domain")
>      },
> +    {.name = "system",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("current live state information including attached interfaces")
> +    },
>      {.name = NULL}
>  };
>  
> @@ -311,6 +354,7 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
>      char *dump;
>      unsigned int flags = 0;
>      int inactive;
> +    bool system = false;
>  
>      if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
>          return false;
> @@ -319,6 +363,16 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
>      if (inactive)
>          flags |= VIR_NETWORK_XML_INACTIVE;
>  
> +    system = vshCommandOptBool(cmd, "system");
> +    if (system) {
> +        flags = VIR_NETWORK_XML_IFACE_ATTACHED;
> +        if (virNetworkIsActive(network) <= 0) {
> +            vshError(ctl, "%s", _("--system only works on active network"));
> +            virNetworkFree(network);
> +            return false;
> +        }
> +    }
> +
>      dump = virNetworkGetXMLDesc(network, flags);
>  
>      if (dump != NULL) {
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 804458e..3373ada 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2672,16 +2672,20 @@ to get a description of the XML network format used by libvirt.
>  Define a persistent virtual network from an XML I<file>, the network is just
>  defined but not instantiated (started).
>  
> -=item B<net-destroy> I<network>
> +=item B<net-destroy> I<network>  [I<--force>]
>  
>  Destroy (stop) a given transient or persistent virtual network
>  specified by its name or UUID. This takes effect immediately.
> +If I<--force> is specified, then forcefully destroy the virtual
> +network even if it's in use.
>  
> -=item B<net-dumpxml> I<network> [I<--inactive>]
> +=item B<net-dumpxml> I<network> [I<--inactive>] [I<--system>]
>  
>  Output the virtual network information as an XML dump to stdout.
>  If I<--inactive> is specified, then physical functions are not
>  expanded into their associated virtual functions.
> +If I<--system> is specified, then directly output the current
> +live state corresponding to this network from system.
>  
>  =item B<net-edit> I<network>
>  
> 

Documenting new command arguments, that's nice.

Michal