On Fri, Jul 18, 2008 at 12:27:03PM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Thu, Jul 17, 2008 at 01:20:59PM +0400, Evgeniy Sokolov wrote: ...
Index: virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.155 diff -u -p -r1.155 virsh.c --- virsh.c 29 May 2008 14:56:12 -0000 1.155 +++ virsh.c 17 Jul 2008 09:04:17 -0000 @@ -978,7 +978,8 @@ cmdUndefine(vshControl * ctl, vshCmd * c if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE;
- if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", &name))) + if (!(dom = vshCommandOptDomainBy(ctl, cmd, "domain", &name, + VSH_BYNAME|VSH_BYUUID)))
Before this change, we'd get a hint that undefining a running domain is not possible:
$ virsh -q -c test:///default undefine 1 virsh # libvir: Test error test: internal error Domain is still running error: Failed to undefine domain 1 virsh #
Now, the hint is gone, and the new diagnostics are misleading, since domain "1" certainly does exist:
$ ./virsh -q -c test:///default undefine 1 virsh # libvir: Test error : Domain not found error: failed to get domain '1' virsh #
oh, good catch !
With the patch below virsh tells what's wrong:
$ ./virsh -q -c test:///default undefine 1 error: a running domain like 1 cannot be undefined; to undefine, first shutdown then undefine using its name or UUID [Exit 1]
My first attempt called vshCommandOptDomainBy-with-BYID only upon failure of the with-VSH_BYNAME|VSH_BYUUID call, but then you'd still get this misleading diagnostic:
error: failed to get domain '1'
It also adds a test that essentially does this:
$ ./virsh -q -c test:///default undefine 1 error: a running domain like 1 cannot be undefined; to undefine, first shutdown then undefine using its name or UUID [Exit 1] $ ./virsh -q -c test:///default undefine test libvir: Test error test: internal error Domain is still running error: Failed to undefine domain test [Exit 1] $ ./virsh -q -c test:///default 'shutdown 1; undefine test' Domain 1 is being shutdown Domain test has been undefined
Good idea ! +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/