[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 14/21] nbd/client: Split handshake into two functions
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PULL 14/21] nbd/client: Split handshake into two functions |
Date: |
Mon, 21 Jan 2019 16:49:00 -0600 |
An upcoming patch will add the ability for qemu-nbd to list
the services provided by an NBD server. Share the common
code of the TLS handshake by splitting the initial exchange
into a separate function, leaving only the export handling
in the original function. Functionally, there should be no
change in behavior in this patch, although some of the code
motion may be difficult to follow due to indentation changes
(view with 'git diff -w' for a smaller changeset).
I considered an enum for the return code coordinating state
between the two functions, but in the end just settled with
ample comments.
Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Richard W.M. Jones <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
---
nbd/client.c | 145 +++++++++++++++++++++++++++++++----------------
nbd/trace-events | 2 +-
2 files changed, 96 insertions(+), 51 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index 9028fd09397..77cc12356f2 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -809,22 +809,26 @@ static int nbd_negotiate_simple_meta_context(QIOChannel
*ioc,
return received;
}
-int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
- const char *hostname, QIOChannel **outioc,
- NBDExportInfo *info, Error **errp)
+/*
+ * nbd_start_negotiate:
+ * 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, client must still parse export size
+ * 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
+ */
+static int nbd_start_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
+ const char *hostname, QIOChannel **outioc,
+ bool structured_reply, bool *zeroes,
+ Error **errp)
{
uint64_t magic;
- bool zeroes = true;
- bool structured_reply = info->structured_reply;
- bool base_allocation = info->base_allocation;
- trace_nbd_receive_negotiate(tlscreds, hostname ? hostname : "<null>");
-
- assert(info->name);
- trace_nbd_receive_negotiate_name(info->name);
- info->structured_reply = false;
- info->base_allocation = false;
+ trace_nbd_start_negotiate(tlscreds, hostname ? hostname : "<null>");
+ *zeroes = true;
if (outioc) {
*outioc = NULL;
}
@@ -868,7 +872,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds
*tlscreds,
clientflags |= NBD_FLAG_C_FIXED_NEWSTYLE;
}
if (globalflags & NBD_FLAG_NO_ZEROES) {
- zeroes = false;
+ *zeroes = false;
clientflags |= NBD_FLAG_C_NO_ZEROES;
}
/* client requested flags */
@@ -890,7 +894,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds
*tlscreds,
}
}
if (fixedNewStyle) {
- int result;
+ int result = 0;
if (structured_reply) {
result = nbd_request_simple_option(ioc,
@@ -899,39 +903,85 @@ int nbd_receive_negotiate(QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
if (result < 0) {
return -EINVAL;
}
- info->structured_reply = result == 1;
}
+ return 2 + result;
+ } else {
+ return 1;
+ }
+ } else if (magic == NBD_CLIENT_MAGIC) {
+ if (tlscreds) {
+ error_setg(errp, "Server does not support STARTTLS");
+ return -EINVAL;
+ }
+ return 0;
+ } else {
+ error_setg(errp, "Bad server magic received: 0x%" PRIx64, magic);
+ return -EINVAL;
+ }
+}
- if (info->structured_reply && base_allocation) {
- result = nbd_negotiate_simple_meta_context(ioc, info, errp);
- if (result < 0) {
- return -EINVAL;
- }
- info->base_allocation = result == 1;
- }
+/*
+ * nbd_receive_negotiate:
+ * Connect to server, complete negotiation, and move into transmission phase.
+ * Returns: negative errno: failure talking to server
+ * 0: server is connected
+ */
+int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
+ const char *hostname, QIOChannel **outioc,
+ NBDExportInfo *info, Error **errp)
+{
+ int result;
+ bool zeroes;
+ bool base_allocation = info->base_allocation;
+ uint32_t oldflags;
+
+ assert(info->name);
+ trace_nbd_receive_negotiate_name(info->name);
+
+ result = nbd_start_negotiate(ioc, tlscreds, hostname, outioc,
+ info->structured_reply, &zeroes, errp);
+
+ info->structured_reply = false;
+ info->base_allocation = false;
+ if (tlscreds && *outioc) {
+ ioc = *outioc;
+ }
- /* 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
- * NBD_OPT_LIST for nicer error messages about a missing
- * export, then use NBD_OPT_EXPORT_NAME. */
- result = nbd_opt_go(ioc, info, errp);
+ switch (result) {
+ case 3: /* newstyle, with structured replies */
+ info->structured_reply = true;
+ if (base_allocation) {
+ result = nbd_negotiate_simple_meta_context(ioc, info, errp);
if (result < 0) {
return -EINVAL;
}
- if (result > 0) {
- return 0;
- }
- /* Check our desired export is present in the
- * server export list. Since NBD_OPT_EXPORT_NAME
- * cannot return an error message, running this
- * query gives us better error reporting if the
- * export name is not available.
- */
- if (nbd_receive_query_exports(ioc, info->name, errp) < 0) {
- return -EINVAL;
- }
+ info->base_allocation = result == 1;
}
+ /* fall through */
+ case 2: /* newstyle, try OPT_GO */
+ /* 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
+ * NBD_OPT_LIST for nicer error messages about a missing
+ * export, then use NBD_OPT_EXPORT_NAME. */
+ result = nbd_opt_go(ioc, info, errp);
+ if (result < 0) {
+ return -EINVAL;
+ }
+ if (result > 0) {
+ return 0;
+ }
+ /* Check our desired export is present in the
+ * server export list. Since NBD_OPT_EXPORT_NAME
+ * cannot return an error message, running this
+ * query gives us better error reporting if the
+ * export name is not available.
+ */
+ if (nbd_receive_query_exports(ioc, info->name, errp) < 0) {
+ return -EINVAL;
+ }
+ /* fall through */
+ case 1: /* newstyle, but limited to EXPORT_NAME */
/* write the export name request */
if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, info->name,
errp) < 0) {
@@ -950,17 +1000,12 @@ int nbd_receive_negotiate(QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
return -EINVAL;
}
info->flags = be16_to_cpu(info->flags);
- } else if (magic == NBD_CLIENT_MAGIC) {
- uint32_t oldflags;
-
+ break;
+ case 0: /* oldstyle, parse length and flags */
if (*info->name) {
error_setg(errp, "Server does not support non-empty export names");
return -EINVAL;
}
- if (tlscreds) {
- error_setg(errp, "Server does not support STARTTLS");
- return -EINVAL;
- }
if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) {
error_prepend(errp, "Failed to read export length: ");
@@ -978,9 +1023,9 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds
*tlscreds,
return -EINVAL;
}
info->flags = oldflags;
- } else {
- error_setg(errp, "Bad server magic received: 0x%" PRIx64, magic);
- return -EINVAL;
+ break;
+ default:
+ return result;
}
trace_nbd_receive_negotiate_size_flags(info->size, info->flags);
diff --git a/nbd/trace-events b/nbd/trace-events
index b4802c1570e..663d11687ab 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -14,7 +14,7 @@ nbd_receive_starttls_new_client(void) "Setting up TLS"
nbd_receive_starttls_tls_handshake(void) "Starting TLS handshake"
nbd_opt_meta_request(const char *optname, const char *context, const char
*export) "Requesting %s %s for export %s"
nbd_opt_meta_reply(const char *optname, const char *context, uint32_t id)
"Received %s mapping of %s to id %" PRIu32
-nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving
negotiation tlscreds=%p hostname=%s"
+nbd_start_negotiate(void *tlscreds, const char *hostname) "Receiving
negotiation tlscreds=%p hostname=%s"
nbd_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64
nbd_receive_negotiate_server_flags(uint32_t globalflags) "Global flags are
0x%" PRIx32
nbd_receive_negotiate_name(const char *name) "Requesting NBD export name '%s'"
--
2.20.1
- [Qemu-devel] [PULL 00/21] NBD patches through 2019-01-21, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 02/21] maint: Allow for EXAMPLES in texi2pod, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 01/21] iotests: Make 233 output more reliable, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 03/21] qemu-nbd: Enhance man page, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 04/21] qemu-nbd: Sanity check partition bounds, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 05/21] nbd/server: Hoist length check to qmp_nbd_server_add, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 17/21] nbd/client: Add nbd_receive_export_list(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 08/21] nbd/client: Refactor nbd_receive_list(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 16/21] nbd/client: Refactor nbd_opt_go() to support NBD_OPT_INFO, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 06/21] nbd/server: Favor [u]int64_t over off_t, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 14/21] nbd/client: Split handshake into two functions,
Eric Blake <=
- [Qemu-devel] [PULL 09/21] nbd/client: Move export name into NBDExportInfo, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 11/21] nbd/client: Split out nbd_send_meta_query(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 18/21] nbd/client: Add meta contexts to nbd_receive_export_list(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 12/21] nbd/client: Split out nbd_receive_one_meta_context(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 10/21] nbd/client: Change signature of nbd_negotiate_simple_meta_context(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 15/21] nbd/client: Pull out oldstyle size determination, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 20/21] nbd/client: Work around 3.0 bug for listing meta contexts, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 07/21] qemu-nbd: Avoid strtol open-coding, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 13/21] nbd/client: Refactor return of nbd_receive_negotiate(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 21/21] iotests: Enhance 223, 233 to cover 'qemu-nbd --list', Eric Blake, 2019/01/21