"Daniel P. Berrange" <berrange@xxxxxxxxxx> 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
>From d8a30f1f4da5db63b0c3cef2a67183a76c7dc989 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Fri, 18 Jul 2008 10:53:25 +0200
Subject: [PATCH] better diagnostic when failing to undefine a running domain via ID
* src/virsh.c (cmdUndefine): Tell user to shutdown and then use name or UUID.
* tests/undefine: New test. Exercise virsh's undefine command.
* tests/Makefile.am (test_scripts): Add undefine.
---
src/virsh.c | 14 +++++++++++++
tests/Makefile.am | 1 +
tests/undefine | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 70 insertions(+), 0 deletions(-)
create mode 100755 tests/undefine
diff --git a/src/virsh.c b/src/virsh.c
index f1296ec..620f103 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -974,10 +974,24 @@ cmdUndefine(vshControl * ctl, vshCmd * cmd)
virDomainPtr dom;
int ret = TRUE;
char *name;
+ int found;
+ int id;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
+ name = vshCommandOptString(cmd, "domain", &found);
+ if (!found)
+ return FALSE;
+
+ if (name && virStrToLong_i(name, NULL, 10, &id) == 0
+ && id >= 0 && (dom = virDomainLookupByID(ctl->conn, id))) {
+ vshError(ctl, FALSE, _("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, "domain", &name,
VSH_BYNAME|VSH_BYUUID)))
return FALSE;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2fc5a2f..5686678 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -51,6 +51,7 @@ test_scripts += \
int-overflow \
read-bufsiz \
read-non-seekable \
+ undefine \
vcpupin
endif
diff --git a/tests/undefine b/tests/undefine
new file mode 100755
index 0000000..3936e22
--- /dev/null
+++ b/tests/undefine
@@ -0,0 +1,55 @@
+#!/bin/sh
+# exercise virsh's "undefine" command
+
+# Copyright (C) 2008 Free Software Foundation, 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
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ virsh --version
+fi
+
+. $srcdir/test-lib.sh
+
+fail=0
+
+# Attempt to undefine a running domain, by domain name.
+virsh -q -c test:///default undefine test > out 2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
+libvir: Test error test: internal error Domain is still running
+error: Failed to undefine domain test
+EOF
+compare out exp || fail=1
+
+# A different diagnostic when specifying a domain ID
+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
+EOF
+compare out exp || fail=1
+
+# Succeed, now: first shut down, then undefine, both via name.
+virsh -q -c test:///default 'shutdown test; undefine test' > out 2>&1
+test $? = 0 || fail=1
+cat <<\EOF > exp || fail=1
+Domain test is being shutdown
+Domain test has been undefined
+EOF
+compare out exp || fail=1
+
+(exit $fail); exit $fail
--
1.5.6.3.440.ge3a8d
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list