qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v1.1 DRAFT] nbd: Minimal structured read for cli


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH v1.1 DRAFT] nbd: Minimal structured read for client
Date: Tue, 3 Oct 2017 12:59:31 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

Eric?


27.09.2017 18:10, Vladimir Sementsov-Ogievskiy wrote:
Minimal implementation: drop most of additional error information.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---

Hi!

Here is a draft of how to refactor reply-payload receiving if you don't
like my previous simple (but not flexible) try. Ofcourse, if we agree on this
approach this patch should be splitted into several ones and many things
(error handling) should be improved.

The idea is:

nbd_read_reply_entry reads only reply header through nbd/client.c code.

Then, the payload is read through block/nbd-client-cmds.c:
simple payload: generic per-command handler, however it should only exist
   for CMD_READ
structured NONE: no payload, handle in nbd_co_receive_one_chunk
structured error: read by nbd_handle_structured_error_payload
   defined in block/nbd-client-cmds.c
structured success: read by per-[command X reply-type] handler
   defined in block/nbd-client-cmds.c

For now nbd-client-cmds.c looks more like nbd-payload.c, but, may be, we
should move command sending special-casing (CMD_WRITE) to it too..

Don't waste time on careful reviewing this patch, let's consider first
the concept of nbd-client-cmds.c,

  block/nbd-client.h      |  10 +++
  include/block/nbd.h     |  82 ++++++++++++++++--
  nbd/nbd-internal.h      |  25 ------
  block/nbd-client-cmds.c | 220 ++++++++++++++++++++++++++++++++++++++++++++++++
  block/nbd-client.c      | 118 ++++++++++++++++++++------
  nbd/client.c            | 128 ++++++++++++++++------------
  block/Makefile.objs     |   2 +-
  7 files changed, 475 insertions(+), 110 deletions(-)
  create mode 100644 block/nbd-client-cmds.c

diff --git a/block/nbd-client.h b/block/nbd-client.h
index b435754b82..abb88e4ea5 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -35,6 +35,8 @@ typedef struct NBDClientSession {
      NBDClientRequest requests[MAX_NBD_REQUESTS];
      NBDReply reply;
      bool quit;
+
+    bool structured_reply;
  } NBDClientSession;
NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
@@ -60,4 +62,12 @@ void nbd_client_detach_aio_context(BlockDriverState *bs);
  void nbd_client_attach_aio_context(BlockDriverState *bs,
                                     AioContext *new_context);
+int nbd_handle_structured_payload(QIOChannel *ioc, int cmd,
+                                  NBDStructuredReplyChunk *reply, void 
*opaque);
+int nbd_handle_simple_payload(QIOChannel *ioc, int cmd, void *opaque);
+
+int nbd_handle_structured_error_payload(QIOChannel *ioc,
+                                        NBDStructuredReplyChunk *reply,
+                                        int *request_ret);
+
  #endif /* NBD_CLIENT_H */
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 314f2f9bbc..b9a4e1dfa9 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -57,12 +57,6 @@ struct NBDRequest {
  };
  typedef struct NBDRequest NBDRequest;
