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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list