[libnbd,v2,09/23] block_status: Accept 64-bit extents during block status

Commit Message

Eric Blake Nov. 14, 2022, 10:51 p.m. UTC

Support a server giving us a 64-bit extent.  Note that the protocol
says a server should not give a 64-bit answer when extended headers
are not negotiated; we can handle that by reporting EPROTO but
otherwise accepting the information.  Meanwhile, when extended headers
are in effect, even a 32-bit original query can produce a 64-bit
answer; and likewise, a 64-bit query may have so much information that
the server truncates it to a 32-bit answer, so we must be prepared for
either type of response.  Since we already store 64-bit extents
internally, the user's 32-bit callback doesn't have to care which
reply chunk the server uses (the shim takes care of that, and an
upcoming patch adds new APIs to let the client use a 64-bit callback).
Of course, until a later patch enables extended headers negotiation,
no compliant server will trigger the new code here.

Implementation-wise, 'normal' and 'wide' are two different types but
have the same underlying size; keeping the two names makes it easier
to reason about when values are still in network byte order from the
server or native endian for local processing.
---
 lib/internal.h                      |  3 +
 generator/states-reply-structured.c | 88 ++++++++++++++++++++++-------
 2 files changed, 72 insertions(+), 19 deletions(-)

Patch

diff --git a/lib/internal.h b/lib/internal.h
index 0c23f882..b91fe6f6 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -248,6 +248,7 @@  struct nbd_handle {
         struct nbd_structured_reply_offset_data offset_data;
         struct nbd_structured_reply_offset_hole offset_hole;
         struct nbd_structured_reply_block_status_hdr bs_hdr;
+        struct nbd_structured_reply_block_status_ext_hdr bs_ext_hdr;
         struct {
           struct nbd_structured_reply_error error;
           char msg[NBD_MAX_STRING]; /* Common to all error types */
@@ -306,6 +307,8 @@  struct nbd_handle {
     char *storage;      /* malloc's view */
     nbd_extent *normal; /* Our 64-bit preferred internal form; n slots */
     uint32_t *narrow;   /* 32-bit NBD_REPLY_TYPE_BLOCK_STATUS form; n*2 slots */
+    struct nbd_block_descriptor_ext *wide;
+                        /* 64-bit NBD_REPLY_TYPE_BLOCK_STATUS_EXT; n slots */
   } bs_entries;

   /* Commands which are waiting to be issued [meaning the request
diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c
index d23e56a9..7e313b5a 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -22,6 +22,8 @@ 
 #include <stdint.h>
 #include <inttypes.h>

+#include "minmax.h"
+
 /* Structured reply must be completely inside the bounds of the
  * requesting command.
  */
@@ -147,6 +149,24 @@   REPLY.STRUCTURED_REPLY.CHECK:
     /* Start by reading the context ID. */
     h->rbuf = &h->sbuf.reply.payload.bs_hdr;
     h->rlen = sizeof h->sbuf.reply.payload.bs_hdr;
+    h->sbuf.reply.payload.bs_ext_hdr.count = 0;
+    SET_NEXT_STATE (%RECV_BS_HEADER);
+    break;
+
+  case NBD_REPLY_TYPE_BLOCK_STATUS_EXT:
+    if (cmd->type != NBD_CMD_BLOCK_STATUS ||
+        length < 24 ||
+        (length-8) % sizeof(struct nbd_block_descriptor_ext))
+      goto resync;
+    if (!h->extended_headers) {
+      debug (h, "unexpected 64-bit block status without extended headers, "
+             "this is probably a server bug");
+      if (cmd->error == 0)
+        cmd->error = EPROTO;
+    }
+    /* Start by reading the context ID. */
+    h->rbuf = &h->sbuf.reply.payload.bs_ext_hdr;
+    h->rlen = sizeof h->sbuf.reply.payload.bs_ext_hdr;
     SET_NEXT_STATE (%RECV_BS_HEADER);
     break;

@@ -437,6 +457,7 @@   REPLY.STRUCTURED_REPLY.RECV_BS_HEADER:
   struct command *cmd = h->reply_cmd;
   uint32_t length;
   uint32_t count;
+  uint16_t type;

   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
@@ -446,24 +467,44 @@   REPLY.STRUCTURED_REPLY.RECV_BS_HEADER:
     return 0;
   case 0:
     length = h->sbuf.reply.hdr.structured.length; /* normalized in CHECK */
+    type = be16toh (h->sbuf.reply.hdr.structured.type);

     assert (cmd); /* guaranteed by CHECK */
     assert (cmd->type == NBD_CMD_BLOCK_STATUS);
-    assert (length >= 12);
-    length -= sizeof h->sbuf.reply.payload.bs_hdr;
-    count = length / (2 * sizeof (uint32_t));

-    /* Read raw data into a subset of h->bs_entries, then expand it
+    if (type == NBD_REPLY_TYPE_BLOCK_STATUS) {
+      assert (length >= 12);
+      length -= sizeof h->sbuf.reply.payload.bs_hdr;
+      count = length / (2 * sizeof (uint32_t));
+    }
+    else {
+      assert (type == NBD_REPLY_TYPE_BLOCK_STATUS_EXT);
+      assert (length >= 24);
+      length -= sizeof h->sbuf.reply.payload.bs_ext_hdr;
+      count = length / sizeof (struct nbd_block_descriptor_ext);
+      if (h->sbuf.reply.payload.bs_ext_hdr.count &&
+          count != be32toh (h->sbuf.reply.payload.bs_ext_hdr.count)) {
+        h->rbuf = NULL;
+        h->rlen = length;
+        SET_NEXT_STATE (%RESYNC);
+        return 0;
+      }
+    }
+    /* Normalize count for later use. */
+    h->sbuf.reply.payload.bs_ext_hdr.count = count;
+
+    /* Read raw data into an overlap of h->bs_entries, then move it
      * into place later during byte-swapping.
      */
     free (h->bs_entries.storage);
-    h->bs_entries.storage = malloc (count * sizeof *h->bs_entries.normal);
+    h->bs_entries.storage = malloc (MAX (count * sizeof *h->bs_entries.normal,
+                                         length));
     if (h->bs_entries.storage == NULL) {
       SET_NEXT_STATE (%.DEAD);
       set_error (errno, "malloc");
       return 0;
     }
-    h->rbuf = h->bs_entries.narrow;
+    h->rbuf = h->bs_entries.storage;
     h->rlen = length;
     SET_NEXT_STATE (%RECV_BS_ENTRIES);
   }
@@ -471,11 +512,10 @@   REPLY.STRUCTURED_REPLY.RECV_BS_HEADER:

  REPLY.STRUCTURED_REPLY.RECV_BS_ENTRIES:
   struct command *cmd = h->reply_cmd;
-  uint32_t length;
   uint32_t count;
   size_t i;
   uint32_t context_id;
-  uint32_t *raw;
+  uint16_t type;

   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
@@ -484,25 +524,35 @@   REPLY.STRUCTURED_REPLY.RECV_BS_ENTRIES:
     SET_NEXT_STATE (%.READY);
     return 0;
   case 0:
-    length = h->sbuf.reply.hdr.structured.length; /* normalized in CHECK */
+    type = be16toh (h->sbuf.reply.hdr.structured.type);
+    count = h->sbuf.reply.payload.bs_ext_hdr.count; /* normalized in BS_HEADER */

     assert (cmd); /* guaranteed by CHECK */
     assert (cmd->type == NBD_CMD_BLOCK_STATUS);
     assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
-    assert (h->bs_entries.normal);
-    assert (length >= 12);
+    assert (h->bs_entries.normal && count);
     assert (h->meta_valid);
-    count = (length - sizeof h->sbuf.reply.payload.bs_hdr) /
-      (2 * sizeof (uint32_t));

     /* Need to byte-swap the entries returned, but apart from that we
-     * don't validate them.  Reverse order is essential, since we are
-     * expanding in-place from narrow to wider type.
+     * don't validate them.
      */
-    raw = h->bs_entries.narrow;
-    for (i = count; i-- > 0; ) {
-      h->bs_entries.normal[i].flags = be32toh (raw[i * 2 + 1]);
-      h->bs_entries.normal[i].length = be32toh (raw[i * 2]);
+    if (type == NBD_REPLY_TYPE_BLOCK_STATUS) {
+      uint32_t *raw = h->bs_entries.narrow;
+
+      /* Expanding in-place from narrow to wide, must use reverse order. */
+      for (i = count; i-- > 0; ) {
+        h->bs_entries.normal[i].flags = be32toh (raw[i * 2 + 1]);
+        h->bs_entries.normal[i].length = be32toh (raw[i * 2]);
+      }
+    }
+    else {
+      struct nbd_block_descriptor_ext *wide = h->bs_entries.wide;
+
+      assert (type == NBD_REPLY_TYPE_BLOCK_STATUS_EXT);
+      for (i = 0; i < count; i++) {
+        h->bs_entries.normal[i].length = be64toh (wide[i].length);
+        h->bs_entries.normal[i].flags = be64toh (wide[i].status_flags);
+      }
     }

     /* Look up the context ID. */