[Spice-devel] [spice-server] dcc: Fix seamless migration

[Frediano Ziglio]

> 
> Since commit ef4b1bdb "red-channel-client: Prevent too tight loop
> waiting for ACKs", after seamless migration, the display is no longer
> updated on the client-side, even though the VM is functional (responds
> to keyboard input, reconnecting the client restores the display
> functionality, ...).
> 
> This is mainly caused because after migration,
> red_channel_client_waiting_for_ack() will be true until
> red_channel_client_ack_zero_messages_window() is called.
> 
> What happens is the following:
> The dcc is created, and dcc_start() pushes a RED_PIPE_ITEM_TYPE_SET_ACK
> message.
> This calls prepare_pipe_add(), which will enable write event on the dcc
> watch. red_channel_client_event() will be called, which will trigger a
> red_channel_client_push(). Since red_channel_client_waiting_for_ack()
> returns true, we won't get any item to push, and (because of commit
> ef4b1bdb), we will disable write notifications on the watch.
> At this point, rcc->priv->pipe is no longer empty, so prepare_pipe_add()
> is not going to reenable the write notifications.
> 
> Then red_channel_client_ack_zero_messages_window() is finally called as
> part of dcc_handle_migrate_data(), so from this point on,
> red_channel_client_waiting_for_ack() is no longer true.
> 
> However, nothing ever reenables WRITE events, nor empties
> rcc->priv->pipe, so nothing ever gets pushed, causing no display updates
> at all after a migration, even if the VM is functional (input, ...)
> apart from that.
> 
> This commit reenables WRITE events in
> red_channel_client_ack_zero_messages_window() if we were waiting for
> ack.
> 
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>  server/red-channel-client.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> index bc1d0f3c4..967c93ed2 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -1323,6 +1323,10 @@ void red_channel_client_push(RedChannelClient *rcc)
>      while ((pipe_item = red_channel_client_pipe_item_get(rcc))) {
>          red_channel_client_send_item(rcc, pipe_item);
>      }
> +    /* prepare_pipe_add() will reenable WRITE events when the
> rcc->priv->pipe is empty
> +     * red_channel_client_ack_zero_messages_window() will reenable WRITE
> events
> +     * if we were waiting for acks to be received
> +     */
>      if ((red_channel_client_no_item_being_sent(rcc) &&
>      g_queue_is_empty(&rcc->priv->pipe)) ||
>          red_channel_client_waiting_for_ack(rcc)) {
>          red_channel_client_watch_update_mask(rcc, SPICE_WATCH_EVENT_READ);
> @@ -1671,6 +1675,10 @@ static void
> red_channel_client_pipe_clear(RedChannelClient *rcc)
>  
>  void red_channel_client_ack_zero_messages_window(RedChannelClient *rcc)
>  {
> +    if (red_channel_client_waiting_for_ack(rcc)) {
> +        red_channel_client_watch_update_mask(rcc,
> +
> SPICE_WATCH_EVENT_READ|SPICE_WATCH_EVENT_WRITE);
> +    }
>      rcc->priv->ack_data.messages_window = 0;
>  }
>  

Looks good. I would remove the if condition.

Frediano