[Spice-devel] [PATCH spice-server 07/14] agent: use SpiceCharDeviceWriteBuffer for agent data from the client

[Hans de Goede]

Hi,

On 06/27/2012 05:16 PM, Yonit Halperin wrote:
> This is an intermediate patch. The next patch will actually
> push the buffer to the device, instead of copying it.
> ---
>   server/main_channel.c |   10 +++++++++-
>   server/reds.c         |   33 +++++++++++++++++++++++++++++++++
>   server/reds.h         |    2 ++
>   3 files changed, 44 insertions(+), 1 deletions(-)
>
> diff --git a/server/main_channel.c b/server/main_channel.c
> index f9492b0..00a6b0f 100644
> --- a/server/main_channel.c
> +++ b/server/main_channel.c
> @@ -902,8 +902,13 @@ static uint8_t *main_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
>                                                  uint32_t size)
>   {
>       MainChannel *main_chan = SPICE_CONTAINEROF(rcc->channel, MainChannel, base);
> +    MainChannelClient *mcc = SPICE_CONTAINEROF(rcc, MainChannelClient, base);
>
> -    return main_chan->recv_buf;
> +    if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
> +        return reds_get_agent_data_buffer(mcc, size);
> +    } else {
> +        return main_chan->recv_buf;
> +    }
>   }
>
>   static void main_channel_release_msg_rcv_buf(RedChannelClient *rcc,
> @@ -911,6 +916,9 @@ static void main_channel_release_msg_rcv_buf(RedChannelClient *rcc,
>                                                  uint32_t size,
>                                                  uint8_t *msg)
>   {
> +    if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
> +        reds_release_agent_data_buffer(msg);
> +    }
>   }
>
>   static int main_channel_config_socket(RedChannelClient *rcc)
> diff --git a/server/reds.c b/server/reds.c
> index 17b8f7e..5d78d58 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -172,6 +172,8 @@ typedef struct VDIPortState {
>       Ring external_bufs;
>       Ring internal_bufs;
>       Ring write_queue;
> +    SpiceCharDeviceWriteBuffer *recv_from_client_buf;
> +    int recv_from_client_buf_pushed;
>       AgentMsgFilter write_filter;
>
>       /* read from agent */
> @@ -1179,6 +1181,37 @@ void reds_on_main_agent_tokens(MainChannelClient *mcc, uint32_t num_tokens)
>                                                   num_tokens);
>   }
>
> +uint8_t *reds_get_agent_data_buffer(MainChannelClient *mcc, size_t size)
> +{
> +    VDIPortState *dev_state = &reds->agent_state;
> +    RedClient *client;
> +
> +    if (!dev_state->base) {
> +        return NULL;
> +    }
> +
> +    client = main_channel_client_get_base(mcc)->client;
> +    dev_state->recv_from_client_buf = spice_char_device_write_buffer_get(dev_state->base,
> +                                                                         client,
> +                                                                         size + sizeof(VDIChunkHeader));
> +    dev_state->recv_from_client_buf_pushed = FALSE;
> +    return dev_state->recv_from_client_buf->buf + sizeof(VDIChunkHeader);
> +}

This assumes that there will be max 1 agent_data_buffer in flight at all times. So I suggest adding
a "spice_assert(dev_state->recv_from_client_buf == NULL)".

> +void reds_release_agent_data_buffer(uint8_t *buf)
> +{
> +    VDIPortState *dev_state = &reds->agent_state;
> +
> +    spice_assert(buf == dev_state->recv_from_client_buf->buf + sizeof(VDIChunkHeader));
> +
> +    if (!dev_state->recv_from_client_buf_pushed) {
> +        spice_char_device_write_buffer_release(reds->agent_state.base,
> +                                               dev_state->recv_from_client_buf);
> +        dev_state->recv_from_client_buf = NULL;
> +        dev_state->recv_from_client_buf_pushed = FALSE;
> +    }
> +}

And move the dev_state->recv_from_client_buf = NULL out of the if block here.

Regards,

Hans