On Thu, 2019-01-17 at 00:06 +0400, Marc-André Lureau wrote: > 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? Sorry, I forgot to reply to this email. I've got some time now so I've started looking into it. Jonathon > > > > > 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, > > > SpiceChann > > > el *channel, > > > VirtViewer > > > Session *session); > > > +static void > > > virt_viewer_session_spice_session_disconnected(SpiceSession *s, > > > + VirtV > > > iewerSessionSpice *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(VirtViewerSessionS > > > pice *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_ses > > > sion_spice_channel_new), self, 0); > > > virt_viewer_signal_connect_object(self->priv->session, > > > "channel-destroy", > > > G_CALLBACK(virt_viewer_ses > > > sion_spice_channel_destroy), self, 0); > > > + virt_viewer_signal_connect_object(self->priv->session, > > > "disconnected", > > > + G_CALLBACK(virt_viewer_ses > > > sion_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(VirtViewerSessionS > > > pice *self) > > > return TRUE; > > > } > > > > > > +static void > > > +virt_viewer_session_spice_session_disconnected(G_GNUC_UNUSED > > > SpiceSession *s, > > > + VirtViewerSession > > > Spice *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
