On Tue, Sep 26, 2006 at 04:50:37PM +0100, Keir Fraser wrote:
> On 26/9/06 16:26, "Muli Ben-Yehuda" <muli@xxxxxxxxxx> wrote:
>
> > The attached patch makes xenconsole send and receive console messages
> > over a remote connection, instead of via stdin/stdout - if given the
> > --remote switch. It is useful for proxy'ing real console message or
> > other protocol messages (such as gdb) between dom0 and a remote
> > host. We're currently using it for proxying gdb between gdbstub in a
> > partition that talks gdb over the console page to a remote host
> > running gdb.
> >
> > Changed since last version:
> > - fixed compile warning - type of 'buf' in handle_read_fd() should be
> > char*, not unsigned char*.
> >
> > Signed-off-by: Muli Ben-Yehuda <muli@xxxxxxxxxx>
>
> I should have spotted this before, but I'm unsure whether the message buffer
> ring stuff is really needed.
It is necessary in theory, although we might be able to get around it
in practice. The theory says that an fd is not guaranteed to be
writable, so we need a place to store stuff we want to write to it
until it becomes writable. The code isn't that complex (it's even
using Xen style rings now! :-)) so I don't see what we lose by being
correct both in theory and in practice.
If you prefer I can split the buffering support into a separate
patch to make it easier to bisect should the need arise.
> On the write (to the guest) side, we only read from the tty_fd (or
> socket in your case) if there is space in the shared console
> ring. On the read (from the guest) side, we already have a buffer
> (see e.g., buffer_append() in console/daemon/io.c). It may not be
> the best buffer code, and we may want to change it in future to
> support logging to files, but it does work! So can you not just send
> a minimal patch to add socket support? It should be really small
> (like 10s of lines).
I can certainly do that - in fact, that's what I started with. But
even the current xenconsole code suffers from the theoretical problem
mentioned above of writing to an fd without checking that it is
writable first. All it takes to exploit it is to run `xenconsole |
<socket>' and make the system run out of memory so that the socket is
temporarily not writable. Granted, if this happens you have bigger
problems, but why not do things right?
Cheers,
Muli
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|