Hi On Fri, Jan 4, 2019 at 11:08 PM Marc-André Lureau <marcandre.lureau@xxxxxxxxx> wrote: > > 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? To not loose track of this regression, I opened https://bugzilla.redhat.com/show_bug.cgi?id=1666869 Daniel, Bugzilla is not that great to track upstream issues. Would pagure issues be better? > > 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 -- Marc-André Lureau _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
