On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@xxxxxxxxxx>
The current I/O code for LXC uses a hand crafted event loop
to forward I/O between the container& host app, based on
epoll to handle EOF on PTYs. This event loop is not easily
extendable to add more consoles, or monitor other types of
s/extendable/extensible/
file descriptors.
Remove the custom event loop and replace it with a normal
libvirt event loop. When detecting EOF on a PTY, disable
the event watch on that FD, and fork off a background thread
that does a edge-triggered epoll() on the FD. When the FD
finally shows new incoming data, the thread re-enables the
watch on the FD and exits.
When getting EOF from a read() on the PTY, the existing code
would do waitpid(WNOHANG) to see if the container had exited.
Unfortunately there is a race condition, because even though
the process has closed its stdio handles, it might still
exist.
To deal with this the new event loop uses a SIG_CHILD handler
to perform the waitpid only when the container is known to
have actually exited.
* src/lxc/lxc_controller.c: Rewrite the event loop to use
the standard APIs.
---
cfg.mk | 2 +-
src/lxc/lxc_controller.c | 607 ++++++++++++++++++++++++++++++----------------
2 files changed, 404 insertions(+), 205 deletions(-)
Big; hopefully I get through it today.
+
+static void lxcConsoleEOFThread(void *opaque)
+{
+ struct lxcConsoleEOFData *data = opaque;
+ int ret;
+ int epollfd = -1;
+ struct epoll_event event;
+
+ VIR_WARN("MOnitor %d", data->fd);
s/MOnitor/Monitor/
should this be VIR_DEBUG instead of VIR_WARN?
+ if ((epollfd = epoll_create(2))< 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to create epoll fd"));
+ goto cleanup;
+ }
Should we be using epoll_create1(EPOLL_CLOEXEC) instead?
+static void lxcConsoleIO(int watch, int fd, int events, void *opaque)
+{
...
+ if (done> 0)
+ *len += done;
+ else {
+ VIR_WARN("Read fd %d done %d errno %d", fd, (int)done, errno);
+ }
Use {} on both branches or neither, but not just one branch.
+error:
+ virEventRemoveHandle(console->contWatch);
+ virEventRemoveHandle(console->hostWatch);
+ console->contWatch = console->hostWatch = -1;
+ quit = true;
+ virMutexUnlock(&lock);
Some of your code set 'quit = true' outside the 'lock' mutex; was that
intentional?
+ if (virMutexInit(&lock)< 0)
+ goto cleanup2;
+
+ if (pipe(sigpipe)< 0) {
Shouldn't the signal pipe be non-blocking? I'm not sure if cloexec
matters, but for consistency, we might as well set it. In other words,
should this be:
pipe2(sigpipe, O_CLOEXEC|O_NONBLOCK)
+ if (virEventAddHandle(sigpipe[0],
+ VIR_EVENT_HANDLE_READABLE,
+ lxcSignalChildIO,
+&container,
+ NULL)< 0) {
+ lxcError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to watch signal socket"));
s/socket/pipe/
ACK with nits fixed.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list