Re: [libvirt] [PATCH] repeat lookup by name in LookupByID

"Daniel P. Berrange" <berrange(a)redhat.com&gt; 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 #

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