Re: [PATCH 1/3] Revert "tools/libxl: Adjust datacopiers POLLHUP handling when the fd is also readable"

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-04-02 at 16:04 +0100, Ian Jackson wrote:
> The bootloader code is relying on detecting POLLHUP, and 7e9ec50b
> breaks that.  7e9ec50b, when handling a pty master, violates the
> specification of the datacopier interface (as defined).
> 
> When the bootloader exits, several things change, all at once:
>  (a) The master pty fd (held by libxl) starts to signal POLLHUP
>     and maybe also POLLIN.
>  (b) The child exits (so that the SIGCHLD self-pipe signals POLLIN,
>     which will be handled by the libxl child process code.
>  (c) reads on the master pty fd start to return EOF
> 
> From the point of view of the datacopier these might happen in any
> order.  I think there is a latent bug with (c), which I will discuss
> later in this email.

later in another mail maybe?

> In a recent bug report from a FreeBSD installation, the datacopier
> gets told about (a) before (b).  But 7e9ec50b filters the POLLHUP out,
> so that the dc signals eof rather than hup.  As a result in
> bootloader_copyfail we take the error path.
> 
> This reverts commit 7e9ec50b0535bf2630da9d279a060775817d136d.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_aoutils.c |    3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index da102a0..3942634 100644
> --- a/tools/libxl/libxl_aoutils.c
> +++ b/tools/libxl/libxl_aoutils.c
> @@ -205,9 +205,6 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
>      libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
>      STATE_AO_GC(dc->ao);
>  
> -    if ((revents & (POLLHUP|POLLIN)) == (POLLHUP|POLLIN))
> -        revents &= ~POLLHUP;
> -
>      if (datacopier_pollhup_handled(egc, dc, revents, 0))
>          return;
>  



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