From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, libguestfs@redhat.com,
vsementsov@yandex-team.ru, Kevin Wolf <kwolf@redhat.com>,
Hanna Reitz <hreitz@redhat.com>
Subject: [PATCH v4 08/24] nbd: Use enum for various negotiation modes
Date: Thu, 8 Jun 2023 08:56:37 -0500 [thread overview]
Message-ID: <20230608135653.2918540-9-eblake@redhat.com> (raw)
In-Reply-To: <20230608135653.2918540-1-eblake@redhat.com>
Deciphering the hard-coded list of integer return values from
nbd_start_negotiate() will only get more confusing when adding support
for 64-bit extended headers. Better is to name things in an enum.
Although the function in question is private to client.c, putting the
enum in a public header and including an enum-to-string conversion
will allow its use in more places in upcoming patches.
The enum is intentionally laid out so that operators like <= can be
used to group multiple modes with similar characteristics, and where
the least powerful mode has value 0, even though this patch does not
exploit that. No semantic change intended.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v4: new patch, expanding enum idea from v3 4/14
---
include/block/nbd.h | 11 +++++++++++
nbd/client.c | 46 ++++++++++++++++++++++++---------------------
nbd/common.c | 17 +++++++++++++++++
3 files changed, 53 insertions(+), 21 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 59db69bafa5..aba4279b56c 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -52,6 +52,16 @@ typedef struct NBDOptionReplyMetaContext {
/* metadata context name follows */
} QEMU_PACKED NBDOptionReplyMetaContext;
+/* Track results of negotiation */
+typedef enum NBDMode {
+ /* Keep this list in a continuum of increasing features. */
+ NBD_MODE_OLDSTYLE, /* server lacks newstyle negotiation */
+ NBD_MODE_EXPORT_NAME, /* newstyle but only OPT_EXPORT_NAME safe */
+ NBD_MODE_SIMPLE, /* newstyle but only simple replies */
+ NBD_MODE_STRUCTURED, /* newstyle, structured replies enabled */
+ /* TODO add NBD_MODE_EXTENDED */
+} NBDMode;
+
/* Transmission phase structs
*
* Note: these are _NOT_ the same as the network representation of an NBD
@@ -404,6 +414,7 @@ const char *nbd_rep_lookup(uint32_t rep);
const char *nbd_info_lookup(uint16_t info);
const char *nbd_cmd_lookup(uint16_t info);
const char *nbd_err_lookup(int err);
+const char *nbd_mode_lookup(NBDMode mode);
/* nbd/client-connection.c */
typedef struct NBDClientConnection NBDClientConnection;
diff --git a/nbd/client.c b/nbd/client.c
index 1b5569556fe..479208d5d9d 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -875,10 +875,7 @@ static int nbd_list_meta_contexts(QIOChannel *ioc,
* Start the handshake to the server. After a positive return, the server
* is ready to accept additional NBD_OPT requests.
* Returns: negative errno: failure talking to server
- * 0: server is oldstyle, must call nbd_negotiate_finish_oldstyle
- * 1: server is newstyle, but can only accept EXPORT_NAME
- * 2: server is newstyle, but lacks structured replies
- * 3: server is newstyle and set up for structured replies
+ * non-negative: enum NBDMode describing server abilities
*/
static int nbd_start_negotiate(AioContext *aio_context, QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
@@ -969,16 +966,16 @@ static int nbd_start_negotiate(AioContext *aio_context, QIOChannel *ioc,
return -EINVAL;
}
}
- return 2 + result;
+ return result ? NBD_MODE_STRUCTURED : NBD_MODE_SIMPLE;
} else {
- return 1;
+ return NBD_MODE_EXPORT_NAME;
}
} else if (magic == NBD_CLIENT_MAGIC) {
if (tlscreds) {
error_setg(errp, "Server does not support STARTTLS");
return -EINVAL;
}
- return 0;
+ return NBD_MODE_OLDSTYLE;
} else {
error_setg(errp, "Bad server magic received: 0x%" PRIx64, magic);
return -EINVAL;
@@ -1032,6 +1029,9 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
result = nbd_start_negotiate(aio_context, ioc, tlscreds, hostname, outioc,
info->structured_reply, &zeroes, errp);
+ if (result < 0) {
+ return result;
+ }
info->structured_reply = false;
info->base_allocation = false;
@@ -1039,8 +1039,8 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
ioc = *outioc;
}
- switch (result) {
- case 3: /* newstyle, with structured replies */
+ switch ((NBDMode)result) {
+ case NBD_MODE_STRUCTURED:
info->structured_reply = true;
if (base_allocation) {
result = nbd_negotiate_simple_meta_context(ioc, info, errp);
@@ -1050,7 +1050,7 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
info->base_allocation = result == 1;
}
/* fall through */
- case 2: /* newstyle, try OPT_GO */
+ case NBD_MODE_SIMPLE:
/* Try NBD_OPT_GO first - if it works, we are done (it
* also gives us a good message if the server requires
* TLS). If it is not available, fall back to
@@ -1073,7 +1073,7 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
return -EINVAL;
}
/* fall through */
- case 1: /* newstyle, but limited to EXPORT_NAME */
+ case NBD_MODE_EXPORT_NAME:
/* write the export name request */
if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, info->name,
errp) < 0) {
@@ -1089,7 +1089,7 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
return -EINVAL;
}
break;
- case 0: /* oldstyle, parse length and flags */
+ case NBD_MODE_OLDSTYLE:
if (*info->name) {
error_setg(errp, "Server does not support non-empty export names");
return -EINVAL;
@@ -1099,7 +1099,7 @@ int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
}
break;
default:
- return result;
+ g_assert_not_reached();
}
trace_nbd_receive_negotiate_size_flags(info->size, info->flags);
@@ -1155,10 +1155,13 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
if (tlscreds && sioc) {
ioc = sioc;
}
+ if (result < 0) {
+ goto out;
+ }
- switch (result) {
- case 2:
- case 3:
+ switch ((NBDMode)result) {
+ case NBD_MODE_SIMPLE:
+ case NBD_MODE_STRUCTURED:
/* newstyle - use NBD_OPT_LIST to populate array, then try
* NBD_OPT_INFO on each array member. If structured replies
* are enabled, also try NBD_OPT_LIST_META_CONTEXT. */
@@ -1179,7 +1182,7 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
memset(&array[count - 1], 0, sizeof(*array));
array[count - 1].name = name;
array[count - 1].description = desc;
- array[count - 1].structured_reply = result == 3;
+ array[count - 1].structured_reply = result == NBD_MODE_STRUCTURED;
}
for (i = 0; i < count; i++) {
@@ -1195,7 +1198,7 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
break;
}
- if (result == 3 &&
+ if (result == NBD_MODE_STRUCTURED &&
nbd_list_meta_contexts(ioc, &array[i], errp) < 0) {
goto out;
}
@@ -1204,11 +1207,12 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
/* Send NBD_OPT_ABORT as a courtesy before hanging up */
nbd_send_opt_abort(ioc);
break;
- case 1: /* newstyle, but limited to EXPORT_NAME */
+ case NBD_MODE_EXPORT_NAME:
error_setg(errp, "Server does not support export lists");
/* We can't even send NBD_OPT_ABORT, so merely hang up */
goto out;
- case 0: /* oldstyle, parse length and flags */
+ case NBD_MODE_OLDSTYLE:
+ /* Lone export name is implied, but we can parse length and flags */
array = g_new0(NBDExportInfo, 1);
array->name = g_strdup("");
count = 1;
@@ -1226,7 +1230,7 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
}
break;
default:
- goto out;
+ g_assert_not_reached();
}
*info = array;
diff --git a/nbd/common.c b/nbd/common.c
index ddfe7d11837..989fbe54a19 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -248,3 +248,20 @@ int nbd_errno_to_system_errno(int err)
}
return ret;
}
+
+
+const char *nbd_mode_lookup(NBDMode mode)
+{
+ switch (mode) {
+ case NBD_MODE_OLDSTYLE:
+ return "oldstyle";
+ case NBD_MODE_EXPORT_NAME:
+ return "export name only";
+ case NBD_MODE_SIMPLE:
+ return "simple headers";
+ case NBD_MODE_STRUCTURED:
+ return "structured replies";
+ default:
+ return "<unknown>";
+ }
+}
--
2.40.1
next prev parent reply other threads:[~2023-06-08 14:00 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-08 13:56 [PATCH v4 00/24] qemu patches for 64-bit NBD extensions Eric Blake
2023-06-08 13:56 ` [PATCH v4 01/24] nbd/client: Use smarter assert Eric Blake
2023-06-08 13:56 ` [PATCH v4 02/24] nbd: Consistent typedef usage in header Eric Blake
2023-06-08 14:17 ` [Libguestfs] " Eric Blake
2023-06-12 11:59 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 03/24] nbd/server: Prepare for alternate-size headers Eric Blake
2023-06-12 13:53 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 04/24] nbd/server: Refactor to pass full request around Eric Blake
2023-06-08 13:56 ` [PATCH v4 05/24] nbd: s/handle/cookie/ to match NBD spec Eric Blake
2023-06-08 14:32 ` [Libguestfs] " Eric Blake
2023-06-12 14:12 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 06/24] nbd/client: Simplify cookie vs. index computation Eric Blake
2023-06-12 14:27 ` Vladimir Sementsov-Ogievskiy
2023-06-12 19:13 ` Eric Blake
2023-06-08 13:56 ` [PATCH v4 07/24] nbd/client: Add safety check on chunk payload length Eric Blake
2023-06-08 13:56 ` Eric Blake [this message]
2023-06-12 14:39 ` [PATCH v4 08/24] nbd: Use enum for various negotiation modes Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 09/24] nbd: Replace bool structured_reply with mode enum Eric Blake
2023-06-12 15:07 ` Vladimir Sementsov-Ogievskiy
2023-06-12 19:24 ` [Libguestfs] " Eric Blake
2023-07-19 20:11 ` Eric Blake
2023-06-08 13:56 ` [PATCH v4 10/24] nbd/client: Pass mode through to nbd_send_request Eric Blake
2023-06-12 15:48 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 11/24] nbd: Add types for extended headers Eric Blake
2023-06-12 16:11 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 12/24] nbd: Prepare for 64-bit request effect lengths Eric Blake
2023-06-08 18:26 ` [Libguestfs] " Eric Blake
2023-06-16 18:16 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 13/24] nbd/server: Refactor handling of request payload Eric Blake
2023-06-08 18:29 ` [Libguestfs] " Eric Blake
2023-06-16 18:29 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 14/24] nbd/server: Prepare to receive extended header requests Eric Blake
2023-06-16 18:35 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 15/24] nbd/server: Prepare to send extended header replies Eric Blake
2023-06-16 18:48 ` Vladimir Sementsov-Ogievskiy
2023-08-04 19:28 ` Eric Blake
2023-08-07 17:20 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 16/24] nbd/server: Support 64-bit block status Eric Blake
2023-06-27 13:23 ` Vladimir Sementsov-Ogievskiy
2023-08-04 19:36 ` Eric Blake
2023-08-07 17:28 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 17/24] nbd/server: Enable initial support for extended headers Eric Blake
2023-06-27 13:26 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 18/24] nbd/client: Plumb errp through nbd_receive_replies Eric Blake
2023-06-08 19:10 ` [Libguestfs] " Eric Blake
2023-06-27 13:31 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 19/24] nbd/client: Initial support for extended headers Eric Blake
2023-06-27 14:22 ` Vladimir Sementsov-Ogievskiy
2023-08-07 19:20 ` Eric Blake
2023-06-08 13:56 ` [PATCH v4 20/24] nbd/client: Accept 64-bit block status chunks Eric Blake
2023-06-27 14:50 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 21/24] nbd/client: Request extended headers during negotiation Eric Blake
2023-06-27 14:55 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 22/24] nbd/server: Refactor list of negotiated meta contexts Eric Blake
2023-06-27 15:11 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 23/24] nbd/server: Prepare for per-request filtering of BLOCK_STATUS Eric Blake
2023-06-08 19:15 ` [Libguestfs] " Eric Blake
2023-06-27 15:19 ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 24/24] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS Eric Blake
2023-06-08 19:19 ` [Libguestfs] " Eric Blake
2023-06-27 19:42 ` Vladimir Sementsov-Ogievskiy
2023-08-07 20:23 ` [Libguestfs] " Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230608135653.2918540-9-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=libguestfs@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@yandex-team.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).