[Spice-devel] [PATCH] spice-qemu-char: buffer more data going to spice-server

[Uri Lublin]

On 03/05/2011 08:16 PM, Alon Levy wrote:
> Remove the assert that forces spice-server to always consume all data. We
> already had a buffer, so this just means an extra memmove when that data
> is not empty and a second chunk is sent from guest to server.
>
> This should fix BZ 672191, as long as spice chardev is used (and not
> device, which is deprecated).
>
> This fixes assert when sending data faster then server can tcp it to the
> client (happens when copy pasting a screenshot or large file).
>
> A longer term solution is to use the throttling support in virtio-serial,
> to avoid adding yet another buffer.
> ---
>   spice-qemu-char.c |   12 +++++++-----
>   1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index 517f337..8ca4089 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -110,14 +110,16 @@ static int spice_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>
>       dprintf(s, 2, "%s: %d\n", __func__, len);
>       vmc_register_interface(s);
> -    assert(s->datalen == 0);
> -    if (s->bufsize<  len) {
> -        s->bufsize = len;

A note for reviewers, almost always (s->datalen == 0) so the memmove is probably 
does not noticeably change performance.

> +    if (s->datalen>  0) {
> +        memmove(s->buffer, s->datapos, s->datalen);
> +    }
> +    if (s->bufsize<  len + s->datalen) {
> +        s->bufsize = MAX(s->bufsize, MAX(len*2, len + s->datalen));
nitpick, the outer MAX is not needed because of the if-condifition.
I'm not sure how beneficial is the len*2 (before it was len + s->datalen and 
s->datalen was 0).

>           s->buffer = qemu_realloc(s->buffer, s->bufsize);
>       }
> -    memcpy(s->buffer, buf, len);
> +    memcpy(s->buffer + s->datalen, buf, len);
>       s->datapos = s->buffer;
> -    s->datalen = len;
> +    s->datalen += len;
>       spice_server_char_device_wakeup(&s->sin);
>       return len;
>   }


ACK.