Re: [PATCH 01/20] libxl: handle POLLERR, POLLHUP, POLLNVAL properly

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

On Fri, 2012-04-13 at 19:39 +0100, Ian Jackson wrote:
> Pass POLLERR and POLLHUP to fd callbacks, as is necessary.
> Crash on POLLNVAL since that means our fds are messed up.
> 
> Document the behaviour (including the fact that poll sometimes sets
> POLLHUP or POLLERR even if only POLLIN was requested.
> 
> Fix the one current fd callback to do something with POLLERR|POLLHUP.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

> ---
>  tools/libxl/libxl_event.c    |    7 ++++++-
>  tools/libxl/libxl_internal.h |    5 +++++
>  2 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
> index 638b9ab..5e1a207 100644
> --- a/tools/libxl/libxl_event.c
> +++ b/tools/libxl/libxl_event.c
> @@ -335,6 +335,9 @@ static void watchfd_callback(libxl__egc *egc, libxl__ev_fd *ev,
>  {
>      EGC_GC;
>  
> +    if (revents & (POLLERR|POLLHUP))
> +        LIBXL__EVENT_DISASTER(egc, "unexpected poll event on watch fd", 0, 0);
> +
>      for (;;) {
>          char **event = xs_check_watch(CTX->xsh);
>          if (!event) {
> @@ -739,7 +742,9 @@ static int afterpoll_check_fd(libxl__poller *poller,
>          /* again, stale slot entry */
>          return 0;
>  
> -    int revents = fds[slot].revents & events;
> +    assert(!(fds[slot].revents & POLLNVAL));
> +
> +    int revents = fds[slot].revents & (events | POLLERR | POLLHUP);
>      /* we mask in case requested events have changed */
>  
>      return revents;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index a4b933b..af631fb 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -127,6 +127,11 @@ void libxl__alloc_failed(libxl_ctx *, const char *func,
>  typedef struct libxl__ev_fd libxl__ev_fd;
>  typedef void libxl__ev_fd_callback(libxl__egc *egc, libxl__ev_fd *ev,
>                                     int fd, short events, short revents);
> +  /* Note that revents may contain POLLERR or POLLHUP regardless of
> +   * events; otherwise revents contains only bits in events.  Contrary
> +   * to the documentation for poll(2), POLLERR and POLLHUP can occur
> +   * even if only POLLIN was set in events.  (POLLNVAL is a fatal
> +   * error and will cause libxl event machinery to fail an assertion.) */
>  struct libxl__ev_fd {
>      /* caller should include this in their own struct */
>      /* read-only for caller, who may read only when registered: */