Eric Blake
2023-May-29 16:24 UTC
[Libguestfs] [libnbd PATCH] internal: s/handle/cookie/ to match NBD spec
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;
--
2.40.1
Eric Blake
2023-May-29 16:24 UTC
[Libguestfs] [nbdkit PATCH] server: s/handle/cookie/ to match NBD spec
Externally, libnbd has been exposing the 64-bit opaque marker for each
NBD packet as the "cookie", because it was less confusing when
contrasted with '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, keeping nbd-protocol.h in sync with libnbd.
[1] https://github.com/NetworkBlockDevice/nbd/commit/ca4392eb2b
Signed-off-by: Eric Blake <eblake at redhat.com>
---
common/protocol/nbd-protocol.h | 6 +++---
server/protocol.c | 24 ++++++++++++------------
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/common/protocol/nbd-protocol.h b/common/protocol/nbd-protocol.h
index 0217891e..50275dcd 100644
--- a/common/protocol/nbd-protocol.h
+++ b/common/protocol/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/server/protocol.c b/server/protocol.c
index d9a5e282..cb530e65 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -363,7 +363,7 @@ nbd_errno (int error, uint16_t flags)
}
static bool
-send_simple_reply (uint64_t handle, uint16_t cmd, uint16_t flags,
+send_simple_reply (uint64_t cookie, uint16_t cmd, uint16_t flags,
const char *buf, uint32_t count,
uint32_t error)
{
@@ -374,7 +374,7 @@ send_simple_reply (uint64_t handle, uint16_t cmd, uint16_t
flags,
int f = (cmd == NBD_CMD_READ && !error) ? SEND_MORE : 0;
reply.magic = htobe32 (NBD_SIMPLE_REPLY_MAGIC);
- reply.handle = handle;
+ reply.cookie = cookie;
reply.error = htobe32 (nbd_errno (error, flags));
r = conn->send (&reply, sizeof reply, f);
@@ -395,7 +395,7 @@ send_simple_reply (uint64_t handle, uint16_t cmd, uint16_t
flags,
}
static bool
-send_structured_reply_read (uint64_t handle, uint16_t cmd,
+send_structured_reply_read (uint64_t cookie, uint16_t cmd,
const char *buf, uint32_t count, uint64_t offset)
{
GET_CONN;
@@ -412,7 +412,7 @@ send_structured_reply_read (uint64_t handle, uint16_t cmd,
assert (cmd == NBD_CMD_READ);
reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC);
- reply.handle = handle;
+ reply.cookie = cookie;
reply.flags = htobe16 (NBD_REPLY_FLAG_DONE);
reply.type = htobe16 (NBD_REPLY_TYPE_OFFSET_DATA);
reply.length = htobe32 (count + sizeof offset_data);
@@ -522,7 +522,7 @@ extents_to_block_descriptors (struct nbdkit_extents
*extents,
}
static bool
-send_structured_reply_block_status (uint64_t handle,
+send_structured_reply_block_status (uint64_t cookie,
uint16_t cmd, uint16_t flags,
uint32_t count, uint64_t offset,
struct nbdkit_extents *extents)
@@ -545,7 +545,7 @@ send_structured_reply_block_status (uint64_t handle,
return connection_set_status (STATUS_DEAD);
reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC);
- reply.handle = handle;
+ reply.cookie = cookie;
reply.flags = htobe16 (NBD_REPLY_FLAG_DONE);
reply.type = htobe16 (NBD_REPLY_TYPE_BLOCK_STATUS);
reply.length = htobe32 (sizeof context_id +
@@ -578,7 +578,7 @@ send_structured_reply_block_status (uint64_t handle,
}
static bool
-send_structured_reply_error (uint64_t handle, uint16_t cmd, uint16_t flags,
+send_structured_reply_error (uint64_t cookie, uint16_t cmd, uint16_t flags,
uint32_t error)
{
GET_CONN;
@@ -588,7 +588,7 @@ send_structured_reply_error (uint64_t handle, uint16_t cmd,
uint16_t flags,
int r;
reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC);
- reply.handle = handle;
+ reply.cookie = cookie;
reply.flags = htobe16 (NBD_REPLY_FLAG_DONE);
reply.type = htobe16 (NBD_REPLY_TYPE_ERROR);
reply.length = htobe32 (0 /* no human readable error */ + sizeof error_data);
@@ -740,16 +740,16 @@ protocol_recv_request_send_reply (void)
*/
if (!conn->structured_replies ||
(cmd != NBD_CMD_READ && cmd != NBD_CMD_BLOCK_STATUS))
- return send_simple_reply (request.handle, cmd, flags, buf, count, error);
+ return send_simple_reply (request.cookie, cmd, flags, buf, count, error);
if (error)
- return send_structured_reply_error (request.handle, cmd, flags, error);
+ return send_structured_reply_error (request.cookie, cmd, flags, error);
if (cmd == NBD_CMD_READ)
- return send_structured_reply_read (request.handle, cmd, buf, count,
+ return send_structured_reply_read (request.cookie, cmd, buf, count,
offset);
/* NBD_CMD_BLOCK_STATUS */
- return send_structured_reply_block_status (request.handle, cmd, flags,
+ return send_structured_reply_block_status (request.cookie, cmd, flags,
count, offset, extents);
}
--
2.40.1
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>