Laszlo Ersek
2023-May-30 11:18 UTC
[Libguestfs] [libnbd PATCH] internal: s/handle/cookie/ to match NBD spec
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 <eblake at redhat.com> > --- > 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 cookie) is stored at the same offset. > */ > - 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 <lersek at redhat.com>
Eric Blake
2023-May-30 17:44 UTC
[Libguestfs] [libnbd PATCH] internal: s/handle/cookie/ to match NBD spec
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 <lersek at redhat.com>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