Hi
On Tue, Sep 18, 2018 at 8:25 PM Jonathon Jongsma <jjongsma@xxxxxxxxxx> 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/virt-tools-list
--
Marc-André Lureau
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
