[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 14/22] nbd/client: Split out nbd_receive_one_meta
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v2 14/22] nbd/client: Split out nbd_receive_one_meta_context() |
Date: |
Sat, 15 Dec 2018 07:53:16 -0600 |
Refactor nbd_negotiate_simple_meta_context() to more closely
resemble the pattern of nbd_receive_list(), separating the
argument validation for one pass from the caller making a loop
over passes. No major semantic change (although one error
message loses the original query). The diff may be a bit hard
to follow due to indentation changes and handling ACK first
rather than last.
Signed-off-by: Eric Blake <address@hidden>
---
v2: split patch into easier-to-review pieces [Rich, Vladimir]
---
nbd/client.c | 144 +++++++++++++++++++++++++++--------------------
nbd/trace-events | 2 +-
2 files changed, 84 insertions(+), 62 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index 5b6b9964097..0e5a9d59dbd 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -669,6 +669,83 @@ static int nbd_send_one_meta_context(QIOChannel *ioc,
return ret;
}
+/* nbd_receive_one_meta_context:
+ * Called in a loop to receive and trace one set/list meta context reply.
+ * Pass non-NULL @name or @id to collect results back to the caller, which
+ * must eventually call g_free().
+ * return 1 if more contexts are expected,
+ * 0 if operation is complete,
+ * -1 with errp set for any error
+ */
+static int nbd_receive_one_meta_context(QIOChannel *ioc,
+ uint32_t opt,
+ char **name,
+ uint32_t *id,
+ Error **errp)
+{
+ int ret;
+ NBDOptionReply reply;
+ char *local_name = NULL;
+ uint32_t local_id;
+
+ if (nbd_receive_option_reply(ioc, opt, &reply, errp) < 0) {
+ return -1;
+ }
+
+ ret = nbd_handle_reply_err(ioc, &reply, errp);
+ if (ret <= 0) {
+ return ret;
+ }
+
+ if (reply.type == NBD_REP_ACK) {
+ if (reply.length != 0) {
+ error_setg(errp, "Unexpected length to ACK response");
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+ return 0;
+ } else if (reply.type != NBD_REP_META_CONTEXT) {
+ error_setg(errp, "Unexpected reply type %u (%s), expected %u (%s)",
+ reply.type, nbd_rep_lookup(reply.type),
+ NBD_REP_META_CONTEXT, nbd_rep_lookup(NBD_REP_META_CONTEXT));
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+
+ if (reply.length <= sizeof(local_id) ||
+ reply.length > NBD_MAX_BUFFER_SIZE) {
+ error_setg(errp, "Failed to negotiate meta context, server "
+ "answered with unexpected length %" PRIu32,
+ reply.length);
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+
+ if (nbd_read(ioc, &local_id, sizeof(local_id), errp) < 0) {
+ return -1;
+ }
+ local_id = be32_to_cpu(local_id);
+
+ reply.length -= sizeof(local_id);
+ local_name = g_malloc(reply.length + 1);
+ if (nbd_read(ioc, local_name, reply.length, errp) < 0) {
+ g_free(local_name);
+ return -1;
+ }
+ local_name[reply.length] = '\0';
+ trace_nbd_opt_meta_reply(nbd_opt_lookup(opt), local_name, local_id);
+
+ if (name) {
+ *name = local_name;
+ } else {
+ g_free(local_name);
+ }
+ if (id) {
+ *id = local_id;
+ }
+ return 1;
+}
+
/* nbd_negotiate_simple_meta_context:
* Request the server to set the meta context for export @info->name
* using @info->x_dirty_bitmap with a fallback to "base:allocation",
@@ -690,7 +767,6 @@ static int nbd_negotiate_simple_meta_context(QIOChannel
*ioc,
* function should lose the term _simple.
*/
int ret;
- NBDOptionReply reply;
const char *context = info->x_dirty_bitmap ?: "base:allocation";
bool received = false;
@@ -699,44 +775,17 @@ static int nbd_negotiate_simple_meta_context(QIOChannel
*ioc,
return -1;
}
- if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply,
- errp) < 0)
- {
- return -1;
- }
-
- ret = nbd_handle_reply_err(ioc, &reply, errp);
- if (ret <= 0) {
- return ret;
- }
-
- while (reply.type == NBD_REP_META_CONTEXT) {
+ while (1) {
char *name;
- if (reply.length <= sizeof(info->context_id) ||
- reply.length > NBD_MAX_BUFFER_SIZE) {
- error_setg(errp, "Failed to negotiate meta context '%s', server "
- "answered with unexpected length %" PRIu32, context,
- reply.length);
- nbd_send_opt_abort(ioc);
+ ret = nbd_receive_one_meta_context(ioc, NBD_OPT_SET_META_CONTEXT,
+ &name, &info->context_id, errp);
+ if (ret < 0) {
return -1;
+ } else if (ret == 0) {
+ return received;
}
- if (nbd_read(ioc, &info->context_id, sizeof(info->context_id),
- errp) < 0) {
- return -1;
- }
- info->context_id = be32_to_cpu(info->context_id);
-
- reply.length -= sizeof(info->context_id);
- name = g_malloc(reply.length + 1);
- if (nbd_read(ioc, name, reply.length, errp) < 0) {
- g_free(name);
- return -1;
- }
- name[reply.length] = '\0';
- trace_nbd_opt_meta_reply(name, info->context_id);
-
if (received) {
error_setg(errp, "Server replied with more than one context");
g_free(name);
@@ -754,34 +803,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel
*ioc,
}
g_free(name);
received = true;
-
- /* receive NBD_REP_ACK */
- if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply,
- errp) < 0)
- {
- return -1;
- }
-
- ret = nbd_handle_reply_err(ioc, &reply, errp);
- if (ret <= 0) {
- return ret;
- }
}
-
- if (reply.type != NBD_REP_ACK) {
- error_setg(errp, "Unexpected reply type %u (%s), expected %u (%s)",
- reply.type, nbd_rep_lookup(reply.type),
- NBD_REP_ACK, nbd_rep_lookup(NBD_REP_ACK));
- nbd_send_opt_abort(ioc);
- return -1;
- }
- if (reply.length) {
- error_setg(errp, "Unexpected length to ACK response");
- nbd_send_opt_abort(ioc);
- return -1;
- }
-
- return received;
}
int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
diff --git a/nbd/trace-events b/nbd/trace-events
index 00872a6f9d4..02956c96042 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -12,7 +12,7 @@ nbd_receive_query_exports_success(const char *wantname)
"Found desired export na
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 *context, uint32_t id) "Received mapping of
context %s to id %" PRIu32
+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_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64
nbd_receive_negotiate_server_flags(uint32_t globalflags) "Global flags are
0x%" PRIx32
--
2.17.2
- [Qemu-devel] [PATCH v2 10/22] nbd/client: Move export name into NBDExportInfo, (continued)
- [Qemu-devel] [PATCH v2 10/22] nbd/client: Move export name into NBDExportInfo, Eric Blake, 2018/12/15
- [Qemu-devel] [PATCH v2 12/22] nbd/client: Improve error handling in nbd_negotiate_simple_meta_context(), Eric Blake, 2018/12/15
- [Qemu-devel] [PATCH v2 09/22] nbd/client: Refactor nbd_receive_list(), Eric Blake, 2018/12/15
- [Qemu-devel] [PATCH v2 13/22] nbd/client: Split out nbd_send_one_meta_context(), Eric Blake, 2018/12/15
- [Qemu-devel] [PATCH v2 14/22] nbd/client: Split out nbd_receive_one_meta_context(),
Eric Blake <=
- [Qemu-devel] [PATCH v2 15/22] nbd/client: Refactor return of nbd_receive_negotiate(), Eric Blake, 2018/12/15
- [Qemu-devel] [PATCH v2 16/22] nbd/client: Split handshake into two functions, Eric Blake, 2018/12/15
- [Qemu-devel] [PATCH v2 17/22] nbd/client: Pull out oldstyle size determination, Eric Blake, 2018/12/15
- [Qemu-devel] [PATCH v2 19/22] nbd/client: Add meta contexts to nbd_receive_export_list(), Eric Blake, 2018/12/15
- [Qemu-devel] [PATCH v2 18/22] nbd/client: Add nbd_receive_export_list(), Eric Blake, 2018/12/15