[libvirt] Re: [Libvir] [PATCH] lxc: loop in tty forwarding process

[Daniel Veillard]

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 ?

[...]
>  
> -    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
/* ... */

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/