Looks good, thanks for fixing this.
ack
On Thu, Oct 10, 2013 at 5:15 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> After the previous commit, spice_channel_switch_protocol() is now
> called when needed, but it's not doing anything. What happens is
> that spice_channel_switch_protocol() triggers a channel disconnection
> and then it queues an idle to reconnect (after having changed the
> protocol version to be used).
>
> When spice_channel_recv_link_hdr() returns, we need to jump out of
> the coroutine to let the idle trigger and the new channel coroutine
> be started. But jumping out of the coroutine will call channel_disconnect()
> which calls channel_reset() which disables the idle switch_protocol()
> just queued. This causes the connection attempt to be apparently
> stuck with nothing happening.
>
> Falling back to the older SPICE protocol is not the only situation
> when we need to drop the current connection attempt and reconnect,
> we also need to do that when the remote server returns
> SPICE_LINK_ERR_NEED_SECURED to let the client know it needs to use
> a secure port for this channel. This is handled by the 'switch_tls'
> variable set in spice_channel_recv_link_msg and handled in
> spice_channel_coroutine().
>
> 'switch_tls' does the same thing as spice_channel_switch_protocol(),
> except that it calls spice_channel_connect() after channel_disconnect()
> has been called, which means the idle queued by channel_connect()
> won't get cleared.
>
> This all that commit does, remove the spice_channel_switch_protocol()
> method and use the same codepath as 'switch_tls' to handle the
> reconnection.
> ---
> gtk/spice-channel.c | 22 ++++++++--------------
> 1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index c0e7bba..08418f7 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -1169,21 +1169,13 @@ static void spice_channel_send_link(SpiceChannel *channel)
> free(buffer);
> }
>
> -static void spice_channel_switch_protocol(SpiceChannel *channel, gint version)
> -{
> - SpiceChannelPrivate *c = channel->priv;
> -
> - g_object_set(c->session, "protocol", version, NULL);
> - SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
> - spice_channel_connect(channel);
> -}
> -
> /* coroutine context */
> -static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel)
> +static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel, gboolean *switch_protocol)
> {
> SpiceChannelPrivate *c = channel->priv;
> int rc;
>
> + *switch_protocol = FALSE;
> rc = spice_channel_read(channel, &c->peer_hdr, sizeof(c->peer_hdr));
> if (rc != sizeof(c->peer_hdr)) {
> g_warning("incomplete link header (%d/%" G_GSIZE_FORMAT ")",
> @@ -1216,7 +1208,8 @@ error:
> incompatible. Try with the oldest protocol in this case: */
> if (c->link_hdr.major_version != 1) {
> SPICE_DEBUG("%s: error, switching to protocol 1 (spice 0.4)", c->name);
> - spice_channel_switch_protocol(channel, 1);
> + *switch_protocol = TRUE;
> + g_object_set(c->session, "protocol", 1, NULL);
> return FALSE;
> }
>
> @@ -2228,6 +2221,7 @@ static void *spice_channel_coroutine(void *data)
> guint verify;
> int rc, delay_val = 1;
> gboolean switch_tls = FALSE;
> + gboolean switch_protocol = FALSE;
>
> CHANNEL_DEBUG(channel, "Started background coroutine %p", &c->coroutine);
>
> @@ -2350,7 +2344,7 @@ connected:
> }
>
> spice_channel_send_link(channel);
> - if (spice_channel_recv_link_hdr(channel) == FALSE)
> + if (spice_channel_recv_link_hdr(channel, &switch_protocol) == FALSE)
> goto cleanup;
> spice_channel_recv_link_msg(channel, &switch_tls);
> if (switch_tls)
> @@ -2365,8 +2359,8 @@ cleanup:
>
> SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
>
> - if (switch_tls && !c->tls) {
> - c->tls = true;
> + if (switch_protocol || (switch_tls && !c->tls)) {
> + c->tls = switch_tls;
> spice_channel_connect(channel);
> g_object_unref(channel);
> } else
> --
> 1.8.3.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel