[Spice-devel] [PATCH spice-gtk 06/13] migration: set connecting state before fd request

[Christophe Fergeau]

On Sun, Nov 09, 2014 at 05:31:38PM +0100, Marc-André Lureau wrote:
> During migration, the main channel initiating the process is waiting on
> connection completion or error. However, if the migration is cancelled,
> but the main channel state is still NONE, no error event will be fired,
> and the main channel will remain frozen.

Do you mean SPICE_CHANNEL_STATE_UNCONNECTED rather than 'NONE' and are
you referring to the
if (c->state == SPICE_CHANNEL_STATE_UNCONNECTED)
    return;

c->has_error = TRUE; /* break the loop */

blocks in channel_disconnect/spice_channel_disconnect() ?

> 
> Setting connecting state before requesting the channel fd will emit an
> error on disconnect, and cancel migration.
> ---
>  gtk/spice-channel.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index 93c2a78..9375c68 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -2461,10 +2461,8 @@ static gboolean channel_connect(SpiceChannel *channel)
>          g_warning("%s: channel setup incomplete", __FUNCTION__);
>          return false;
>      }
> -    if (c->state != SPICE_CHANNEL_STATE_UNCONNECTED) {
> -        g_warning("Invalid channel_connect state: %d", c->state);
> -        return true;
> -    }
> +
> +    c->state = SPICE_CHANNEL_STATE_CONNECTING;
>  
>      if (spice_session_get_client_provided_socket(c->session)) {
>          if (c->fd == -1) {
> @@ -2476,7 +2474,7 @@ static gboolean channel_connect(SpiceChannel *channel)
>              return true;
>          }
>      }
> -    c->state = SPICE_CHANNEL_STATE_CONNECTING;
> +
>      c->xmit_queue_blocked = FALSE;
>  
>      g_return_val_if_fail(c->sock == NULL, FALSE);
> @@ -2533,6 +2531,11 @@ gboolean spice_channel_open_fd(SpiceChannel *channel, int fd)
>      c = channel->priv;
>      c->fd = fd;
>  
> +    if (c->state > SPICE_CHANNEL_STATE_CONNECTING) {
> +        g_warning("Invalid channel_connect state: %d", c->state);
> +        return true;
> +    }
> +

This hunk is in both spice_channel_connect() and spice_channel_open_fd()
(without the g_warning), I guess this could stay in channel_connect()
and be removed from spice_channel_connect() ? Rereading the commit log,
this movement seems unrelated to what you are fixing anyway?

>      return channel_connect(channel);
>  }
>  
> -- 
> 1.9.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20141112/502de3ca/attachment-0001.sig>