Re: [PATCH 5/6] tools/libxl: Extend datacopier to support reading into a buffer

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

On Wed, 2015-02-18 at 16:34 +0000, Andrew Cooper wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> Implement a read-only mode for libxl__datacopier.  The mode is invoked
> when readbuf is set and writefd is < 0.  On success, the callback passes
> the number of bytes read.

http://lists.xen.org/archives/html/xen-devel/2014-09/msg01803.html

> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_aoutils.c  |   58 +++++++++++++++++++++++++-----------------
>  tools/libxl/libxl_internal.h |    4 ++-
>  2 files changed, 38 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index 037244e..a402e5c 100644
> --- a/tools/libxl/libxl_aoutils.c
> +++ b/tools/libxl/libxl_aoutils.c
> @@ -134,7 +134,7 @@ static void datacopier_check_state(libxl__egc *egc, libxl__datacopier_state *dc)
>      STATE_AO_GC(dc->ao);
>      int rc;
>      
> -    if (dc->used) {
> +    if (dc->used && !dc->readbuf) {
>          if (!libxl__ev_fd_isregistered(&dc->towrite)) {
>              rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable,
>                                         dc->writefd, POLLOUT);
> @@ -147,7 +147,7 @@ static void datacopier_check_state(libxl__egc *egc, libxl__datacopier_state *dc)
>          }
>      } else if (!libxl__ev_fd_isregistered(&dc->toread) || dc->maxread == 0) {
>          /* we have had eof */
> -        datacopier_callback(egc, dc, 0, 0);
> +        datacopier_callback(egc, dc, 0, dc->readbuf ? dc->used : 0);
>          return;
>      } else {
>          /* nothing buffered, but still reading */
> @@ -215,24 +215,30 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
>      }
>      assert(revents & POLLIN);
>      for (;;) {
> -        while (dc->used >= dc->maxsz) {
> -            libxl__datacopier_buf *rm = LIBXL_TAILQ_FIRST(&dc->bufs);
> -            dc->used -= rm->used;
> -            assert(dc->used >= 0);
> -            LIBXL_TAILQ_REMOVE(&dc->bufs, rm, entry);
> -            free(rm);
> -        }
> +        libxl__datacopier_buf *buf = NULL;
> +        int r;
> +
> +        if (dc->readbuf) {
> +            r = read(ev->fd, dc->readbuf + dc->used, dc->maxread);
> +        } else {
> +            while (dc->used >= dc->maxsz) {
> +                libxl__datacopier_buf *rm = LIBXL_TAILQ_FIRST(&dc->bufs);
> +                dc->used -= rm->used;
> +                assert(dc->used >= 0);
> +                LIBXL_TAILQ_REMOVE(&dc->bufs, rm, entry);
> +                free(rm);
> +            }
>  
> -        libxl__datacopier_buf *buf =
> -            LIBXL_TAILQ_LAST(&dc->bufs, libxl__datacopier_bufs);
> -        if (!buf || buf->used >= sizeof(buf->buf)) {
> -            buf = malloc(sizeof(*buf));
> -            if (!buf) libxl__alloc_failed(CTX, __func__, 1, sizeof(*buf));
> -            buf->used = 0;
> -            LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry);
> -        }
> -        int r = read(ev->fd, buf->buf + buf->used,
> +            buf = LIBXL_TAILQ_LAST(&dc->bufs, libxl__datacopier_bufs);
> +            if (!buf || buf->used >= sizeof(buf->buf)) {
> +                buf = malloc(sizeof(*buf));
> +                if (!buf) libxl__alloc_failed(CTX, __func__, 1, sizeof(*buf));
> +                buf->used = 0;
> +                LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry);
> +            }
> +            r = read(ev->fd, buf->buf + buf->used,
>                       min_t(size_t, sizeof(buf->buf) - buf->used, dc->maxread));
> +        }
>          if (r < 0) {
>              if (errno == EINTR) continue;
>              if (errno == EWOULDBLOCK) break;
> @@ -255,10 +261,12 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
>                  return;
>              }
>          }
> -        buf->used += r;
> +        if (!dc->readbuf) {
> +            buf->used += r;
> +            assert(buf->used <= sizeof(buf->buf));
> +        }
>          dc->used += r;
>          dc->maxread -= r;
> -        assert(buf->used <= sizeof(buf->buf));
>          assert(dc->maxread >= 0);
>          if (dc->maxread == 0)
>              break;
> @@ -316,15 +324,19 @@ int libxl__datacopier_start(libxl__datacopier_state *dc)
>  
>      libxl__datacopier_init(dc);
>  
> +    assert(dc->readfd >= 0 || dc->writefd >= 0);
> +
>      if (dc->readfd >= 0) {
>          rc = libxl__ev_fd_register(gc, &dc->toread, datacopier_readable,
>                                     dc->readfd, POLLIN);
>          if (rc) goto out;
>      }
>  
> -    rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable,
> -                               dc->writefd, POLLOUT);
> -    if (rc) goto out;
> +    if (dc->writefd >= 0) {
> +        rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable,
> +                                   dc->writefd, POLLOUT);
> +        if (rc) goto out;
> +    }
>  
>      return 0;
>  
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index b7fd13d..c2e7aa4 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2524,7 +2524,8 @@ _hidden void libxl__device_disk_local_initiate_detach(libxl__egc *egc,
>  
>  /* onwrite==1 means failure happened when writing, logged, errnoval is valid
>   * onwrite==0 means failure happened when reading
> - *     errnoval==0 means we got eof and all data was written
> + *     errnoval>=0 means we got eof and all data was written or number of bytes
> + *                 written when in read mode
>   *     errnoval!=0 means we had a read error, logged
>   * onwrite==-1 means some other internal failure, errnoval not valid, logged
>   * If we get POLLHUP, we call callback_pollhup(..., onwrite, -1);
> @@ -2553,6 +2554,7 @@ struct libxl__datacopier_state {
>      /* remaining fields are private to datacopier */
>      libxl__ev_fd toread, towrite;
>      ssize_t used;
> +    void *readbuf;
>      LIBXL_TAILQ_HEAD(libxl__datacopier_bufs, libxl__datacopier_buf) bufs;
>  };
>