From d19149dda888d36cea58b6cdf7446f98bd1bf734 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Tue, 24 Jan 2012 15:55:19 +0100 Subject: [PATCH] virCommandProcessIO(): make poll() usage more robust 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. Signed-off-by: Laszlo Ersek --- AUTHORS | 1 + src/util/command.c | 17 ++--------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/AUTHORS b/AUTHORS index 74f3157746..783c48a16e 100644 --- a/AUTHORS +++ b/AUTHORS @@ -216,6 +216,7 @@ Patches have also been contributed by: Yuri Chornoivan Deepak C Shetty Martin Kletzander + Laszlo Ersek [....send patches to get your name here....] diff --git a/src/util/command.c b/src/util/command.c index f05493edfa..dc3cfc5430 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; - } - } } } -- 2.47.2