-struct NBDReply {
-    uint64_t handle;
-    uint32_t error;
-};
-typedef struct NBDReply NBDReply;
-
  typedef struct NBDSimpleReply {
      uint32_t magic;  /* NBD_SIMPLE_REPLY_MAGIC */
      uint32_t error;
@@ -77,6 +71,24 @@ typedef struct NBDStructuredReplyChunk {
      uint32_t length; /* length of payload */
  } QEMU_PACKED NBDStructuredReplyChunk;
+typedef union NBDReply {
+    NBDSimpleReply simple;
+    NBDStructuredReplyChunk structured;
+    struct {
+        uint32_t magic;
+        uint32_t _skip;
+        uint64_t handle;
+    } QEMU_PACKED;
+} NBDReply;
+
+#define NBD_SIMPLE_REPLY_MAGIC      0x67446698
+#define NBD_STRUCTURED_REPLY_MAGIC  0x668e33ef
+
+static inline bool nbd_reply_is_simple(NBDReply *reply)
+{
+    return reply->magic == NBD_SIMPLE_REPLY_MAGIC;
+}
+
  typedef struct NBDStructuredRead {
      NBDStructuredReplyChunk h;
      uint64_t offset;
@@ -88,6 +100,11 @@ typedef struct NBDStructuredError {
      uint16_t message_length;
  } QEMU_PACKED NBDStructuredError;
+typedef struct NBDPayloadOffsetHole {
+    uint64_t offset;
+    uint32_t hole_size;
+} QEMU_PACKED NBDPayloadOffsetHole;
+
  /* Transmission (export) flags: sent from server to client during handshake,
     but describe what will happen during transmission */
  #define NBD_FLAG_HAS_FLAGS         (1 << 0) /* Flags are there */
@@ -178,12 +195,54 @@ enum {
#define NBD_SREP_TYPE_NONE 0
  #define NBD_SREP_TYPE_OFFSET_DATA   1
+#define NBD_SREP_TYPE_OFFSET_HOLE   2
  #define NBD_SREP_TYPE_ERROR         NBD_SREP_ERR(1)
+#define NBD_SREP_TYPE_ERROR_OFFSET  NBD_SREP_ERR(2)
+
+static inline bool nbd_srep_type_is_error(int type)
+{
+    return type & (1 << 15);
+}
+
+/* NBD errors are based on errno numbers, so there is a 1:1 mapping,
+ * but only a limited set of errno values is specified in the protocol.
+ * Everything else is squashed to EINVAL.
+ */
+#define NBD_SUCCESS    0
+#define NBD_EPERM      1
+#define NBD_EIO        5
+#define NBD_ENOMEM     12
+#define NBD_EINVAL     22
+#define NBD_ENOSPC     28
+#define NBD_ESHUTDOWN  108
+
+static inline int nbd_errno_to_system_errno(int err)
+{
+    switch (err) {
+    case NBD_SUCCESS:
+        return 0;
+    case NBD_EPERM:
+        return EPERM;
+    case NBD_EIO:
+        return EIO;
+    case NBD_ENOMEM:
+        return ENOMEM;
+    case NBD_ENOSPC:
+        return ENOSPC;
+    case NBD_ESHUTDOWN:
+        return ESHUTDOWN;
+    case NBD_EINVAL:
+        return EINVAL;
+    }
+
+    return EINVAL;
+}
/* Details collected by NBD_OPT_EXPORT_NAME and NBD_OPT_GO */
  struct NBDExportInfo {
      /* Set by client before nbd_receive_negotiate() */
      bool request_sizes;
+    bool structured_reply;
      /* Set by server results during nbd_receive_negotiate() */
      uint64_t size;
      uint16_t flags;
@@ -233,4 +292,15 @@ void nbd_client_put(NBDClient *client);
  void nbd_server_start(SocketAddress *addr, const char *tls_creds,
                        Error **errp);
+/* nbd_read
+ * Reads @size bytes from @ioc. Returns 0 on success.
+ */
+static inline int nbd_read(QIOChannel *ioc, void *buffer, size_t size,
+                           Error **errp)
+{
+    return qio_channel_read_all(ioc, buffer, size, errp) < 0 ? -EIO : 0;
+}
+
+int nbd_drop(QIOChannel *ioc, size_t size, Error **errp);
+
  #endif
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 6b0d1183ba..9f7b6b68e8 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -47,8 +47,6 @@
  #define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124)
#define NBD_REQUEST_MAGIC 0x25609513
-#define NBD_SIMPLE_REPLY_MAGIC      0x67446698
-#define NBD_STRUCTURED_REPLY_MAGIC  0x668e33ef
  #define NBD_OPTS_MAGIC              0x49484156454F5054LL
  #define NBD_CLIENT_MAGIC            0x0000420281861253LL
  #define NBD_REP_MAGIC               0x0003e889045565a9LL
@@ -65,18 +63,6 @@
  #define NBD_SET_TIMEOUT             _IO(0xab, 9)
  #define NBD_SET_FLAGS               _IO(0xab, 10)
-/* NBD errors are based on errno numbers, so there is a 1:1 mapping,
- * but only a limited set of errno values is specified in the protocol.
- * Everything else is squashed to EINVAL.
- */
-#define NBD_SUCCESS    0
-#define NBD_EPERM      1
-#define NBD_EIO        5
-#define NBD_ENOMEM     12
-#define NBD_EINVAL     22
-#define NBD_ENOSPC     28
-#define NBD_ESHUTDOWN  108
-
  /* nbd_read_eof
   * Tries to read @size bytes from @ioc.
   * Returns 1 on success
@@ -96,15 +82,6 @@ static inline int nbd_read_eof(QIOChannel *ioc, void 
*buffer, size_t size,
      return ret;
  }
-/* nbd_read
- * Reads @size bytes from @ioc. Returns 0 on success.
- */
-static inline int nbd_read(QIOChannel *ioc, void *buffer, size_t size,
-                           Error **errp)
-{
-    return qio_channel_read_all(ioc, buffer, size, errp) < 0 ? -EIO : 0;
-}
-
  /* nbd_write
   * Writes @size bytes to @ioc. Returns 0 on success.
   */
@@ -137,6 +114,4 @@ const char *nbd_rep_lookup(uint32_t rep);
  const char *nbd_info_lookup(uint16_t info);
  const char *nbd_cmd_lookup(uint16_t info);
-int nbd_drop(QIOChannel *ioc, size_t size, Error **errp);
-
  #endif
diff --git a/block/nbd-client-cmds.c b/block/nbd-client-cmds.c
new file mode 100644
index 0000000000..488a3dc267
--- /dev/null
+++ b/block/nbd-client-cmds.c
@@ -0,0 +1,220 @@
+/*
+ * QEMU Block driver for NBD
+ *
+ * Copyright (c) 2017 Parallels International GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "nbd-client.h"
+
+
+typedef int (*NBDCommandFn)(QIOChannel *ioc,
+                            NBDStructuredReplyChunk *chunk,
+                            void *opaque);
+typedef struct NBDCommand {
+    int (*read_simple_payload)(QIOChannel *ioc, void *opaque);
+    NBDCommandFn read_offset_data;
+    NBDCommandFn read_offset_hole;
+} NBDCommand;
+
+static int nbd_cmd_read__siple_payload(QIOChannel *ioc, void *opaque)
+{
+    QEMUIOVector *qiov = (QEMUIOVector *)opaque;
+    return qio_channel_readv_all(ioc, qiov->iov, qiov->niov, NULL);
+}
+
+static int nbd_cmd_read__offset_data(QIOChannel *ioc,
+                                     NBDStructuredReplyChunk *chunk,
+                                     void *opaque)
+{
+    QEMUIOVector *qiov = (QEMUIOVector *)opaque;
+    QEMUIOVector sub_qiov;
+    uint64_t offset;
+    size_t data_size;
+    int ret;
+
+    if (chunk->length < sizeof(offset)) {
+        return -1;
+    }
+
+    if (nbd_read(ioc, &offset, sizeof(offset), NULL) < 0) {
+        return -1;
+    }
+    be64_to_cpus(&offset);
+
+    data_size = chunk->length - sizeof(offset);
+    if (offset + data_size > qiov->size) {
+        return -1;
+    }
+
+    qemu_iovec_init(&sub_qiov, qiov->niov);
+    qemu_iovec_concat(&sub_qiov, qiov, offset, data_size);
+    ret = qio_channel_readv_all(ioc, sub_qiov.iov, sub_qiov.niov, NULL);
+    qemu_iovec_destroy(&sub_qiov);
+
+    return ret;
+}
+
+static int nbd_cmd_read__offset_hole(QIOChannel *ioc,
+                                     NBDStructuredReplyChunk *chunk,
+                                     void *opaque)
+{
+    QEMUIOVector *qiov = (QEMUIOVector *)opaque;
+    NBDPayloadOffsetHole pl;
+
+    if (chunk->length != sizeof(pl)) {
+        return -1;
+    }
+
+    if (nbd_read(ioc, &pl, sizeof(pl), NULL) < 0) {
+        return -1;
+    }
+
+    be64_to_cpus(&pl.offset);
+    be32_to_cpus(&pl.hole_size);
+
+    if (pl.offset + pl.hole_size > qiov->size) {
+        return -1;
+    }
+
+    qemu_iovec_memset(qiov, pl.offset, 0, pl.hole_size);
+
+    return 0;
+}
+
+NBDCommand nbd_cmd_read = {
+    .read_simple_payload = nbd_cmd_read__siple_payload,
+    .read_offset_data = nbd_cmd_read__offset_data,
+    .read_offset_hole = nbd_cmd_read__offset_hole,
+};
+
+static NBDCommand *nbd_get_cmd(int cmd)
+{
+    switch (cmd) {
+    case NBD_CMD_READ:
+        return &nbd_cmd_read;
+    }
+
+    return NULL;
+}
+
+static NBDCommandFn nbd_cmd_get_handler(int cmd, int reply_type)
+{
+    NBDCommand *command = nbd_get_cmd(cmd);
+
+    if (command == NULL) {
+        return NULL;
+    }
+
+    switch (reply_type) {
+    case NBD_SREP_TYPE_OFFSET_DATA:
+        return command->read_offset_data;
+    case NBD_SREP_TYPE_OFFSET_HOLE:
+        return command->read_offset_hole;
+    }
+
+    return NULL;
+}
+
+/* nbd_handle_structured_payload
+ * Find corresponding handler and call it.
+ * If not found return -1 (fail)
+ */
+int nbd_handle_structured_payload(QIOChannel *ioc, int cmd, 
NBDStructuredReplyChunk *chunk,
+                                  void *opaque)
+{
+    NBDCommandFn fn = nbd_cmd_get_handler(cmd, chunk->type);
+
+    if (fn == NULL) {
+        return -1;
+    }
+
+    return fn(ioc, chunk, opaque);
+}
+
+/* nbd_handle_simple_payload
+ * Find corresponding handler and call it.
+ * If not found return 0 (success)
+ */
+int nbd_handle_simple_payload(QIOChannel *ioc, int cmd, void *opaque)
+{
+    NBDCommand *command = nbd_get_cmd(cmd);
+
+    if (command == NULL || command->read_simple_payload == NULL) {
+        return 0;
+    }
+
+    return command->read_simple_payload(ioc, opaque);
+}
+
+int nbd_handle_structured_error_payload(QIOChannel *ioc, 
NBDStructuredReplyChunk *chunk,
+                                        int *request_ret)
+{
+    uint32_t error;
+    uint16_t message_size;
+    int ret;
+    assert(chunk->type & (1 << 15));
+
+    switch (chunk->type) {
+    case NBD_SREP_TYPE_ERROR:
+    case NBD_SREP_TYPE_ERROR_OFFSET:
+        ret = nbd_read(ioc, &error, sizeof(error), NULL);
+        if (ret < 0) {
+            return ret;
+        }
+        be32_to_cpus(&error);
+
+        ret = nbd_read(ioc, &message_size, sizeof(message_size), NULL);
+        if (ret < 0) {
+            return ret;
+        }
+        be16_to_cpus(&message_size);
+
+        if (message_size > 0) {
+            /* TODO: provide error message to user */
+            ret = nbd_drop(ioc, message_size, NULL);
+            if (ret < 0) {
+                return ret;
+            }
+        }
+
+        if (chunk->type == NBD_SREP_TYPE_ERROR_OFFSET) {
+            /* drop 64bit offset */
+            ret = nbd_drop(ioc, 8, NULL);
+            if (ret < 0) {
+                return ret;
+            }
+        }
+        break;
+    default:
+        /* unknown error */
+        ret = nbd_drop(ioc, chunk->length, NULL);
+        if (ret < 0) {
+            return ret;
+        }
+
+        error = NBD_EINVAL;
+    }
+
+    *request_ret = nbd_errno_to_system_errno(error);
+    return 0;
+}
diff --git a/block/nbd-client.c b/block/nbd-client.c
index e4f0c789f4..cf80564a83 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -179,9 +179,9 @@ err:
      return rc;
  }
-static int nbd_co_receive_reply(NBDClientSession *s,
-                                uint64_t handle,
-                                QEMUIOVector *qiov)
+static int nbd_co_receive_one_chunk(NBDClientSession *s, uint64_t handle,
+                                    int request_cmd, void *request_opaque,
+                                    bool *cont)
  {
      int ret;
      int i = HANDLE_TO_INDEX(s, handle);
@@ -191,29 +191,93 @@ static int nbd_co_receive_reply(NBDClientSession *s,
      qemu_coroutine_yield();
      s->requests[i].receiving = false;
      if (!s->ioc || s->quit) {
-        ret = -EIO;
-    } else {
-        assert(s->reply.handle == handle);
-        ret = -s->reply.error;
-        if (qiov && s->reply.error == 0) {
-            if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
-                                      NULL) < 0) {
-                ret = -EIO;
-                s->quit = true;
+        *cont = false;
+        return -EIO;
+    }
+
+    assert(s->reply.handle == handle);
+
+    if (nbd_reply_is_simple(&s->reply)) {
+        *cont = false;
+        ret = -s->reply.simple.error;
+        if (s->structured_reply && request_cmd == NBD_CMD_READ) {
+            goto fatal;
+        }
+        if (ret == 0) {
+            if (nbd_handle_simple_payload(s->ioc, request_cmd,
+                                          request_opaque) < 0)
+            {
+                goto fatal;
              }
          }
+        goto out;
+    }
+
+    /* handle structured reply chunk */
+
+    *cont = !(s->reply.structured.flags & NBD_SREP_FLAG_DONE);
- /* Tell the read handler to read another header. */
-        s->reply.handle = 0;
+    if (s->reply.structured.type == NBD_SREP_TYPE_NONE) {
+        goto out;
      }
- s->requests[i].coroutine = NULL;
+    if (nbd_srep_type_is_error(s->reply.structured.type)) {
+        if (nbd_handle_structured_error_payload(s->ioc, &s->reply.structured,
+                                                &ret) < 0)
+        {
+            goto fatal;
+        }
+        goto out;
+    }
+
+    /* here we deal with successful not-NONE structured reply */
+    if (nbd_handle_structured_payload(s->ioc, request_cmd,
+                                      &s->reply.structured,
+                                      request_opaque) < 0)
+    {
+        goto fatal;
+    }
+
+out:
+    /* For assert at loop start in nbd_read_reply_entry */
+    s->reply.handle = 0;
- /* Kick the read_reply_co to get the next reply. */
      if (s->read_reply_co) {
          aio_co_wake(s->read_reply_co);
      }
+ return ret;
+
+fatal:
+    /* protocol or ioc failure */
+    *cont = false;
+    s->quit = true;
+    if (s->read_reply_co) {
+        aio_co_wake(s->read_reply_co);
+    }
+
+    return -EIO;
+}
+
+static int nbd_co_receive_reply(NBDClientSession *s,
+                                uint64_t handle,
+                                int request_cmd,
+                                void *request_opaque)
+{
+    int ret = 0;
+    int i = HANDLE_TO_INDEX(s, handle);
+    bool cont = true;
+
+    while (cont) {
+        int rc = nbd_co_receive_one_chunk(s, handle, request_cmd,
+                                          request_opaque, &cont);
+        if (rc < 0 && ret == 0) {
+            ret = rc;
+        }
+    }
+
+    s->requests[i].coroutine = NULL;
+
      qemu_co_mutex_lock(&s->send_mutex);
      s->in_flight--;
      qemu_co_queue_next(&s->free_sema);
@@ -224,22 +288,28 @@ static int nbd_co_receive_reply(NBDClientSession *s,
static int nbd_co_request(BlockDriverState *bs,
                            NBDRequest *request,
-                          QEMUIOVector *qiov)
+                          void *request_opaque)
  {
      NBDClientSession *client = nbd_get_client_session(bs);
      int ret;
+    QEMUIOVector *send_qiov = NULL;
+
+    if (request->type == NBD_CMD_WRITE) {
+        /* TODO: move request sending special casing to nbd-client-cmds.c like
+         * receiving part. */
+        send_qiov = request_opaque;
+        request_opaque = NULL;
+        assert(send_qiov &&
+               request->len == iov_size(send_qiov->iov, send_qiov->niov));
+    }
- assert(!qiov || request->type == NBD_CMD_WRITE ||
-           request->type == NBD_CMD_READ);
-    assert(!qiov || request->len == iov_size(qiov->iov, qiov->niov));
-    ret = nbd_co_send_request(bs, request,
-                              request->type == NBD_CMD_WRITE ? qiov : NULL);
+    ret = nbd_co_send_request(bs, request, send_qiov);
      if (ret < 0) {
          return ret;
      }
- return nbd_co_receive_reply(client, request->handle,
-                                request->type == NBD_CMD_READ ? qiov : NULL);
+    return nbd_co_receive_reply(client, request->handle, request->type,
+                                request_opaque);
  }
int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
diff --git a/nbd/client.c b/nbd/client.c
index 51ae492e92..d0e9b8f138 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -22,38 +22,6 @@
  #include "trace.h"
  #include "nbd-internal.h"
-static int nbd_errno_to_system_errno(int err)
-{
-    int ret;
-    switch (err) {
-    case NBD_SUCCESS:
-        ret = 0;
-        break;
-    case NBD_EPERM:
-        ret = EPERM;
-        break;
-    case NBD_EIO:
-        ret = EIO;
-        break;
-    case NBD_ENOMEM:
-        ret = ENOMEM;
-        break;
-    case NBD_ENOSPC:
-        ret = ENOSPC;
-        break;
-    case NBD_ESHUTDOWN:
-        ret = ESHUTDOWN;
-        break;
-    default:
-        trace_nbd_unknown_error(err);
-        /* fallthrough */
-    case NBD_EINVAL:
-        ret = EINVAL;
-        break;
-    }
-    return ret;
-}
-
  /* Definitions for opaque data types */
static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
@@ -719,6 +687,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char 
*name,
          if (fixedNewStyle) {
              int result;
+ result = nbd_request_simple_option(ioc, NBD_OPT_STRUCTURED_REPLY,
+                                               errp);
+            if (result < 0) {
+                goto fail;
+            }
+            info->structured_reply = result > 0;
+
              /* 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
@@ -759,6 +734,12 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char 
*name,
              goto fail;
          }
          be16_to_cpus(&info->flags);
+
+        if (info->structured_reply && !(info->flags & NBD_CMD_FLAG_DF)) {
+            error_setg(errp, "Structured reply is negotiated, "
+                             "but DF flag is not.");
+            goto fail;
+        }
      } else if (magic == NBD_CLIENT_MAGIC) {
          uint32_t oldflags;
@@ -942,6 +923,46 @@ int nbd_send_request(QIOChannel *ioc, NBDRequest *request)
      return nbd_write(ioc, buf, sizeof(buf), NULL);
  }
+/* nbd_receive_simple_reply
+ * Read simple reply except magic field (which should be already read)
+ */
+static int nbd_receive_simple_reply(QIOChannel *ioc, NBDSimpleReply *reply,
+                                   Error **errp)
+{
+    int ret = nbd_read(ioc, (uint8_t *)reply + sizeof(reply->magic),
+                       sizeof(*reply) - sizeof(reply->magic), errp);
+    if (ret < 0) {
+        return ret;
+    }
+
+    be32_to_cpus(&reply->error);
+    be64_to_cpus(&reply->handle);
+
+    return 0;
+}
+
+/* nbd_receive_structured_reply_chunk
+ * Read structured reply chunk except magic field (which should be already 
read)
+ * Payload is not read.
+ */
+static int nbd_receive_structured_reply_chunk(QIOChannel *ioc,
+                                              NBDStructuredReplyChunk *chunk,
+                                              Error **errp)
+{
+    int ret = nbd_read(ioc, (uint8_t *)chunk + sizeof(chunk->magic),
+                       sizeof(*chunk) - sizeof(chunk->magic), errp);
+    if (ret < 0) {
+        return ret;
+    }
+
+    be16_to_cpus(&chunk->flags);
+    be16_to_cpus(&chunk->type);
+    be64_to_cpus(&chunk->handle);
+    be32_to_cpus(&chunk->length);
+
+    return 0;
+}
+
  /* nbd_receive_reply
   * Returns 1 on success
   *         0 on eof, when no data was read (errp is not set)
@@ -949,39 +970,38 @@ int nbd_send_request(QIOChannel *ioc, NBDRequest *request)
   */
  int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
  {
-    uint8_t buf[NBD_REPLY_SIZE];
-    uint32_t magic;
      int ret;
- ret = nbd_read_eof(ioc, buf, sizeof(buf), errp);
+    ret = nbd_read_eof(ioc, &reply->magic, sizeof(reply->magic), errp);
      if (ret <= 0) {
          return ret;
      }
- /* Reply
-       [ 0 ..  3]    magic   (NBD_SIMPLE_REPLY_MAGIC)
-       [ 4 ..  7]    error   (0 == no error)
-       [ 7 .. 15]    handle
-     */
+    be32_to_cpus(&reply->magic);
- magic = ldl_be_p(buf);
-    reply->error  = ldl_be_p(buf + 4);
-    reply->handle = ldq_be_p(buf + 8);
+    switch (reply->magic) {
+    case NBD_SIMPLE_REPLY_MAGIC:
+        ret = nbd_receive_simple_reply(ioc, &reply->simple, errp);
+        reply->simple.error = nbd_errno_to_system_errno(reply->simple.error);
- reply->error = nbd_errno_to_system_errno(reply->error);
-
-    if (reply->error == ESHUTDOWN) {
-        /* This works even on mingw which lacks a native ESHUTDOWN */
-        error_setg(errp, "server shutting down");
+        if (reply->simple.error == ESHUTDOWN) {
+            /* This works even on mingw which lacks a native ESHUTDOWN */
+            error_setg(errp, "server shutting down");
+            return -EINVAL;
+        }
+        trace_nbd_receive_reply(reply->magic, reply->simple.error,
+                                reply->simple.handle);
+        break;
+    case NBD_STRUCTURED_REPLY_MAGIC:
+        ret = nbd_receive_structured_reply_chunk(ioc, &reply->structured, 
errp);
+        break;
+    default:
+        error_setg(errp, "invalid magic (got 0x%" PRIx32 ")", reply->magic);
          return -EINVAL;
      }
-    trace_nbd_receive_reply(magic, reply->error, reply->handle);
-
-    if (magic != NBD_SIMPLE_REPLY_MAGIC) {
-        error_setg(errp, "invalid magic (got 0x%" PRIx32 ")", magic);
-        return -EINVAL;
+    if (ret < 0) {
+        return ret;
      }
return 1;
  }
-
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 6eaf78a046..c99420f42b 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -12,7 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
  block-obj-y += null.o mirror.o commit.o io.o
  block-obj-y += throttle-groups.o
-block-obj-y += nbd.o nbd-client.o sheepdog.o
+block-obj-y += nbd.o nbd-client.o nbd-client-cmds.o sheepdog.o
  block-obj-$(CONFIG_LIBISCSI) += iscsi.o
  block-obj-$(if $(CONFIG_LIBISCSI),y,n) += iscsi-opts.o
  block-obj-$(CONFIG_LIBNFS) += nfs.o


--
Best regards,
Vladimir




reply via email to

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