[virt-tools-list] [virt-viewer][PATCH 2/3] session-spice: Destroy the channel instead of emit a "session-disconnect" signal

[Christophe Fergeau]

On Thu, Apr 09, 2015 at 04:11:42PM +0200, Fabiano Fidêncio wrote:
> On Thu, Apr 9, 2015 at 2:36 PM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
> > On Thu, Apr 9, 2015 at 2:16 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> >> On Thu, Apr 09, 2015 at 01:36:50PM +0200, Fabiano Fidêncio wrote:
> >>> Whenever we reach this branch while connecting we will  create a new
> >>> spice session (from the dialog showed to the user). So, destroying the
> >>> channel on this situation seems sane enough.
> >>> It also avoids an error dialog to be popped out twice with (basically)
> >>> the same information.
> >>>
> >>> Related to: rhbz#1085216
> >>> ---
> >>>  src/virt-viewer-session-spice.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> >>> index 851d2f6..10dd149 100644
> >>> --- a/src/virt-viewer-session-spice.c
> >>> +++ b/src/virt-viewer-session-spice.c
> >>> @@ -610,7 +610,7 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel,
> >>>                  spice_session_connect(self->priv->session);
> >>>              }
> >>>          } else {
> >>> -            g_signal_emit_by_name(session, "session-disconnected", error ? error->message : NULL);
> >>> +            virt_viewer_session_spice_channel_destroy(NULL, channel, session);
> >>
> >> Are the SPICE_CHANNEL_ERROR_IO, SPICE_CHANNEL_ERROR_LINK,
> >> SPICE_CHANNEL_ERROR_TLS below different or should they also destroy the
> >> channel rather than emitting a "session-disconnected" signal?
> >
> > That's a good question :-)
> > Let me try to reach those errors and see if we can destroy the channel
> > instead of emitting a "session-disconnected" signal.
> 
> Actually, reaching those errors is not something easy to do.
> >From simple tests here forcing ERROR_IO and ERROR_LINK on spice-gtk,
> seems we can also destroy the channel on these situations.
> For the ERROR_IO, if we don't do this, an error is popped out twice.
> For the ERROR_LINK, there is no error dialog being showed at all, with
> or without my patch (but this is a different problem to be addressed
> in a different patch).
> For the ERROR_TLS, as far as I can see in the spice-gtk code,
> everytime we reach this error, we don't even try to reconnect. What
> makes me think that we can just destroy the channel in this case as
> well.
> 
> Would you prefer the change for _ERROR_{IO,LINK,TLS} on this patch or
> would be better for you to have it on a different one?

I'd do it in this patch. Sorry for the delay!

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20150415/1c5a8bc4/attachment.sig>