Re: [Xen-devel] [PATCH 23/29] tools/libxl: Extend datacopier to support reading into a buffer

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-09-10 at 18:11 +0100, Andrew Cooper wrote:
> From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> 
> Implement a read-only mode for libxl__datacopier. 

Read-only in the sense that it only reads, or in the sense that one may
only read from it?

What is the purpose of this mode?

Where does the data go?

>  The mode is invoked
> when readbuf is set and writefd is < 0.  On success, the callback passes
> the number of bytes read.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_aoutils.c  |   59 
> +++++++++++++++++++++++++-----------------
>  tools/libxl/libxl_internal.h |    4 ++-
>  2 files changed, 38 insertions(+), 25 deletions(-)
> 
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index 6502325..9183716 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,26 +215,31 @@ 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,
>                       (sizeof(buf->buf) - buf->used) < dc->maxread ?
>                           (sizeof(buf->buf) - buf->used) : dc->maxread);
> +        }
>          if (r < 0) {
>              if (errno == EINTR) continue;
>              if (errno == EWOULDBLOCK) break;
> @@ -257,10 +262,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;
> @@ -324,9 +331,13 @@ int libxl__datacopier_start(libxl__datacopier_state *dc)
>          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;
> +    }
> +
> +    assert(dc->readfd >= 0 || dc->writefd >= 0);
>  
>      return 0;
>  
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index d93a6ee..056843a 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2404,7 +2404,8 @@ typedef struct libxl__datacopier_buf 
> libxl__datacopier_buf;
>  
>  /* 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);
> @@ -2433,6 +2434,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;
>  };
>  



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