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

[Marc-André Lureau]

Hi

On Fri, Jan 4, 2019 at 11:08 PM Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
>
> 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?

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 at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
>
>
>
> --
> Marc-André Lureau



-- 
Marc-André Lureau