Re: [Xen-devel] [PATCH 6/6] tools/libxl: Fix datacopier POLLHUP handling to not always be fatal

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/03/15 18:10, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH 6/6] tools/libxl: Fix datacopier POLLHUP 
> handling to not always be fatal"):
>> On Fri, 2015-02-20 at 13:55 +0000, Andrew Cooper wrote:
>>> A different and far less invasive approach might be to have a per-fd
>>> revent ignore mask.  This would at least allow the callbacks to be made
>>> when the datacopier is in a consistent state.
>> Ian, any thoughts on this?
> I think there is a real bug but the patch is misconceived :-/.
>
> Andrew says on IRC:
>
> 18:02 <andyhhp> Diziet: POLLHUP|POLLIN is a valid revent to recieve
> 18:03 <andyhhp> which happens when the writer has written into the
>                 pipe and closed the fd
>
> This is true [1] and will indeed cause the datacopier to quit before
> it has finished copying all the data, which I think is indeed a bug.
>
> On the other hand, some fd objects can return POLLHUP without other
> indications.  In those cases we should fail.
>
> I think the right answer is for the dc to ignore POLLHUP iff there are
> other bits set which are going to be handled.
>
> Care needs to be taken about how this interacts with
>   7253e0fd1aeb3ae7d4714bcc1d86b846b3331995
>   libxl: react correctly to bootloader pty master POLLHUP
>
> Ian.
>
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html

I don't think anything good can come from ignoring a POLLHUP on
write_fd, as a subsequent call to write() is either going to hit a hard
error, or block.  (Indeed, [1] indicates that POLLHUP and POLLOUT are
mutually exclusive.)

On the read side, transforming POLLHUP|POLLIN to POLLIN should be safe,
as read() will continue to work until EOF.

Given [1], this should be safe even interacting with 7253e0fd1 as
POLLHUP should be set for every subsequent poll(), even once EOF has
been reached.

~Andrew


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel