On 01/03/2012 10:58 AM, Michal Privoznik wrote:
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.
Looks like this also solves: https://bugzilla.redhat.com/show_bug.cgi?id=770788
--- src/util/command.c | 21 +++++++++++++++++---- 1 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c index f5effdf..9b553f0 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;
Oh my - you're right - we stack-allocated struct pollfd fds[3]; without initialization earlier on.
@@ -1708,8 +1711,18 @@ virCommandProcessIO(virCommandPtr cmd) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); } } + } else if (fds[i].revents & POLLHUP) { + 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; + }
Should we also be probing for POLLERR? (POLLNVAL shouldn't ever be present, because it would indicate a bigger bug in our code). ACK - what you have is a strict improvement, even if we further decide to check for POLLERR. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org