On Tue, May 06, 2008 at 12:51:08AM -0700, Dave Leskovec wrote: > This patch changes the lxc tty forwarding process to use epoll instead of poll. > This is done to avoid a cpu consuming loop when a user disconnects from the > container console. > > During some testing, we found that when the slave end of a tty is closed, calls > to poll() on the master end will return immediately with POLLHUP until the slave > end is opened again. The result of this is that if you connect to the container > console using virsh and then ctrl-] out of it, the container tty process will > chew up the processor handling POLLHUP. This event can't be disabled regardless > of what is set in the events field. > > This can be avoided by using epoll. When used in edge triggered mode, you see > the initial EPOLLHUP but will not see another one until someone connects and > then disconnects from the console again. This also drove some changes into how > the regular input data is handled. Once an EPOLLIN is seen from an fd, another > one will not be surfaced until all data has been read from the file (EAGAIN is > returned by read). Sounds fine in principle but i have a couple of questions with the patch > +#include <stdbool.h> err ... what is that ? looks like a linux specific header, do we really need this ? epoll is linux specific I think but #include <sys/epoll.h> should be sufficient no ?

Daniel Veillard <veillard(a)redhat.com&gt; wrote: > On Tue, May 06, 2008 at 12:51:08AM -0700, Dave Leskovec wrote: > Sounds fine in principle but i have a couple of questions with the patch > >> +#include <stdbool.h> > > err ... what is that ? looks like a linux specific header, do we really > need this ? epoll is linux specific I think but #include <sys/epoll.h> > should be sufficient no ? <stdbool.h> is not Linux specific. It's the C99-specified header that provides e.g,. the "bool" type. Good C code has been able to use the "bool" type portably (at least through autoconf/gnulib-provided insulation) for many years. These days you rarely need the compatibility shims, since nearly everyone has a c99-compliant compiler.

Daniel Veillard wrote: > On Tue, May 06, 2008 at 12:51:08AM -0700, Dave Leskovec wrote: [...] >> - close(vm->parentTty); >> + //close(vm->parentTty); >> close(vm->containerTtyFd); > > if we really don't need this anymore just remove it, if you have doubts then > maybe this should be clarified. In any case let's stick to old style comments > /* ... */ > That shouldn't be commented out. I've restored it in the attached updated patch.