[Libguestfs] [libnbd PATCH 4/8] states: Prepare for read callback

[Richard W.M. Jones]

On Mon, Jun 17, 2019 at 07:07:54PM -0500, Eric Blake wrote:
> The next patch will add a new 'nbd_aio_pread_callback' function for
> allowing the user more immediate access as each chunk of a structured
> reply read is received. But before we do that, let's refactor the
> command code. This includes a revert of 12843a1a, since the read
> callback will need both a buffer and the user's opaque object at the
> same time.

This patch on its own is fine, although of course if you want to push
it upstream now you'll need to adjust the commit message.

ACK

Rich.

>  generator/states-reply-structured.c |  8 ++++----
>  lib/internal.h                      | 17 +++++++++++++----
>  lib/rw.c                            |  9 ++++++---
>  3 files changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c
> index 6740400..657106e 100644
> --- a/generator/states-reply-structured.c
> +++ b/generator/states-reply-structured.c
> @@ -123,7 +123,7 @@
>        set_error (0, "invalid length in NBD_REPLY_TYPE_BLOCK_STATUS");
>        return -1;
>      }
> -    if (cmd->extent_fn == NULL) {
> +    if (cmd->cb.fn.extent == NULL) {
>        SET_NEXT_STATE (%.DEAD);
>        set_error (0, "not expecting NBD_REPLY_TYPE_BLOCK_STATUS here");
>        return -1;
> @@ -375,7 +375,7 @@
>      length = be32toh (h->sbuf.sr.structured_reply.length);
> 
>      assert (cmd); /* guaranteed by CHECK */
> -    assert (cmd->extent_fn);
> +    assert (cmd->cb.fn.extent);
>      assert (h->bs_entries);
>      assert (length >= 12);
> 
> @@ -401,8 +401,8 @@
>        if (meta_context) {
>          /* Call the caller's extent function. */
>          errno = 0;
> -        if (cmd->extent_fn (cmd->data, meta_context->name, cmd->offset,
> -                            &h->bs_entries[1], (length-4) / 4) == -1)
> +        if (cmd->cb.fn.extent (cmd->cb.opaque, meta_context->name, cmd->offset,
> +                               &h->bs_entries[1], (length-4) / 4) == -1)
>            cmd->error = errno ? errno : EPROTO;
>        }
>        else
> diff --git a/lib/internal.h b/lib/internal.h
> index 1f8f789..cb0e170 100644
> --- a/lib/internal.h
> +++ b/lib/internal.h
> @@ -231,7 +231,16 @@ struct socket {
>    const struct socket_ops *ops;
>  };
> 
> -typedef int (*extent_fn) (void *data, const char *metacontext, uint64_t offset, uint32_t *entries, size_t nr_entries);
> +typedef int (*extent_fn) (void *data, const char *metacontext, uint64_t offset,
> +                          uint32_t *entries, size_t nr_entries);
> +
> +struct command_cb {
> +  void *opaque;
> +  union {
> +    extent_fn extent;
> +    /* More to come */
> +  } fn;
> +};
> 
>  struct command_in_flight {
>    struct command_in_flight *next;
> @@ -240,8 +249,8 @@ struct command_in_flight {
>    uint64_t handle;
>    uint64_t offset;
>    uint32_t count;
> -  void *data; /* Buffer for read/write, opaque for block status */
> -  extent_fn extent_fn;
> +  void *data; /* Buffer for read/write */
> +  struct command_cb cb;
>    bool data_seen; /* For read, true if at least one data chunk seen */
>    uint32_t error; /* Local errno value */
>  };
> @@ -300,7 +309,7 @@ extern const char *nbd_internal_name_of_nbd_cmd (uint16_t type);
>  extern int64_t nbd_internal_command_common (struct nbd_handle *h,
>                                              uint16_t flags, uint16_t type,
>                                              uint64_t offset, uint64_t count,
> -                                            void *data, extent_fn extent);
> +                                            void *data, struct command_cb *cb);
> 
>  /* socket.c */
>  struct socket *nbd_internal_socket_create (int fd);
> diff --git a/lib/rw.c b/lib/rw.c
> index ad9c8a0..dc81c57 100644
> --- a/lib/rw.c
> +++ b/lib/rw.c
> @@ -143,7 +143,7 @@ int64_t
>  nbd_internal_command_common (struct nbd_handle *h,
>                               uint16_t flags, uint16_t type,
>                               uint64_t offset, uint64_t count, void *data,
> -                             extent_fn extent)
> +                             struct command_cb *cb)
>  {
>    struct command_in_flight *cmd, *prev_cmd;
> 
> @@ -183,7 +183,8 @@ nbd_internal_command_common (struct nbd_handle *h,
>    cmd->offset = offset;
>    cmd->count = count;
>    cmd->data = data;
> -  cmd->extent_fn = extent;
> +  if (cb)
> +    cmd->cb = *cb;
> 
>    /* If structured replies were negotiated then we trust the server to
>     * send back sufficient data to cover the whole buffer.  It's tricky
> @@ -360,6 +361,8 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
>                                 void *data, extent_fn extent,
>                                 uint32_t flags)
>  {
> +  struct command_cb cb = { .opaque = data, .fn.extent = extent, };
> +
>    if (!h->structured_replies) {
>      set_error (ENOTSUP, "server does not support structured replies");
>      return -1;
> @@ -383,5 +386,5 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
>    }
> 
>    return nbd_internal_command_common (h, flags, NBD_CMD_BLOCK_STATUS, offset,
> -                                      count, data, extent);
> +                                      count, NULL, &cb);
>  }
> -- 
> 2.20.1
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top