[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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.

-- 
Best Regards,
Dave Leskovec
IBM Linux Technology Center
Open Virtualization
---
 src/lxc_driver.c |  172 ++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 128 insertions(+), 44 deletions(-)

Index: b/src/lxc_driver.c
===================================================================
--- a/src/lxc_driver.c	2008-05-05 23:21:56.000000000 -0700
+++ b/src/lxc_driver.c	2008-05-07 11:04:38.000000000 -0700
@@ -26,9 +26,10 @@
 #ifdef WITH_LXC
 
 #include <fcntl.h>
-#include <poll.h>
+#include <sys/epoll.h>
 #include <sched.h>
 #include <sys/utsname.h>
+#include <stdbool.h>
 #include <string.h>
 #include <sys/types.h>
 #include <termios.h>
@@ -552,7 +553,7 @@
     int rc = -1;
     char tempTtyName[PATH_MAX];
 
-    *ttymaster = posix_openpt(O_RDWR|O_NOCTTY);
+    *ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK);
     if (*ttymaster < 0) {
         lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
                  _("posix_openpt failed: %s"), strerror(errno));
@@ -593,75 +594,155 @@
 }
 
 /**
+ * lxcFdForward:
+ * @readFd: file descriptor to read
+ * @writeFd: file desriptor to write
+ *
+ * Reads 1 byte of data from readFd and writes to writeFd.
+ *
+ * Returns 0 on success, EAGAIN if returned on read, or -1 in case of error
+ */
+static int lxcFdForward(int readFd, int writeFd)
+{
+    int rc = -1;
+    char buf[2];
+
+    if (1 != (saferead(readFd, buf, 1))) {
+        if (EAGAIN == errno) {
+            rc = EAGAIN;
+            goto cleanup;
+        }
+
+        lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                 _("read of fd %d failed: %s"), readFd, strerror(errno));
+        goto cleanup;
+    }
+
+    if (1 != (safewrite(writeFd, buf, 1))) {
+        lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                 _("write to fd %d failed: %s"), writeFd, strerror(errno));
+        goto cleanup;
+    }
+
+    rc = 0;
+
+cleanup:
+    return rc;
+}
+
+typedef struct _lxcTtyForwardFd_t {
+    int fd;
+    bool active;
+} lxcTtyForwardFd_t;
+
+/**
  * lxcTtyForward:
  * @fd1: Open fd
  * @fd1: Open fd
  *
  * Forwards traffic between fds.  Data read from fd1 will be written to fd2
- * Data read from fd2 will be written to fd1.  This process loops forever.
+ * This process loops forever.
+ * This uses epoll in edge triggered mode to avoid a hard loop on POLLHUP
+ * events when the user disconnects the virsh console via ctrl-]
  *
  * Returns 0 on success or -1 in case of error
  */
 static int lxcTtyForward(int fd1, int fd2)
 {
     int rc = -1;
-    int i;
-    char buf[2];
-    struct pollfd fds[2];
-    int numFds = 0;
-
-    if (0 <= fd1) {
-        fds[numFds].fd = fd1;
-        fds[numFds].events = POLLIN;
-        ++numFds;
+    int epollFd;
+    struct epoll_event epollEvent;
+    int numEvents;
+    int numActive = 0;
+    lxcTtyForwardFd_t fdArray[2];
+    int timeout = -1;
+    int curFdOff = 0;
+    int writeFdOff = 0;
+
+    fdArray[0].fd = fd1;
+    fdArray[0].active = false;
+    fdArray[1].fd = fd2;
+    fdArray[1].active = false;
+
+    /* create the epoll fild descriptor */
+    epollFd = epoll_create(2);
+    if (0 > epollFd) {
+        lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                 _("epoll_create(2) failed: %s"), strerror(errno));
+        goto cleanup;
     }
 
-    if (0 <= fd2) {
-        fds[numFds].fd = fd2;
-        fds[numFds].events = POLLIN;
-        ++numFds;
+    /* add the file descriptors the epoll fd */
+    memset(&epollEvent, 0x00, sizeof(epollEvent));
+    epollEvent.events = EPOLLIN|EPOLLET;    /* edge triggered */
+    epollEvent.data.fd = fd1;
+    epollEvent.data.u32 = 0;                /* fdArray position */
+    if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, fd1, &epollEvent)) {
+        lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                 _("epoll_ctl(fd1) failed: %s"), strerror(errno));
+        goto cleanup;
     }
