Re: [Xen-devel] [PATCH 5 of 9] libxl: only use interactive PyGrub mode when a console is attached

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2011-10-11 at 11:45 +0100, Roger Pau Monnà wrote:
> 2011/10/11 Ian Campbell <Ian.Campbell@xxxxxxxxxx>:
> > On Mon, 2011-10-10 at 15:51 +0100, Roger Pau Monnà wrote:
> >> > That sounds unfortunate.  I agree with you.
> >>
> >> Well Ian Campbell proposed to use a timeout in select and drop data
> >> when the timeout is hit, that what I've implemented in
> >>
> >> libxl: drop bootloader output if xenconsole file descriptor is not
> >> available for writing
> >>
> >> The patch is quite simple and solves the problem, I don't think it
> >> interferes with normal boot process, since a 1s timeout is quite high
> >> for data to be delivered from xenconsoled_fd to the user if the
> >> console is attached.
> >
> > The case where a user types
> >        xl create /a/domain
> >        xl console adomain
> >
> > might have a reasonably long delay in it before the console is
> > connected. Increasing the libxl buffer size would mitigate that but
> > you'd still have the problem of not reading from an fd if the cons is !=
> > 0.
> >
> > Can you print out the actual values of the various producers and
> > consumers at the point of the hang? (maybe track total bytes too for
> > debug?)
> 
> 
> The process gets stuck after writing 1022 bytes to xenconsole_fd, and
> the buffer is 24 consumed 48 produced (although that varies, sometimes
> I get ~700 produced and ~20 consumed, but the total written bits is
> 1022 always).

Thanks.

> 
> > Perhaps where we have:
> >        if (xenconsoled_prod == xenconsoled_cons)
> >            xenconsoled_prod = xenconsoled_cons = 0;
> >        if (bootloader_prod == bootloader_cons)
> >            bootloader_prod = bootloader_cons = 0;
> > We could also add some suitable memmoves and associated prod/cons
> > manipulation, such that the unconsumed data is always pulled to the head
> > of the buffer. That might be a reasonably simple way to improve things
> > and allow larger buffers to be used? More often than not the memmove
> > won't be getting called since either XXX_prod == XXX_cons or XXX_cons is
> > already 0, IOW having a partially consumed buffer would be unusual?
> 
> I don't really like moving memory around, since it's slow, but I think
> implementing a circular buffer would really complicate things.

I suspect that under normal use the move will never happen because we
will always have either written the entire buffer (so cons==prod,
cheaply reset both to zero) or none of it (so cons==0 and there is
nothing to do).

In the case you are suffering from I expect it will happen exactly once
and then we will be in the cons==0 state from then on unless we become
able to write something.

If that is correct then I don't think the move will be a problem in
practice. Also the buffers aren't all that big so moving them cant bee
all that expensive in the grand scheme of things.

libxl configures xenconsoled to have a limit of 1048576
(LIBXL_XENCONSOLE_LIMIT) by default. Hopefully we can get away with
something smaller for the bootloader interactions.

>  I will
> post a new patch that implements the following:
> 
> 1. Increase buffer.
> 2. Use memmove to move memory to the head of the buffer and fill it
> until the buffer is full.
> 3. On timeout and full buffer, drop data from the head of the buffer
> (older data) and fill it with new data (move the buffer and append new
> data at the end).

This sounds good.

When xenconsoled drops guest console data it seems to drop data from the
middle. I'm not sure why though. The checkin comment of 2b8efe11096b8
doesn't really go into detail on that aspect.

> Do you thing this should be applied to both xenconsoled and
> bootloader? since the only one giving problems is xenconsoled.

The bootloader buffer is currently larger so we probably don't see any
problems but in theory it's just as vulnerable? We probably don't need
to increase the buffer any further but the other changes probably make
sense.

> > Once we have a larger buffer which we always try to fill discarding data
> > after a timeout when the buffer is full won't be so critical.



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