[Spice-devel] [PATCH] red_worker.c: fix not destroying streams before sending MSG_MIGRATE

[Hans de Goede]

ACK.

On 04/08/2013 10:24 PM, Yonit Halperin wrote:
> When qemu migration completes, we need to stop the streams, and to send
> the corresponding upgrade_items to the client.
> Otherwise, (1) the client might display lossy regions that we don't track
> (streams are not part of the migration data).
> (2) streams_timeout may occur after MSG_MIGRATE has been sent, leading
> to messages being sent to the client after MSG_MIGRATE and before
> MSG_MIGRATE_DATA (e.g., STREAM_CLIP, STREAM_DESTROY, DRAW_COPY).
> No message besides MSG_MIGRATE_DATA should be sent after
> MSG_MIGRATE.
>
> When a msg other than MIGRATE_DATA reached spice-gtk after MSG_MIGRATE,
> spice-gtk sent it to dest server as the migration data, and the dest
> server crashed with a "bad message size" assert.
> ---
>   server/red_worker.c | 25 ++++++++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 25824b4..4842ad6 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -9171,10 +9171,33 @@ void red_disconnect_all_display_TODO_remove_me(RedChannel *channel)
>       red_channel_apply_clients(channel, red_channel_client_disconnect);
>   }
>
> +static void red_destroy_streams(RedWorker *worker)
> +{
> +    RingItem *stream_item;
> +
> +    spice_debug(NULL);
> +    while ((stream_item = ring_get_head(&worker->streams))) {
> +        Stream *stream = SPICE_CONTAINEROF(stream_item, Stream, link);
> +
> +        red_detach_stream_gracefully(worker, stream, NULL);
> +        red_stop_stream(worker, stream);
> +    }
> +}
> +
>   static void red_migrate_display(RedWorker *worker, RedChannelClient *rcc)
>   {
> +    /* We need to stop the streams, and to send upgrade_items to the client.
> +     * Otherwise, (1) the client might display lossy regions that we don't track
> +     * (streams are not part of the migration data) (2) streams_timeout may occur
> +     * after the MIGRATE message has been sent. This can result in messages
> +     * being sent to the client after MSG_MIGRATE and before MSG_MIGRATE_DATA (e.g.,
> +     * STREAM_CLIP, STREAM_DESTROY, DRAW_COPY)
> +     * No message besides MSG_MIGRATE_DATA should be sent after MSG_MIGRATE.
> +     * Notice that red_destroy_streams won't lead to any dev ram changes, since
> +     * handle_dev_stop already took care of releasing all the dev ram resources.
> +     */
> +    red_destroy_streams(worker);
>       if (red_channel_client_is_connected(rcc)) {
> -        red_pipe_add_verb(rcc, SPICE_MSG_DISPLAY_STREAM_DESTROY_ALL);
>           red_channel_client_default_migrate(rcc);
>       }
>   }
>