154 lines
5.4 KiB
Diff
154 lines
5.4 KiB
Diff
commit 06b9c5b9231ef4dbd4b5ff69564305cd4f814879
|
|
Author: Michal Privoznik <mprivozn@redhat.com>
|
|
Date: Tue Jan 3 18:40:55 2012 +0100
|
|
|
|
virCommand: Properly handle POLLHUP
|
|
|
|
It is a good practise to set revents to zero before doing any poll().
|
|
Moreover, we should check if event we waited for really occurred or
|
|
if any of fds we were polling on didn't encountered hangup.
|
|
|
|
diff --git a/src/util/command.c b/src/util/command.c
|
|
index f5effdf..bdaa88b 100644
|
|
--- a/src/util/command.c
|
|
+++ b/src/util/command.c
|
|
@@ -1620,16 +1620,19 @@ virCommandProcessIO(virCommandPtr cmd)
|
|
if (infd != -1) {
|
|
fds[nfds].fd = infd;
|
|
fds[nfds].events = POLLOUT;
|
|
+ fds[nfds].revents = 0;
|
|
nfds++;
|
|
}
|
|
if (outfd != -1) {
|
|
fds[nfds].fd = outfd;
|
|
fds[nfds].events = POLLIN;
|
|
+ fds[nfds].revents = 0;
|
|
nfds++;
|
|
}
|
|
if (errfd != -1) {
|
|
fds[nfds].fd = errfd;
|
|
fds[nfds].events = POLLIN;
|
|
+ fds[nfds].revents = 0;
|
|
nfds++;
|
|
}
|
|
|
|
@@ -1645,8 +1648,8 @@ virCommandProcessIO(virCommandPtr cmd)
|
|
}
|
|
|
|
for (i = 0; i < nfds ; i++) {
|
|
- if (fds[i].fd == errfd ||
|
|
- fds[i].fd == outfd) {
|
|
+ if (fds[i].revents & POLLIN &&
|
|
+ (fds[i].fd == errfd || fds[i].fd == outfd)) {
|
|
char data[1024];
|
|
char **buf;
|
|
size_t *len;
|
|
@@ -1684,7 +1687,10 @@ virCommandProcessIO(virCommandPtr cmd)
|
|
memcpy(*buf + *len, data, done);
|
|
*len += done;
|
|
}
|
|
- } else {
|
|
+ }
|
|
+
|
|
+ if (fds[i].revents & POLLOUT &&
|
|
+ fds[i].fd == infd) {
|
|
int done;
|
|
|
|
/* Coverity 5.3.0 can't see that we only get here if
|
|
@@ -1710,6 +1716,18 @@ 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;
|
|
+ }
|
|
+ }
|
|
}
|
|
}
|
|
|
|
commit d19149dda888d36cea58b6cdf7446f98bd1bf734
|
|
Author: Laszlo Ersek <lersek@redhat.com>
|
|
Date: Tue Jan 24 15:55:19 2012 +0100
|
|
|
|
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 <lersek@redhat.com>
|
|
|
|
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;
|
|
- }
|
|
- }
|
|
}
|
|
}
|
|
|