qemu-devel
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]