From: Eric Blake <eblake@redhat.com>
Date: Thu, 11 Oct 2018 16:29:58 +0000 (-0500)
Subject: virsh: Fix regression with duplicated error messages
X-Git-Tag: v4.9.0-rc1~26
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=35966308b56842213e856a172ef1d495164fa5c5;p=thirdparty%2Flibvirt.git

virsh: Fix regression with duplicated error messages

Commit 4f4c3b13 (v3.3) fixed an issue where performing cleanup of
libvirt objects could sometimes lose error messages, by adding code
to copy the libvirt error into last_error prior to cleanup paths.
However, it caused a regression: on other paths, some errors are now
printed twice, if libvirt still remembers in its thread-local
storage that an error was set even after virsh cleared last_error.
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@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---

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();
 }
 
 /*