Re: [Xen-devel] [PATCH] unnecessary removal of guest console characters

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

Buffering happens in the daemon, not in the inter-domain ring. If we switch
to your strategy then overflow leads to the guest hanging rather than
characters being dropped. And this could happen, for example, if no client
is connected to xenconsoled and emptying the buffers. So I know which
strategy I prefer! Large buffers in the backend daemon, plus rate-limiting
of bursts, is the right answer.

 -- Keir

On 11/4/08 14:46, "Stefano Stabellini" <stefano.stabellini@xxxxxxxxxxxxx>
wrote:

> Hi all,
> currently when max_capacity is set in xenconsoled and the buffer if
> completely filled then some characters in the buffer are discarded (even
> though the rate limitation makes this unlikely to happen).
> A better way to handle this scenario would be to stop reading from the
> ring until the buffer has some free space.
> I am attaching a simple patch that does exactly this.
> Best Regards,
> 
> Stefano Stabellini
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> diff -r 082d3886fded tools/console/daemon/io.c
> --- a/tools/console/daemon/io.c Fri Apr 11 09:14:03 2008 +0100
> +++ b/tools/console/daemon/io.c Fri Apr 11 14:12:51 2008 +0100
> @@ -200,21 +200,6 @@ static void buffer_append(struct domain
>      "on domain %d: %d (%s)\n",
>      dom->domid, errno, strerror(errno));
> }
> -
> - if (buffer->max_capacity &&
> -     buffer->size > buffer->max_capacity) {
> -  /* Discard the middle of the data. */
> -
> -  size_t over = buffer->size - buffer->max_capacity;
> -  char *maxpos = buffer->data + buffer->max_capacity;
> -
> -  memmove(maxpos - over, maxpos, over);
> -  buffer->data = realloc(buffer->data, buffer->max_capacity);
> -  buffer->size = buffer->capacity = buffer->max_capacity;
> -
> -  if (buffer->consumed > buffer->max_capacity - over)
> -   buffer->consumed = buffer->max_capacity - over;
> - }
>  }
>  
>  static bool buffer_empty(struct buffer *buffer)
> @@ -228,6 +213,11 @@ static void buffer_advance(struct buffer
> if (buffer->consumed == buffer->size) {
> buffer->consumed = 0;
> buffer->size = 0;
> +  if (buffer->max_capacity &&
> +      buffer->capacity > buffer->max_capacity) {
> +   buffer->data = realloc(buffer->data, buffer->max_capacity);
> +   buffer->capacity = buffer->max_capacity;
> +  }
> }
>  }
>  
> @@ -1005,9 +995,12 @@ void handle_io(void)
>    d->next_period < next_timeout)
> next_timeout = d->next_period;
> } else if (d->xce_handle != -1) {
> -    int evtchn_fd = xc_evtchn_fd(d->xce_handle);
> -    FD_SET(evtchn_fd, &readfds);
> -    max_fd = MAX(evtchn_fd, max_fd);
> +    if (!d->buffer.max_capacity ||
> +        d->buffer.size < d->buffer.max_capacity) {
> +     int evtchn_fd = xc_evtchn_fd(d->xce_handle);
> +     FD_SET(evtchn_fd, &readfds);
> +     max_fd = MAX(evtchn_fd, max_fd);
> +    }
> }
>  
> if (d->master_fd != -1) {
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel