[Spice-devel] [PATCH spice 3/3] server/reds: Send the agent a CLIENT_DISCONNECTED msg on client disconnect

[Alon Levy]

Looks good to me, couldn't resist a nitpick though.

> Client -> agent messages can spawn multiple VDIChunks. When this
> happens
> the agent re-assembles the chunks into a complete VDAgentMessage
> before
> processing it. The server only guarentees coherency at the chunk
> level,
> so it is not possible for a partial chunk to get delivered to the
> agent.
> 
> But it is possible for some chunks of a VDAgentMessage to be
> delivered to
> the agent followed by a client to disconnect without the rest of the
> VDAgentMessage being delivered!
> 
> This will leave the agent in a wrong state, and the first messages
> send to it
> by the next client to connect will get seen as the rest of the
> VDAgentMessage
> from the previous client.
> 
> This patch sends the agent a new VD_AGENT_CLIENT_DISCONNECTED message
> from the
> VDP_SERVER_PORT, on which the agent can then reset its
> VDP_CLIENT_PORT state.
> 
> Note that no capability check is done for this, since the
> capabilities are
> something negotiated between client and agent. The server will simply
> always
> send this message on client disconnect, relying on older agents
> discarding the
> message since it has an unknown type (which both the windows and
> linux agents
> already do).
> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  server/reds.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index ec80e9e..5c46909 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -563,7 +563,29 @@ void reds_client_disconnect(RedClient *client)
>     // TODO: we need to handle agent properly for all clients!!!!
>     (e.g., cut and paste, how? Maybe throw away messages
>     // if we are in the middle of one from another client)
>      if (reds->num_clients == 0) {
> -       /* Reset write filter to start with clean state on client
> reconnect */
> +        /* Let the agent know the client is disconnected */
> +        if (reds->agent_state.base) {
> +            SpiceCharDeviceWriteBuffer *char_dev_buf;
> +            VDInternalBuf *internal_buf;
> +            uint32_t total_msg_size;
> +
> +            total_msg_size = sizeof(VDIChunkHeader) +
> sizeof(VDAgentMessage);
> +            char_dev_buf =
> spice_char_device_write_buffer_get_server_no_token(
> +                               reds->agent_state.base,
> total_msg_size);
> +            char_dev_buf->buf_used = total_msg_size;
> +            internal_buf = (VDInternalBuf *)char_dev_buf->buf;
> +            internal_buf->chunk_header.port = VDP_SERVER_PORT;
> +            internal_buf->chunk_header.size =
> sizeof(VDAgentMessage);
> +            internal_buf->header.protocol = VD_AGENT_PROTOCOL;
> +            internal_buf->header.type =
> VD_AGENT_CLIENT_DISCONNECTED;
> +            internal_buf->header.opaque = 0;
> +            internal_buf->header.size = 0;

<nitpick> seriously verbose for an empty message, should probably factor out </nitpick>

> +
> +
>            spice_char_device_write_buffer_add(reds->agent_state.base,
> +                                               char_dev_buf);
> +        }
> +
> +        /* Reset write filter to start with clean state on client
> reconnect */
>          agent_msg_filter_init(&reds->agent_state.write_filter,
>          agent_copypaste,
>                                TRUE);
>  
> --
> 1.8.1.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>