[virt-tools-list] [virt-viewer updated PATCH] Show errors generated by connection dialog

[Jonathon Jongsma]

On Thu, 2017-02-09 at 13:00 +0100, Christophe de Dinechin wrote:
> When running 'remote-viewer' without arguments,
> and selecting a non-supported protocol, e.g. ssh://foo,
> the generated error was silently swallowed by the retry loop.
> This was introduced in 06b2c382468876a19600374bd59475e66d488af8.
> ---
>  src/remote-viewer.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index 13c6e7c..e4b0909 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -1196,6 +1196,9 @@ cleanup:
>      type = NULL;
>  
>      if (!ret && priv->open_recent_dialog) {
> +        if (error != NULL) {
> +            virt_viewer_app_simple_message_dialog(app, _("Unable to
> connect: %s"), error->message);
> +        }
>          g_clear_error(&error);
>          goto retry_dialog;
>      }


Since other types of errors already result in error dialogs, I was
initially concerned that this was going to display 2 error dialogs for
for some error cases. But it doesn't. For my own sake (and maybe for
others), I'll just quickly describe why:

Let's take the case where you specify a valid protocol and a valid
hostname, but there is no spice server listening on the given port. In
this case, virt_viewer_app_initial_connect() actually returns TRUE as
soon as we start trying to connect, and no error is set. So
remote_viewer_start() exits cleanly and doesn't execute the 'goto
retry_dialog' statement. The connection continues asynchronously in the
background, and when the connection fails we will get a 'session-
disconnected' signal. The handler function for this signal displays an
error dialog and calls into virt_viewer_app_deactivate() which
eventually ends in the vfunc remote_viewer_deactivated() being called,
which then re-opens the connection dialog if necessary.

So the error-reporting and re-trying paths for these two types of
errors are completely different. It might be nice to try to unify this
a bit at some point. But at the moment, it seems that this patch solves
the problem.

Jonathon