Re: [Libguestfs] [libnbd PATCH] internal: s/handle/cookie/ to match NBD spec

2023-05-31 Thread Richard W.M. Jones
On Tue, May 30, 2023 at 12:44:41PM -0500, Eric Blake wrote:
> On Tue, May 30, 2023 at 01:18:55PM +0200, Laszlo Ersek wrote:
> > On 5/29/23 18:24, Eric Blake wrote:
> > > Externally, we have been exposing the 64-bit opaque marker for each
> > > NBD packet as the "cookie", because it was less confusing when
> > > contrasted with our 'struct nbd_handle *' holding all libnbd state.
> > > It also avoids confusion between the noun 'handle' as a way to
> > > identify a packet and the verb 'handle' for reacting to things like
> > > signals.  Upstream NBD changed their spec to favor the name "cookie"
> > > based on our recommendations[1], and so now we can get rid of our last
> > > uses of the old name.
> > > 
> > > [1] https://github.com/NetworkBlockDevice/nbd/commit/ca4392eb2b
> > > 
> 
> > > */
> > > -  cookie = be64toh (h->sbuf.simple_reply.handle);
> > > +  cookie = be64toh (h->sbuf.simple_reply.cookie);
> > >/* Find the command amongst the commands in flight. */
> > >for (cmd = h->cmds_in_flight, prev_cmd = NULL;
> > > cmd != NULL;
> > 
> > 
> > (I didn't dig into the larger contexts.)
> > 
> > Reviewed-by: Laszlo Ersek 
> 
> Both commits now in; libnbd as c1df4df9 and nbdkit as 5637302c

Thanks for this, also reviewed by me.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [libnbd PATCH] internal: s/handle/cookie/ to match NBD spec

2023-05-30 Thread Eric Blake
On Tue, May 30, 2023 at 01:18:55PM +0200, Laszlo Ersek wrote:
> On 5/29/23 18:24, Eric Blake wrote:
> > Externally, we have been exposing the 64-bit opaque marker for each
> > NBD packet as the "cookie", because it was less confusing when
> > contrasted with our 'struct nbd_handle *' holding all libnbd state.
> > It also avoids confusion between the noun 'handle' as a way to
> > identify a packet and the verb 'handle' for reacting to things like
> > signals.  Upstream NBD changed their spec to favor the name "cookie"
> > based on our recommendations[1], and so now we can get rid of our last
> > uses of the old name.
> > 
> > [1] https://github.com/NetworkBlockDevice/nbd/commit/ca4392eb2b
> > 

> > */
> > -  cookie = be64toh (h->sbuf.simple_reply.handle);
> > +  cookie = be64toh (h->sbuf.simple_reply.cookie);
> >/* Find the command amongst the commands in flight. */
> >for (cmd = h->cmds_in_flight, prev_cmd = NULL;
> > cmd != NULL;
> 
> 
> (I didn't dig into the larger contexts.)
> 
> Reviewed-by: Laszlo Ersek 

Both commits now in; libnbd as c1df4df9 and nbdkit as 5637302c

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [libnbd PATCH] internal: s/handle/cookie/ to match NBD spec

2023-05-30 Thread Laszlo Ersek
On 5/29/23 18:24, Eric Blake wrote:
> Externally, we have been exposing the 64-bit opaque marker for each
> NBD packet as the "cookie", because it was less confusing when
> contrasted with our 'struct nbd_handle *' holding all libnbd state.
> It also avoids confusion between the noun 'handle' as a way to
> identify a packet and the verb 'handle' for reacting to things like
> signals.  Upstream NBD changed their spec to favor the name "cookie"
> based on our recommendations[1], and so now we can get rid of our last
> uses of the old name.
> 
> [1] https://github.com/NetworkBlockDevice/nbd/commit/ca4392eb2b
> 
> Signed-off-by: Eric Blake 
> ---
>  lib/nbd-protocol.h   | 6 +++---
>  generator/states-issue-command.c | 6 +++---
>  generator/states-reply-simple.c  | 2 +-
>  generator/states-reply.c | 4 ++--
>  4 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/nbd-protocol.h b/lib/nbd-protocol.h
> index 0217891e..50275dcd 100644
> --- a/lib/nbd-protocol.h
> +++ b/lib/nbd-protocol.h
> @@ -193,7 +193,7 @@ struct nbd_request {
>uint32_t magic;   /* NBD_REQUEST_MAGIC. */
>uint16_t flags;   /* Request flags. */
>uint16_t type;/* Request type. */
> -  uint64_t handle;  /* Opaque handle. */
> +  uint64_t cookie;  /* Opaque handle. */
>uint64_t offset;  /* Request offset. */
>uint32_t count;   /* Request length. */
>  } NBD_ATTRIBUTE_PACKED;
> @@ -202,7 +202,7 @@ struct nbd_request {
>  struct nbd_simple_reply {
>uint32_t magic;   /* NBD_SIMPLE_REPLY_MAGIC. */
>uint32_t error;   /* NBD_SUCCESS or one of NBD_E*. */
> -  uint64_t handle;  /* Opaque handle. */
> +  uint64_t cookie;  /* Opaque handle. */
>  } NBD_ATTRIBUTE_PACKED;
> 
>  /* Structured reply (server -> client). */
> @@ -210,7 +210,7 @@ struct nbd_structured_reply {
>uint32_t magic;   /* NBD_STRUCTURED_REPLY_MAGIC. */
>uint16_t flags;   /* NBD_REPLY_FLAG_* */
>uint16_t type;/* NBD_REPLY_TYPE_* */
> -  uint64_t handle;  /* Opaque handle. */
> +  uint64_t cookie;  /* Opaque handle. */
>uint32_t length;  /* Length of payload which follows. */
>  } NBD_ATTRIBUTE_PACKED;
> 
> diff --git a/generator/states-issue-command.c 
> b/generator/states-issue-command.c
> index 111e131c..30721946 100644
> --- a/generator/states-issue-command.c
> +++ b/generator/states-issue-command.c
> @@ -44,7 +44,7 @@  ISSUE_COMMAND.START:
>h->request.magic = htobe32 (NBD_REQUEST_MAGIC);
>h->request.flags = htobe16 (cmd->flags);
>h->request.type = htobe16 (cmd->type);
> -  h->request.handle = htobe64 (cmd->cookie);
> +  h->request.cookie = htobe64 (cmd->cookie);
>h->request.offset = htobe64 (cmd->offset);
>h->request.count = htobe32 (cmd->count);
>h->chunks_sent++;
> @@ -74,7 +74,7 @@  ISSUE_COMMAND.PREPARE_WRITE_PAYLOAD:
> 
>assert (h->cmds_to_issue != NULL);
>cmd = h->cmds_to_issue;
> -  assert (cmd->cookie == be64toh (h->request.handle));
> +  assert (cmd->cookie == be64toh (h->request.cookie));
>if (cmd->type == NBD_CMD_WRITE) {
>  h->wbuf = cmd->data;
>  h->wlen = cmd->count;
> @@ -120,7 +120,7 @@  ISSUE_COMMAND.FINISH:
>assert (!h->wlen);
>assert (h->cmds_to_issue != NULL);
>cmd = h->cmds_to_issue;
> -  assert (cmd->cookie == be64toh (h->request.handle));
> +  assert (cmd->cookie == be64toh (h->request.cookie));
>h->cmds_to_issue = cmd->next;
>if (h->cmds_to_issue_tail == cmd)
>  h->cmds_to_issue_tail = NULL;
> diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c
> index 8fd9f62a..19be5418 100644
> --- a/generator/states-reply-simple.c
> +++ b/generator/states-reply-simple.c
> @@ -39,7 +39,7 @@  REPLY.SIMPLE_REPLY.START:
>  if (error || h->structured_replies)
>SET_NEXT_STATE (%^FINISH_COMMAND);
>  else {
> -  uint64_t cookie = be64toh (h->sbuf.simple_reply.handle);
> +  uint64_t cookie = be64toh (h->sbuf.simple_reply.cookie);
>SET_NEXT_STATE (%.DEAD);
>set_error (EPROTO,
>   "no matching cookie %" PRIu64 " found for server reply, "
> diff --git a/generator/states-reply.c b/generator/states-reply.c
> index f7888154..511e5cb1 100644
> --- a/generator/states-reply.c
> +++ b/generator/states-reply.c
> @@ -138,7 +138,7 @@  REPLY.CHECK_SIMPLE_OR_STRUCTURED_REPLY:
> * handle (our cookie) is stored at the same offset.
> */
>h->chunks_received++;
> -  cookie = be64toh (h->sbuf.simple_reply.handle);
> +  cookie = be64toh (h->sbuf.simple_reply.cookie);
>/* Find the command amongst the commands in flight. If the server sends
> * a reply for an unknown cookie, FINISH will diagnose that later.
> */
> @@ -157,7 +157,7 @@  REPLY.FINISH_COMMAND:
>/* NB: This works for both simple and structured replies because the
> * handle (our