[PATCH] virsh: Fix regression with duplicated error messages

[Eric Blake <eblake@xxxxxxxxxx>]

Commit 4f4c3b13 (v3.3) fixed an issue where cleaning libvirt objects
lost error messages, by adding code to copy the libvirt error into
last_error prior to cleanup paths.  However, it caused a regression:
some errors are now printed twice, because libvirt still remembers in
its thread-local storage that an error was set.  For example:

$ virsh -c test:///default snapshot-delete test blah
error: Domain snapshot not found: no domain snapshot with matching name 'blah'
error: Domain snapshot not found: no domain snapshot with matching name 'blah'

Fix things by telling libvirt to discard any thread-local errors at
the same time virsh prints an error message (whether or not the libvirt
error is the same as what is stored in last_error).

Update the virsh-undefine testsuite (partially reverting portions of
commit b620bdee, by removing -q, to more easily pinpoint which commands
are causing which messages), now that there is only one error message
instead of two.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
 tests/virsh-undefine | 20 ++++++++++++++++----
 tools/vsh.c          |  1 +
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/tests/virsh-undefine b/tests/virsh-undefine
index 974b0c71f7..4a9f68dd39 100755
--- a/tests/virsh-undefine
+++ b/tests/virsh-undefine
@@ -30,34 +30,46 @@ fail=0
 # connection is opened to the test driver, it starts life with a new
 # persistent running domain named 'test' with a different uuid, so
 # testing this command requires batch mode use of virsh.
-$abs_top_builddir/tools/virsh -q -c test:///default \
+$abs_top_builddir/tools/virsh -c test:///default \
     'dominfo test; undefine test; dominfo test' > out1 2>&1
 test $? = 0 || fail=1
 sed '/^Persistent/n; /:/d' < out1 > out
 cat <<\EOF > exp || fail=1
 Persistent:     yes
+
+Domain test has been undefined
+
 Persistent:     no
+
 EOF
 compare exp out || fail=1

 # A similar diagnostic when specifying a domain ID
-$abs_top_builddir/tools/virsh -q -c test:///default \
+$abs_top_builddir/tools/virsh -c test:///default \
     'dominfo 1; undefine 1; dominfo 1' > out1 2>&1
 test $? = 0 || fail=1
 sed '/^Persistent/n; /:/d' < out1 > out
 cat <<\EOF > exp || fail=1
 Persistent:     yes
+
+Domain 1 has been undefined
+
 Persistent:     no
+
 EOF
 compare exp out || fail=1

 # Succeed, now: first shut down, then undefine, both via name.
-$abs_top_builddir/tools/virsh -q -c test:///default \
+$abs_top_builddir/tools/virsh -c test:///default \
     'shutdown test; undefine test; dominfo test' > out 2>&1
 test $? = 1 || fail=1
 cat <<\EOF > expout || fail=1
+Domain test is being shutdown
+
+Domain test has been undefined
+
 error: failed to get domain 'test'
-error: Domain not found
+
 EOF
 compare expout out || fail=1

diff --git a/tools/vsh.c b/tools/vsh.c
index 9ea3c4b96a..de887a9e76 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -276,6 +276,7 @@ vshResetLibvirtError(void)
 {
     virFreeError(last_error);
     last_error = NULL;
+    virResetLastError();
 }

 /*
-- 
2.17.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list