All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>, Xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH 6/6] tools/libxl: Fix datacopier POLLHUP handling to not always be fatal
Date: Tue, 3 Mar 2015 18:38:00 +0000	[thread overview]
Message-ID: <54F5FF88.5090904@citrix.com> (raw)
In-Reply-To: <21749.63791.360850.293883@mariner.uk.xensource.com>

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

      reply	other threads:[~2015-03-03 18:38 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-18 16:34 [PATCH 0/6] tools/libxl: Improvements to libxl__datacopier Andrew Cooper
2015-02-18 16:34 ` [PATCH 1/6] tools/libxl: Introduce min and max macros Andrew Cooper
2015-02-20 10:24   ` Ian Campbell
2015-02-20 10:42   ` Frediano Ziglio
2015-02-20 11:08     ` Andrew Cooper
2015-02-18 16:34 ` [PATCH 2/6] tools/libxl: Update datacopier to support sending data only Andrew Cooper
2015-02-20 10:27   ` Ian Campbell
2015-02-20 11:10     ` Andrew Cooper
2015-02-20 11:13       ` Ian Campbell
2015-02-18 16:34 ` [PATCH 3/6] tools/libxl: Allow adding larger amounts of prefixdata to datacopier Andrew Cooper
2015-02-20 10:32   ` Ian Campbell
2015-02-18 16:34 ` [PATCH 4/6] tools/libxl: Allow limiting amount copied by datacopier Andrew Cooper
2015-02-20 10:33   ` Ian Campbell
2015-02-18 16:34 ` [PATCH 5/6] tools/libxl: Extend datacopier to support reading into a buffer Andrew Cooper
2015-02-20 10:34   ` Ian Campbell
2015-02-18 16:34 ` [PATCH 6/6] tools/libxl: Fix datacopier POLLHUP handling to not always be fatal Andrew Cooper
2015-02-20 10:43   ` Ian Campbell
2015-02-20 13:55     ` Andrew Cooper
2015-02-20 14:05       ` Ian Campbell
2015-03-03 18:10         ` Ian Jackson
2015-03-03 18:38           ` Andrew Cooper [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54F5FF88.5090904@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.