Xen devel - Apr 2008 - [PATCH] unnecessary removal of guest console characters

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@eu.citrix.com>


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

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@eu.citrix.com>
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@eu.citrix.com>
> 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@lists.xensource.com
> http://lists.xensource.com/xen-devel


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

Keir Fraser, le Fri 11 Apr 2008 16:03:57 +0100, a écrit
:
> 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.
The guest itself doesn''t hang, the application inside the guest that
produces the output does hang, and that''s what I would expect if I am
not looking at it, rather than the risk of loosing data because of a
non-end-to-end flow control.

Samuel

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

The ''application'' in this case is the kernel. The data in this
case is
console logs and possibly output from an interactive login. Responsiveness
of the former is critical, even, I would say, if that''s at the expense
of
integrity of the latter.

 -- Keir

On 11/4/08 16:11, "Samuel Thibault"
<samuel.thibault@eu.citrix.com> wrote:
> Keir Fraser, le Fri 11 Apr 2008 16:03:57 +0100, a écrit :
>> 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.
> 
> The guest itself doesn''t hang, the application inside the guest
that
> produces the output does hang, and that''s what I would expect if I
am
> not looking at it, rather than the risk of loosing data because of a
> non-end-to-end flow control.
> 
> Samuel


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