[libvirt] [PATCH 1/2] virsh: don't reject undefine on active domain

[Daniel Veillard]

On Thu, Aug 11, 2011 at 08:39:22PM -0600, Eric Blake wrote:
> The public API documents that undefine may be used to transition a
> running persistent domain into a transient one.  Many drivers still
> do not support this usage, but virsh shouldn't be getting in the
> way of those that do support it.
> 
> * tools/virsh.c (cmdUndefine): Allow undefine on active domains;
> the drivers may still reject it, but it is a valid API usage.
> * tests/undefine (error): Fix the test to match.
> ---
>  tests/undefine |    9 +++++----
>  tools/virsh.c  |   15 +--------------
>  2 files changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/tests/undefine b/tests/undefine
> index f89a91e..5c39e27 100755
> --- a/tests/undefine
> +++ b/tests/undefine
> @@ -1,7 +1,7 @@
>  #!/bin/sh
>  # exercise virsh's "undefine" command
> 
> -# Copyright (C) 2008-2009 Red Hat, Inc.
> +# Copyright (C) 2008-2009, 2011 Red Hat, Inc.
> 
>  # This program is free software: you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -30,6 +30,7 @@ fi
>  fail=0
> 
>  # Attempt to undefine a running domain, by domain name.
> +# Although the API allows this, the test hypervisor does not.
>  $abs_top_builddir/tools/virsh -q -c test:///default undefine test > out 2>&1
>  test $? = 1 || fail=1
>  cat <<\EOF > exp || fail=1
> @@ -38,12 +39,12 @@ error: internal error Domain 'test' is still running
>  EOF
>  compare exp out || fail=1
> 
> -# A different diagnostic when specifying a domain ID
> +# A similar diagnostic when specifying a domain ID
>  $abs_top_builddir/tools/virsh -q -c test:///default undefine 1 > out 2>&1
>  test $? = 1 || fail=1
>  cat <<\EOF > exp || fail=1
> -error: a running domain like 1 cannot be undefined;
> -to undefine, first shutdown then undefine using its name or UUID
> +error: Failed to undefine domain 1
> +error: internal error Domain 'test' is still running
>  EOF
>  compare exp out || fail=1
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index b0d43ba..371e010 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -1431,7 +1431,6 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>      virDomainPtr dom;
>      bool ret = true;
>      const char *name = NULL;
> -    int id;
>      int flags = 0;
>      int managed_save = vshCommandOptBool(cmd, "managed-save");
>      int has_managed_save = 0;
> @@ -1449,19 +1448,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>      if (vshCommandOptString(cmd, "domain", &name) <= 0)
>          return false;
> 
> -    if (name && virStrToLong_i(name, NULL, 10, &id) == 0
> -        && id >= 0 && (dom = virDomainLookupByID(ctl->conn, id))) {
> -        vshError(ctl,
> -                 _("a running domain like %s cannot be undefined;\n"
> -                   "to undefine, first shutdown then undefine"
> -                   " using its name or UUID"),
> -                 name);
> -        virDomainFree(dom);
> -        return false;
> -    }
> -    if (!(dom = vshCommandOptDomainBy(ctl, cmd, &name,
> -                                      VSH_BYNAME|VSH_BYUUID)))
> -        return false;
> +    if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
> 
>      has_managed_save = virDomainHasManagedSaveImage(dom, 0);
>      if (has_managed_save < 0) {

 ACK, it also fix the lookup since we are sure to have a name,
maybe that should go in the commit log

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/