[libvirt] [RFC] virCommandProcessIO(): make poll() usage more robust

[Laszlo Ersek]

POLLIN and POLLHUP are not mutually exclusive. Currently the following
seems possible: the child writes 3K to its stdout or stderr pipe, and
immediately closes it. We get POLLIN|POLLHUP (I'm not sure that's possible
on Linux, but SUSv4 seems to allow it). We read 1K and throw away the
rest.

When poll() returns and we're about to check the /revents/ member in a
given array element, let's map all the revents bits to two (independent)
ideas: "let's attempt to read()", and "let's attempt to write()". This
should cover all errors, EOFs, and normal conditions; the read()/write()
call should report any pending error.

Under this approach, both POLLHUP and POLLERR are mapped to "needs read()"
if we're otherwise prepared for POLLIN. POLLERR also maps to "needs
write()" if we're otherwise prepared for POLLOUT. The rest of the mappings
(POLLPRI etc.) would be easy, but probably useless for pipes.

Additionally, SUSv4 doesn't appear to forbid POLLIN|POLLERR (or
POLLOUT|POLLERR) set simultaneously. One could argue that the read() or
write() call would return without blocking in these cases (with an error),
so POLLIN / POLLOUT would be justified beside POLLERR.

The code now penalizes POLLIN|POLLERR differently from plain POLLERR. The
former (ie. read() returning -1) is terminal and we jump to cleanup, while
plain POLLERR masks only the affected file descriptor for the future.
Let's unify those.

Build tested only.

Please keep me CC'd, I'm not subscribed. Thanks.

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 src/util/command.c |   17 ++---------------
 1 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/src/util/command.c b/src/util/command.c
index f05493e..dc3cfc5 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -1710,7 +1710,7 @@ virCommandProcessIO(virCommandPtr cmd)
         }
 
         for (i = 0; i < nfds ; i++) {
-            if (fds[i].revents & POLLIN &&
+            if (fds[i].revents & (POLLIN | POLLHUP | POLLERR) &&
                 (fds[i].fd == errfd || fds[i].fd == outfd)) {
                 char data[1024];
                 char **buf;
@@ -1751,7 +1751,7 @@ virCommandProcessIO(virCommandPtr cmd)
                 }
             }
 
-            if (fds[i].revents & POLLOUT &&
+            if (fds[i].revents & (POLLOUT | POLLERR) &&
                 fds[i].fd == infd) {
                 int done;
 
@@ -1777,19 +1777,6 @@ virCommandProcessIO(virCommandPtr cmd)
                     }
                 }
             }
-
-            if (fds[i].revents & (POLLHUP | POLLERR)) {
-                if (fds[i].fd == errfd) {
-                    VIR_DEBUG("hangup on stderr");
-                    errfd = -1;
-                } else if (fds[i].fd == outfd) {
-                    VIR_DEBUG("hangup on stdout");
-                    outfd = -1;
-                } else {
-                    VIR_DEBUG("hangup on stdin");
-                    infd = -1;
-                }
-            }
         }
     }
 
-- 
1.7.1