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

[libvirt] lxc_controller: don't ignore failed "accept"



Initially I proposed a similar patch here:

    http://thread.gmane.org/gmane.comp.emulators.libvirt/20607

That thread languished, and Eric proposed a nearly identical patch:

    http://thread.gmane.org/gmane.comp.emulators.libvirt/21630

Here's a patch that should address the initial objection.
Now, my only question is about which errno values to ignore.
Obviously, EMFILE, EFAULT, etc. must not be ignored.
I doubt EINTR and EBADF should be ignored, but haven't
tried to prove the case.

>From 6dcd15b0aa594f3d89e810e0a92aa75dab22903e Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering redhat com>
Date: Thu, 20 May 2010 14:30:36 +0200
Subject: [PATCH] lxc_controller.c: don't ignore failed "accept"

* src/lxc/lxc_controller.c (ignorable_epoll_accept_errno): New function.
(lxcControllerMain): Handle a failed accept carefully:
most errno values indicate legitimate failure and must be fatal.
However, ignore a special case: that in which an incoming client quits
between the poll() indicating its presence, and our accept() which
is trying to process it.
---
 src/lxc/lxc_controller.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 6b64372..75a45e9 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -269,6 +269,17 @@ typedef struct _lxcTtyForwardFd_t {
     int active;
 } lxcTtyForwardFd_t;

+/* Return true if it is ok to ignore an accept-after-epoll syscall
+   that fails with the specified errno value.  Else false.  */
+static bool
+ignorable_epoll_accept_errno(int erratum)
+{
+  return (errnum == EINVAL
+          || errnum == ECONNABORTED
+          || errnum == EAGAIN
+          || errnum == EWOULDBLOCK);
+}
+
 /**
  * lxcControllerMain
  * @monitor: server socket fd to accept client requests
@@ -350,6 +361,18 @@ static int lxcControllerMain(int monitor,
         if (numEvents > 0) {
             if (epollEvent.data.fd == monitor) {
                 int fd = accept(monitor, NULL, 0);
+                if (fd < 0) {
+                    /* First reflex may be simply to declare accept failure
+                       to be a fatal error.  However, accept may fail when
+                       a client quits between the above epoll_wait and here.
+                       That case is not fatal, but rather to be expected,
+                       if not common, so ignore it.  */
+                    if (ignorable_epoll_accept_errno(errno))
+                        continue;
+                    virReportSystemError(errno, "%s",
+                                         _("accept(monitor,...) failed"));
+                    goto cleanup;
+                }
                 if (client != -1) { /* Already connected, so kick new one out */
                     close(fd);
                     continue;
--
1.7.1.262.g5ef3d


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