[virt-tools-list] [PATCH virt-viewer] Spice: listen for new 'SpiceSession::disconnected' signal

[Marc-André Lureau]

Hi

On Tue, Sep 18, 2018 at 8:25 PM Jonathon Jongsma <jjongsma at redhat.com> wrote:
>
> Previously we were emitting the VirtViewerSession::session-disconnected
> when we got the Spice::session::channel-destroy signal for the last
> channel. However, since the channels are still valid at this point, and
> because VirtViewerApp quits the application in response to the
> session-disconnected signal, that means that the channels were never
> being properly freed. This was particularly problematic for the usbredir
> channel, which must disconnect any connected USB devices as part of its
> destruction. By using the new SpiceSession::disconnected signal instead,
> we can ensure that all channels have been disconnected and properly
> destroyed before quitting the application.
> ---
>
> NOTE: this is an older patch fell through the cracks. I added the
> SpiceSession::disconnected signal a while ago and was waiting for an
> official spice-gtk release before adding this patch to virt-viewer. The
> new spice-gtk release has been out for a while, but this patch was
> forgotten until now.
>
>  src/virt-viewer-session-spice.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)

This patch introduces a regression, virt-viewer now hangs when the
initial connection fails.
Reverting the patch works again

Jonathon, can you take a look?

thanks

>
> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> index fdc7004..cb06af2 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -50,7 +50,7 @@ struct _VirtViewerSessionSpicePrivate {
>      guint pass_try;
>      gboolean did_auto_conf;
>      VirtViewerFileTransferDialog *file_transfer_dialog;
> -
> +    GError *disconnect_error;
>  };
>
>  #define VIRT_VIEWER_SESSION_SPICE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o), VIRT_VIEWER_TYPE_SESSION_SPICE, VirtViewerSessionSpicePrivate))
> @@ -75,6 +75,8 @@ static void virt_viewer_session_spice_channel_new(SpiceSession *s,
>  static void virt_viewer_session_spice_channel_destroy(SpiceSession *s,
>                                                        SpiceChannel *channel,
>                                                        VirtViewerSession *session);
> +static void virt_viewer_session_spice_session_disconnected(SpiceSession *s,
> +                                                           VirtViewerSessionSpice *session);
>  static void virt_viewer_session_spice_smartcard_insert(VirtViewerSession *session);
>  static void virt_viewer_session_spice_smartcard_remove(VirtViewerSession *session);
>  static gboolean virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self);
> @@ -152,6 +154,7 @@ virt_viewer_session_spice_dispose(GObject *obj)
>          gtk_widget_destroy(GTK_WIDGET(spice->priv->file_transfer_dialog));
>          spice->priv->file_transfer_dialog = NULL;
>      }
> +    g_clear_error(&spice->priv->disconnect_error);
>
>      G_OBJECT_CLASS(virt_viewer_session_spice_parent_class)->dispose(obj);
>  }
> @@ -398,6 +401,8 @@ create_spice_session(VirtViewerSessionSpice *self)
>                                        G_CALLBACK(virt_viewer_session_spice_channel_new), self, 0);
>      virt_viewer_signal_connect_object(self->priv->session, "channel-destroy",
>                                        G_CALLBACK(virt_viewer_session_spice_channel_destroy), self, 0);
> +    virt_viewer_signal_connect_object(self->priv->session, "disconnected",
> +                                      G_CALLBACK(virt_viewer_session_spice_session_disconnected), self, 0);
>
>      usb_manager = spice_usb_device_manager_get(self->priv->session, NULL);
>      if (usb_manager) {
> @@ -1091,6 +1096,13 @@ virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self)
>      return TRUE;
>  }
>
> +static void
> +virt_viewer_session_spice_session_disconnected(G_GNUC_UNUSED SpiceSession *s,
> +                                               VirtViewerSessionSpice *self)
> +{
> +    g_signal_emit_by_name(self, "session-disconnected", self->priv->disconnect_error);
> +}
> +
>  static void
>  virt_viewer_session_spice_channel_destroy(G_GNUC_UNUSED SpiceSession *s,
>                                            SpiceChannel *channel,
> @@ -1129,10 +1141,12 @@ virt_viewer_session_spice_channel_destroy(G_GNUC_UNUSED SpiceSession *s,
>          if (self->priv->usbredir_channel_count == 0)
>              virt_viewer_session_set_has_usbredir(session, FALSE);
>      }
> -
> -    self->priv->channel_count--;
> -    if (self->priv->channel_count == 0)
> -        g_signal_emit_by_name(self, "session-disconnected", error ? error->message : NULL);
> +    if (error) {
> +        g_warning("Channel error: %s", error->message);
> +        if (self->priv->disconnect_error == NULL) {
> +            self->priv->disconnect_error = g_error_copy(error);
> +        }
> +    }
>  }
>
>  VirtViewerSession *
> --
> 2.14.4
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list



-- 
Marc-André Lureau