Re: [PATCH v4 05/24] nbd: s/handle/cookie/ to match NBD spec

2023-06-12 Thread Vladimir Sementsov-Ogievskiy

On 08.06.23 16:56, Eric Blake wrote:

Externally, libnbd exposed the 64-bit opaque marker for each client
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 nown '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
libnbd's recommendations[1], so we can do likewise.

[1]https://github.com/NetworkBlockDevice/nbd/commit/ca4392eb2b

Signed-off-by: Eric Blake


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir




Re: [Libguestfs] [PATCH v4 05/24] nbd: s/handle/cookie/ to match NBD spec

2023-06-08 Thread Eric Blake
On Thu, Jun 08, 2023 at 08:56:34AM -0500, Eric Blake wrote:
> Externally, libnbd exposed the 64-bit opaque marker for each client
> 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 nown 'handle' as a way to identify a

noun

> packet and the verb 'handle' for reacting to things like signals.
> Upstream NBD changed their spec to favor the name "cookie" based on
> libnbd's recommendations[1], so we can do likewise.
> 
> [1] https://github.com/NetworkBlockDevice/nbd/commit/ca4392eb2b
> 
> Signed-off-by: Eric Blake 
> ---

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




[PATCH v4 05/24] nbd: s/handle/cookie/ to match NBD spec

2023-06-08 Thread Eric Blake
Externally, libnbd exposed the 64-bit opaque marker for each client
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 nown '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
libnbd's recommendations[1], so we can do likewise.

[1] https://github.com/NetworkBlockDevice/nbd/commit/ca4392eb2b

Signed-off-by: Eric Blake 
---

v4: new patch
---
 include/block/nbd.h | 11 +++---
 block/nbd.c | 96 +++--
 nbd/client.c| 14 +++
 nbd/server.c| 29 +++---
 nbd/trace-events| 22 +--
 5 files changed, 87 insertions(+), 85 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index e563f1774b0..59db69bafa5 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -58,7 +58,7 @@ typedef struct NBDOptionReplyMetaContext {
  * request and reply!
  */
 typedef struct NBDRequest {
-uint64_t handle;
+uint64_t cookie;
 uint64_t from;
 uint32_t len;
 uint16_t flags; /* NBD_CMD_FLAG_* */
@@ -68,7 +68,7 @@ typedef struct NBDRequest {
 typedef struct NBDSimpleReply {
 uint32_t magic;  /* NBD_SIMPLE_REPLY_MAGIC */
 uint32_t error;
-uint64_t handle;
+uint64_t cookie;
 } QEMU_PACKED NBDSimpleReply;

 /* Header of all structured replies */
@@ -76,7 +76,7 @@ typedef struct NBDStructuredReplyChunk {
 uint32_t magic;  /* NBD_STRUCTURED_REPLY_MAGIC */
 uint16_t flags;  /* combination of NBD_REPLY_FLAG_* */
 uint16_t type;   /* NBD_REPLY_TYPE_* */
-uint64_t handle; /* request handle */
+uint64_t cookie; /* request handle */
 uint32_t length; /* length of payload */
 } QEMU_PACKED NBDStructuredReplyChunk;

@@ -84,13 +84,14 @@ typedef union NBDReply {
 NBDSimpleReply simple;
 NBDStructuredReplyChunk structured;
 struct {
-/* @magic and @handle fields have the same offset and size both in
+/*
+ * @magic and @cookie fields have the same offset and size both in
  * simple reply and structured reply chunk, so let them be accessible
  * without ".simple." or ".structured." specification
  */
 uint32_t magic;
 uint32_t _skip;
-uint64_t handle;
+uint64_t cookie;
 } QEMU_PACKED;
 } NBDReply;

diff --git a/block/nbd.c b/block/nbd.c
index 5aef5cb6bd5..be3c46c6fee 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1,8 +1,8 @@
 /*
- * QEMU Block driver for  NBD
+ * QEMU Block driver for NBD
  *
  * Copyright (c) 2019 Virtuozzo International GmbH.
- * Copyright (C) 2016 Red Hat, Inc.
+ * Copyright Red Hat
  * Copyright (C) 2008 Bull S.A.S.
  * Author: Laurent Vivier 
  *
@@ -50,8 +50,8 @@
 #define EN_OPTSTR ":exportname="
 #define MAX_NBD_REQUESTS16

-#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
-#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))
+#define COOKIE_TO_INDEX(bs, cookie) ((cookie) ^ (uint64_t)(intptr_t)(bs))
+#define INDEX_TO_COOKIE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))

 typedef struct {
 Coroutine *coroutine;
@@ -417,25 +417,25 @@ static void coroutine_fn GRAPH_RDLOCK 
nbd_reconnect_attempt(BDRVNBDState *s)
 reconnect_delay_timer_del(s);
 }

-static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
+static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
 {
 int ret;
-uint64_t ind = HANDLE_TO_INDEX(s, handle), ind2;
+uint64_t ind = COOKIE_TO_INDEX(s, cookie), ind2;
 QEMU_LOCK_GUARD(>receive_mutex);

 while (true) {
-if (s->reply.handle == handle) {
+if (s->reply.cookie == cookie) {
 /* We are done */
 return 0;
 }

-if (s->reply.handle != 0) {
+if (s->reply.cookie != 0) {
 /*
  * Some other request is being handled now. It should already be
- * woken by whoever set s->reply.handle (or never wait in this
+ * woken by whoever set s->reply.cookie (or never wait in this
  * yield). So, we should not wake it here.
  */
-ind2 = HANDLE_TO_INDEX(s, s->reply.handle);
+ind2 = COOKIE_TO_INDEX(s, s->reply.cookie);
 assert(!s->requests[ind2].receiving);

 s->requests[ind].receiving = true;
@@ -445,9 +445,9 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState 
*s, uint64_t handle)
 /*
  * We may be woken for 2 reasons:
  * 1. From this function, executing in parallel coroutine, when our
- *handle is received.
+ *cookie is received.
  * 2. From nbd_co_receive_one_chunk(), when previous request is
- *finished and s->reply.handle set to 0.
+ *