[Spice-devel] [PATCH spice-gtk] channel: switch to protocol 1 on error during link-time

[Christophe Fergeau]

On Wed, Dec 05, 2012 at 11:43:51AM +0100, Marc-André Lureau wrote:
> The Spice server doesn't wait until all the data are received by the
> remote before closing the socket (that would need SO_LINGER?). Under
> some racy conditions, the client may not have received the link reply
> indicating the server protocol version mismatch, which would trigger
> reconnection with compatible protocol.
> 
> It seems to happen on local networks with Windows sockets (error
> WSAECONNRESET). To workaround that issue, spice-gtk can try to
> reconnect with protocol 1 when a socket error is encoutered during
> link-time.
> 
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=874698
> ---
>  gtk/spice-channel.c | 61 ++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 37 insertions(+), 24 deletions(-)
> 
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index 36fe21e..cff047d 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -1164,6 +1164,15 @@ 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 void spice_channel_recv_link_hdr(SpiceChannel *channel)
>  {
> @@ -1172,39 +1181,40 @@ static void spice_channel_recv_link_hdr(SpiceChannel *channel)
>  
>      rc = spice_channel_read(channel, &c->peer_hdr, sizeof(c->peer_hdr));
>      if (rc != sizeof(c->peer_hdr)) {
> -        g_critical("incomplete link header (%d/%" G_GSIZE_FORMAT ")",
> -                   rc, sizeof(c->peer_hdr));
> +        g_warning("incomplete link header (%d/%" G_GSIZE_FORMAT ")",
> +                  rc, sizeof(c->peer_hdr));
>          goto error;
>      }
>      if (c->peer_hdr.magic != SPICE_MAGIC) {
> -        g_critical("invalid SPICE_MAGIC!");
> +        g_warning("invalid SPICE_MAGIC!");
>          goto error;
>      }
>  
>      CHANNEL_DEBUG(channel, "Peer version: %d:%d", c->peer_hdr.major_version, c->peer_hdr.minor_version);
>      if (c->peer_hdr.major_version != c->link_hdr.major_version) {
> -        if (c->peer_hdr.major_version == 1) {
> -            /* enter spice 0.4 mode */
> -            g_object_set(c->session, "protocol", 1, NULL);
> -            CHANNEL_DEBUG(channel, "switching to protocol 1 (spice 0.4)");
> -            SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
> -            spice_channel_connect(channel);
> -            return;
> -        }
> -        g_critical("major mismatch (got %d, expected %d)",
> -                   c->peer_hdr.major_version, c->link_hdr.major_version);
> +        g_warning("major mismatch (got %d, expected %d)",
> +                  c->peer_hdr.major_version, c->link_hdr.major_version);
>          goto error;
>      }
>  
>      c->peer_msg = spice_malloc(c->peer_hdr.size);
>      if (c->peer_msg == NULL) {
> -        g_critical("invalid peer header size: %u", c->peer_hdr.size);
> +        g_warning("invalid peer header size: %u", c->peer_hdr.size);
>          goto error;
>      }

This whole check is not needed as spice_malloc will abort on
allocation failures anyway.
Looks good otherwise,

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20121217/65afaed3/attachment.pgp>