[libvirt] [PATCH 03/11] vsh: Add @silent to vshConnectionHook

[Martin Kletzander]

On Tue, Nov 07, 2017 at 01:22:51PM +0100, Michal Privoznik wrote:
>In near future we will want to not report error when connecting
>fails. In order to achieve that we have to pass a boolean that
>suppresses error messages.
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> tools/virsh-domain.c |  2 +-
> tools/virsh.c        | 67 ++++++++++++++++++++++++++++++++--------------------
> tools/virsh.h        |  5 +++-
> tools/virt-admin.c   | 49 +++++++++++++++++++++-----------------
> tools/vsh.c          |  2 +-
> tools/vsh.h          |  3 ++-
> 6 files changed, 76 insertions(+), 52 deletions(-)
>
>diff --git a/tools/virsh.c b/tools/virsh.c
>index d0c135016..7d6dc2620 100644
>--- a/tools/virsh.c
>+++ b/tools/virsh.c
>@@ -191,15 +191,19 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly)
>     if (interval > 0 &&
>         virConnectSetKeepAlive(c, interval, count) != 0) {
>         if (keepalive_forced) {
>-            vshError(ctl, "%s",
>-                     _("Cannot setup keepalive on connection "
>-                       "as requested, disconnecting"));
>+            if (!silent) {
>+                vshError(ctl, "%s",
>+                         _("Cannot setup keepalive on connection "
>+                           "as requested, disconnecting"));
>+            }
>             virConnectClose(c);
>             c = NULL;
>             goto cleanup;
>         }
>-        vshDebug(ctl, VSH_ERR_INFO, "%s",
>-                 _("Failed to setup keepalive on connection\n"));
>+        if (!silent) {
>+            vshDebug(ctl, VSH_ERR_INFO, "%s",
>+                     _("Failed to setup keepalive on connection\n"));
>+        }

I guess debug messages probably need to be filtered too.  Actually
allocation errors should be covered as well.  And you missed some.  It's
fine as it is, since no auto-completion is perfect, I use lot of them
extensively and I must say I don't care if it outputs something when
some error happens.  The nice thing about auto-completion is that if it
does not work at all or works differently (outputs something it
"shouldn't") nothing happens.  There is no data loss, broken migrations
or whatever.

Anyway looking through all the things that you are modifying here and to
those that could still be modified, I think this could be approached a
bit better.  As I said it's kind of fine if we keep it like this for
now, but it could be even better.  Consider this:

 - we do not modify what messages are reported
 - we leave the function that manages log output to decide
 - when the completer is called we set it up so that there is no log
   output

Guess what, we have first two things in place and for the third one you
can just call vshCloseLogFile(ctl) and you're golden ;)

If you want to make it even better, you can already do that before you
get to any completer.  For example if you specify '-q' multiple times on
the command line it could switch to super quiet mode, e.g.:

diff --git i/tools/virsh.c w/tools/virsh.c
index f830331f6bbe..6b7eafeba0be 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -782,6 +782,8 @@ virshParseArgv(vshControl *ctl, int argc, char **argv)
             vshOpenLogFile(ctl);
             break;
         case 'q':
+            if (ctl->quiet)
+                vshCloseLogFile(ctl);
             ctl->quiet = true;
             break;
         case 't':
--

And just call virsh from the bash completion with '-qq'

And the same for virt-admin of course.  And maybe document it :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171108/c054a6c8/attachment-0001.sig>