-
-    if (0 == numFds) {
-        DEBUG0("No fds to monitor, return");
+    epollEvent.data.fd = fd2;
+    epollEvent.data.u32 = 1;                /* fdArray position */
+    if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, fd2, &epollEvent)) {
+        lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                 _("epoll_ctl(fd2) failed: %s"), strerror(errno));
         goto cleanup;
     }
 
     while (1) {
-        if ((rc = poll(fds, numFds, -1)) <= 0) {
+        /* if active fd's, return if no events, else wait forever */
+        timeout = (numActive > 0) ? 0 : -1;
+        numEvents = epoll_wait(epollFd, &epollEvent, 1, timeout);
+        if (0 < numEvents) {
+            if (epollEvent.events & EPOLLIN) {
+                curFdOff = epollEvent.data.u32;
+                if (!fdArray[curFdOff].active) {
+                    fdArray[curFdOff].active = true;
+                    ++numActive;
+                }
 
-            if ((0 == rc) || (errno == EINTR) || (errno == EAGAIN)) {
+            } else if (epollEvent.events & EPOLLHUP) {
+                DEBUG("EPOLLHUP from fd %d", epollEvent.data.fd);
                 continue;
+            } else {
+                lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                         _("error event %d"), epollEvent.events);
+                goto cleanup;
             }
 
-            lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                     _("poll returned error: %s"), strerror(errno));
-            goto cleanup;
-        }
+        } else if (0 == numEvents) {
+            if (2 == numActive) {
+                /* both fds active, toggle between the two */
+                curFdOff ^= 1;
+            } else {
+                /* only one active, if current is active, use it, else it */
+                /* must be the other one (ie. curFd just went inactive) */
+                curFdOff = fdArray[curFdOff].active ? curFdOff : curFdOff ^ 1;
+            }
 
-        for (i = 0; i < numFds; ++i) {
-            if (!fds[i].revents) {
+        } else  {
+            if (EINTR == errno) {
                 continue;
             }
 
-            if (fds[i].revents & POLLIN) {
-                if (1 != (saferead(fds[i].fd, buf, 1))) {
-                    lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                             _("read of fd %d failed: %s"), i,
-                             strerror(errno));
-                    goto cleanup;
-                }
-
-                if (1 < numFds) {
-                    if (1 != (safewrite(fds[i ^ 1].fd, buf, 1))) {
-                        lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                                 _("write to fd %d failed: %s"), i,
-                                 strerror(errno));
-                        goto cleanup;
-                    }
+            /* error */
+            lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                     _("epoll_wait() failed: %s"), strerror(errno));
+            goto cleanup;
 
-                }
+        }
 
+        if (0 < numActive) {
+            writeFdOff = curFdOff ^ 1;
+            rc = lxcFdForward(fdArray[curFdOff].fd, fdArray[writeFdOff].fd);
+
+            if (EAGAIN == rc) {
+                /* this fd no longer has data, set it as inactive */
+                --numActive;
+                fdArray[curFdOff].active = false;
+            } else if (-1 == rc) {
+                goto cleanup;
             }
 
         }
@@ -671,7 +752,10 @@
     rc = 0;
 
 cleanup:
-    return rc;
+    close(fd1);
+    close(fd2);
+    close(epollFd);
+    exit(rc);
 }
 
 /**

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]