[virt-tools-list] [virt-viewer][PATCH 1/5 v2] virt-viewer: Add a GError arg to extract_connect_info()

[Jonathon Jongsma]

On Thu, 2015-03-26 at 16:35 +0100, Fabiano Fidêncio wrote:
> This is part of a small re-factoring that will have all connection
> errors, when we won't be able to connect regardless of what changes on
> the remote host, being treated by virt_viewer_app_initial_connect(),
> avoiding weird behaviors as we have nowadays (like more than one error
> dialog being shown or having the virt-viewer waiting forever for a guest
> that will never show up).
> 
> Related: rhbz#1085216
> ---
>  src/virt-viewer.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/src/virt-viewer.c b/src/virt-viewer.c
> index 994ee28..8858c16 100644
> --- a/src/virt-viewer.c
> +++ b/src/virt-viewer.c
> @@ -387,7 +387,8 @@ virt_viewer_is_reachable(const gchar *host,
>  
>  static gboolean
>  virt_viewer_extract_connect_info(VirtViewer *self,
> -                                 virDomainPtr dom)
> +                                 virDomainPtr dom,
> +                                 GError **error)
>  {
>      char *type = NULL;
>      char *xpath = NULL;
> @@ -405,12 +406,18 @@ virt_viewer_extract_connect_info(VirtViewer *self,
>      gint port = 0;
>      gchar *uri = NULL;
>      gboolean direct = virt_viewer_app_get_direct(app);
> +    GError *err = NULL;
>  
>      virt_viewer_app_free_connect_info(app);
>  
>      if ((type = virt_viewer_extract_xpath_string(xmldesc, "string(/domain/devices/graphics/@type)")) == NULL) {
> +        gchar *msg = g_strdup_printf(_("Cannot determine the graphic type for the guest %s"), priv->domkey);
> +        g_set_error_literal(&err, VIRT_VIEWER_ERROR, VIRT_VIEWER_ERROR_FAILED, msg);
> +        g_free(msg);
> +

I think using g_set_error() here (and several places below) will do the
same thing and is a lot simpler.

>          virt_viewer_app_simple_message_dialog(app, _("Cannot determine the graphic type for the guest %s"),
>                                                priv->domkey);
> +
>          goto cleanup;
>      }
>  
> @@ -446,8 +453,13 @@ virt_viewer_extract_connect_info(VirtViewer *self,
>  
>      uri = virConnectGetURI(priv->conn);
>      if (virt_viewer_util_extract_host(uri, NULL, &host, &transport, &user, &port) < 0) {
> +        gchar *msg = g_strdup_printf(_("Cannot determine the host for the guest %s"), priv->domkey);
> +        g_set_error_literal(&err, VIRT_VIEWER_ERROR, VIRT_VIEWER_ERROR_FAILED, msg);
> +        g_free(msg);
> +
>          virt_viewer_app_simple_message_dialog(app, _("Cannot determine the host for the guest %s"),
>                                                priv->domkey);
> +
>          goto cleanup;
>      }
>  
> @@ -472,10 +484,16 @@ virt_viewer_extract_connect_info(VirtViewer *self,
>      }
>  
>      if (!virt_viewer_is_reachable(ghost, transport, host, direct)) {
> +        gchar *msg = g_strdup_printf(_("Guest '%s' is not reachable"), priv->domkey);
> +        g_set_error_literal(&err, VIRT_VIEWER_ERROR, VIRT_VIEWER_ERROR_FAILED, msg);
> +        g_free(msg);
> +
>          g_debug("graphics listen '%s' is not reachable from this machine",
>                  ghost ? ghost : "");
> +
>          virt_viewer_app_simple_message_dialog(app, _("Guest '%s' is not reachable"),
>                                                priv->domkey);
> +
>          goto cleanup;
>      }
>  
> @@ -484,6 +502,9 @@ virt_viewer_extract_connect_info(VirtViewer *self,
>      retval = TRUE;
>  
>   cleanup:
> +    if (err)
> +        g_propagate_error(error, err);
> +
>      g_free(gport);
>      g_free(gtlsport);
>      g_free(ghost);
> @@ -515,7 +536,7 @@ virt_viewer_update_display(VirtViewer *self, virDomainPtr dom)
>      g_object_set(app, "guest-name", virDomainGetName(dom), NULL);
>  
>      if (!virt_viewer_app_has_session(app)) {
> -        if (!virt_viewer_extract_connect_info(self, dom))
> +        if (!virt_viewer_extract_connect_info(self, dom, NULL))
>              return FALSE;
>      }
>  

ACK other than the comment above.