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

[Fabiano Fidêncio]

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?

>
>>
>> Christophe
>>
>> _______________________________________________
>> virt-tools-list mailing list
>> virt-tools-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/virt-tools-list
>
>
> Best Regards,
> --
> Fabiano Fidêncio


Best Regards,
-- 
Fabiano Fidêncio