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

[Eric Blake]

On 01/24/2012 07:55 AM, Laszlo Ersek wrote:
> 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.

And I did some runtime testing.

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

Yep, it's list policy to reply-all, anyway.  This sort of meta-comment
fits better after the --- line, so that it doesn't get stored in the git
commit log.

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

ACK and pushed, with commit message amended, and adding you to AUTHORS.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120124/c6de1e0d/attachment-0001.sig>