qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, libguestfs@redhat.com,
	vsementsov@yandex-team.ru, Kevin Wolf <kwolf@redhat.com>,
	Hanna Reitz <hreitz@redhat.com>
Subject: [PATCH v4 12/24] nbd: Prepare for 64-bit request effect lengths
Date: Thu,  8 Jun 2023 08:56:41 -0500	[thread overview]
Message-ID: <20230608135653.2918540-13-eblake@redhat.com> (raw)
In-Reply-To: <20230608135653.2918540-1-eblake@redhat.com>

Widen the length field of NBDRequest to 64-bits, although we can
assert that all current uses are still under 32 bits, because nothing
ever puts us into NBD_MODE_EXTENDED yet.  Thus no semantic change.  No
semantic change yet.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

v4: split off enum changes to earlier patches [Vladimir]
---
 include/block/nbd.h |  4 ++--
 block/nbd.c         | 25 +++++++++++++++++++------
 nbd/client.c        |  1 +
 nbd/server.c        | 11 ++++++++---
 nbd/trace-events    |  8 ++++----
 5 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index f706e38dc72..dc05f5981fb 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -70,8 +70,8 @@ typedef enum NBDMode {
  */
 typedef struct NBDRequest {
     uint64_t cookie;
-    uint64_t from;
-    uint32_t len;
+    uint64_t from;  /* Offset touched by the command */
+    uint64_t len;   /* Effect length; 32 bit limit without extended headers */
     uint16_t flags; /* NBD_CMD_FLAG_* */
     uint16_t type;  /* NBD_CMD_* */
     NBDMode mode;   /* Determines which network representation to use */
diff --git a/block/nbd.c b/block/nbd.c
index c7581794873..57123c17f94 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1306,10 +1306,11 @@ nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
     NBDRequest request = {
         .type = NBD_CMD_WRITE_ZEROES,
         .from = offset,
-        .len = bytes,  /* .len is uint32_t actually */
+        .len = bytes,
     };

-    assert(bytes <= UINT32_MAX); /* rely on max_pwrite_zeroes */
+    /* rely on max_pwrite_zeroes */
+    assert(bytes <= UINT32_MAX || s->info.mode >= NBD_MODE_EXTENDED);

     assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
     if (!(s->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
@@ -1356,10 +1357,11 @@ nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
     NBDRequest request = {
         .type = NBD_CMD_TRIM,
         .from = offset,
-        .len = bytes, /* len is uint32_t */
+        .len = bytes,
     };

-    assert(bytes <= UINT32_MAX); /* rely on max_pdiscard */
+    /* rely on max_pdiscard */
+    assert(bytes <= UINT32_MAX || s->info.mode >= NBD_MODE_EXTENDED);

     assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
     if (!(s->info.flags & NBD_FLAG_SEND_TRIM) || !bytes) {
@@ -1381,8 +1383,7 @@ static int coroutine_fn GRAPH_RDLOCK nbd_client_co_block_status(
     NBDRequest request = {
         .type = NBD_CMD_BLOCK_STATUS,
         .from = offset,
-        .len = MIN(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment),
-                   MIN(bytes, s->info.size - offset)),
+        .len = MIN(bytes, s->info.size - offset),
         .flags = NBD_CMD_FLAG_REQ_ONE,
     };

@@ -1392,6 +1393,10 @@ static int coroutine_fn GRAPH_RDLOCK nbd_client_co_block_status(
         *file = bs;
         return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
     }
+    if (s->info.mode < NBD_MODE_EXTENDED) {
+        request.len = MIN(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment),
+                          request.len);
+    }

     /*
      * Work around the fact that the block layer doesn't do
@@ -1956,6 +1961,14 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
     bs->bl.max_pwrite_zeroes = max;
     bs->bl.max_transfer = max;

+    /*
+     * Assume that if the server supports extended headers, it also
+     * supports unlimited size zero and trim commands.
+     */
+    if (s->info.mode >= NBD_MODE_EXTENDED) {
+        bs->bl.max_pdiscard = bs->bl.max_pwrite_zeroes = 0;
+    }
+
     if (s->info.opt_block &&
         s->info.opt_block > bs->bl.opt_transfer) {
         bs->bl.opt_transfer = s->info.opt_block;
diff --git a/nbd/client.c b/nbd/client.c
index 40a1eb72346..1495a9b0ab1 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1355,6 +1355,7 @@ int nbd_send_request(QIOChannel *ioc, NBDRequest *request)
     uint8_t buf[NBD_REQUEST_SIZE];

     assert(request->mode <= NBD_MODE_STRUCTURED); /* TODO handle extended */
+    assert(request->len <= UINT32_MAX);
     trace_nbd_send_request(request->from, request->len, request->cookie,
                            request->flags, request->type,
                            nbd_cmd_lookup(request->type));
diff --git a/nbd/server.c b/nbd/server.c
index 9b16f7e5405..4ac05d0cd7b 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1439,7 +1439,7 @@ static int coroutine_fn nbd_receive_request(NBDClient *client, NBDRequest *reque
     request->type   = lduw_be_p(buf + 6);
     request->cookie = ldq_be_p(buf + 8);
     request->from   = ldq_be_p(buf + 16);
-    request->len    = ldl_be_p(buf + 24);
+    request->len    = ldl_be_p(buf + 24); /* widen 32 to 64 bits */

     trace_nbd_receive_request(magic, request->flags, request->type,
                               request->from, request->len);
@@ -2357,7 +2357,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
         request->type == NBD_CMD_CACHE)
     {
         if (request->len > NBD_MAX_BUFFER_SIZE) {
-            error_setg(errp, "len (%" PRIu32" ) is larger than max len (%u)",
+            error_setg(errp, "len (%" PRIu64" ) is larger than max len (%u)",
                        request->len, NBD_MAX_BUFFER_SIZE);
             return -EINVAL;
         }
@@ -2373,6 +2373,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
     }

     if (request->type == NBD_CMD_WRITE) {
+        assert(request->len <= NBD_MAX_BUFFER_SIZE);
         if (nbd_read(client->ioc, req->data, request->len, "CMD_WRITE data",
                      errp) < 0)
         {
@@ -2394,7 +2395,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
     }
     if (request->from > client->exp->size ||
         request->len > client->exp->size - request->from) {
-        error_setg(errp, "operation past EOF; From: %" PRIu64 ", Len: %" PRIu32
+        error_setg(errp, "operation past EOF; From: %" PRIu64 ", Len: %" PRIu64
                    ", Size: %" PRIu64, request->from, request->len,
                    client->exp->size);
         return (request->type == NBD_CMD_WRITE ||
@@ -2456,6 +2457,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
     NBDExport *exp = client->exp;

     assert(request->type == NBD_CMD_READ);
+    assert(request->len <= NBD_MAX_BUFFER_SIZE);

     /* XXX: NBD Protocol only documents use of FUA with WRITE */
     if (request->flags & NBD_CMD_FLAG_FUA) {
@@ -2506,6 +2508,7 @@ static coroutine_fn int nbd_do_cmd_cache(NBDClient *client, NBDRequest *request,
     NBDExport *exp = client->exp;

     assert(request->type == NBD_CMD_CACHE);
+    assert(request->len <= NBD_MAX_BUFFER_SIZE);

     ret = blk_co_preadv(exp->common.blk, request->from, request->len,
                         NULL, BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH);
@@ -2539,6 +2542,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
         if (request->flags & NBD_CMD_FLAG_FUA) {
             flags |= BDRV_REQ_FUA;
         }
+        assert(request->len <= NBD_MAX_BUFFER_SIZE);
         ret = blk_co_pwrite(exp->common.blk, request->from, request->len, data,
                             flags);
         return nbd_send_generic_reply(client, request, ret,
@@ -2582,6 +2586,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
             return nbd_send_generic_reply(client, request, -EINVAL,
                                           "need non-zero length", errp);
         }
+        assert(request->len <= UINT32_MAX);
         if (client->export_meta.count) {
             bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE;
             int contexts_remaining = client->export_meta.count;
diff --git a/nbd/trace-events b/nbd/trace-events
index f19a4d0db39..3338da2be2a 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -31,7 +31,7 @@ nbd_client_loop(void) "Doing NBD loop"
 nbd_client_loop_ret(int ret, const char *error) "NBD loop returned %d: %s"
 nbd_client_clear_queue(void) "Clearing NBD queue"
 nbd_client_clear_socket(void) "Clearing NBD socket"
-nbd_send_request(uint64_t from, uint32_t len, uint64_t cookie, uint16_t flags, uint16_t type, const char *name) "Sending request to server: { .from = %" PRIu64", .len = %" PRIu32 ", .cookie = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) }"
+nbd_send_request(uint64_t from, uint64_t len, uint64_t cookie, uint16_t flags, uint16_t type, const char *name) "Sending request to server: { .from = %" PRIu64", .len = %" PRIu64 ", .cookie = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) }"
 nbd_receive_simple_reply(int32_t error, const char *errname, uint64_t cookie) "Got simple reply: { .error = %" PRId32 " (%s), cookie = %" PRIu64" }"
 nbd_receive_structured_reply_chunk(uint16_t flags, uint16_t type, const char *name, uint64_t cookie, uint32_t length) "Got structured reply chunk: { flags = 0x%" PRIx16 ", type = %d (%s), cookie = %" PRIu64 ", length = %" PRIu32 " }"

@@ -60,7 +60,7 @@ nbd_negotiate_options_check_option(uint32_t option, const char *name) "Checking
 nbd_negotiate_begin(void) "Beginning negotiation"
 nbd_negotiate_new_style_size_flags(uint64_t size, unsigned flags) "advertising size %" PRIu64 " and flags 0x%x"
 nbd_negotiate_success(void) "Negotiation succeeded"
-nbd_receive_request(uint32_t magic, uint16_t flags, uint16_t type, uint64_t from, uint32_t len) "Got request: { magic = 0x%" PRIx32 ", .flags = 0x%" PRIx16 ", .type = 0x%" PRIx16 ", from = %" PRIu64 ", len = %" PRIu32 " }"
+nbd_receive_request(uint32_t magic, uint16_t flags, uint16_t type, uint64_t from, uint64_t len) "Got request: { magic = 0x%" PRIx32 ", .flags = 0x%" PRIx16 ", .type = 0x%" PRIx16 ", from = %" PRIu64 ", len = %" PRIu64 " }"
 nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching clients to AIO context %p"
 nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients from AIO context %p"
 nbd_co_send_simple_reply(uint64_t cookie, uint32_t error, const char *errname, int len) "Send simple reply: cookie = %" PRIu64 ", error = %" PRIu32 " (%s), len = %d"
@@ -70,8 +70,8 @@ nbd_co_send_chunk_read_hole(uint64_t cookie, uint64_t offset, size_t size) "Send
 nbd_co_send_extents(uint64_t cookie, unsigned int extents, uint32_t id, uint64_t length, int last) "Send block status reply: cookie = %" PRIu64 ", extents = %u, context = %d (extents cover %" PRIu64 " bytes, last chunk = %d)"
 nbd_co_send_chunk_error(uint64_t cookie, int err, const char *errname, const char *msg) "Send structured error reply: cookie = %" PRIu64 ", error = %d (%s), msg = '%s'"
 nbd_co_receive_request_decode_type(uint64_t cookie, uint16_t type, const char *name) "Decoding type: cookie = %" PRIu64 ", type = %" PRIu16 " (%s)"
-nbd_co_receive_request_payload_received(uint64_t cookie, uint32_t len) "Payload received: cookie = %" PRIu64 ", len = %" PRIu32
-nbd_co_receive_align_compliance(const char *op, uint64_t from, uint32_t len, uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" PRIx64 ", len=0x%" PRIx32 ", align=0x%" PRIx32
+nbd_co_receive_request_payload_received(uint64_t cookie, uint64_t len) "Payload received: cookie = %" PRIu64 ", len = %" PRIu64
+nbd_co_receive_align_compliance(const char *op, uint64_t from, uint64_t len, uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" PRIx64 ", len=0x%" PRIx64 ", align=0x%" PRIx32
 nbd_trip(void) "Reading request"

 # client-connection.c
-- 
2.40.1



  parent reply	other threads:[~2023-06-08 14:00 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-08 13:56 [PATCH v4 00/24] qemu patches for 64-bit NBD extensions Eric Blake
2023-06-08 13:56 ` [PATCH v4 01/24] nbd/client: Use smarter assert Eric Blake
2023-06-08 13:56 ` [PATCH v4 02/24] nbd: Consistent typedef usage in header Eric Blake
2023-06-08 14:17   ` [Libguestfs] " Eric Blake
2023-06-12 11:59     ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 03/24] nbd/server: Prepare for alternate-size headers Eric Blake
2023-06-12 13:53   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 04/24] nbd/server: Refactor to pass full request around Eric Blake
2023-06-08 13:56 ` [PATCH v4 05/24] nbd: s/handle/cookie/ to match NBD spec Eric Blake
2023-06-08 14:32   ` [Libguestfs] " Eric Blake
2023-06-12 14:12   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 06/24] nbd/client: Simplify cookie vs. index computation Eric Blake
2023-06-12 14:27   ` Vladimir Sementsov-Ogievskiy
2023-06-12 19:13     ` Eric Blake
2023-06-08 13:56 ` [PATCH v4 07/24] nbd/client: Add safety check on chunk payload length Eric Blake
2023-06-08 13:56 ` [PATCH v4 08/24] nbd: Use enum for various negotiation modes Eric Blake
2023-06-12 14:39   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 09/24] nbd: Replace bool structured_reply with mode enum Eric Blake
2023-06-12 15:07   ` Vladimir Sementsov-Ogievskiy
2023-06-12 19:24     ` [Libguestfs] " Eric Blake
2023-07-19 20:11       ` Eric Blake
2023-06-08 13:56 ` [PATCH v4 10/24] nbd/client: Pass mode through to nbd_send_request Eric Blake
2023-06-12 15:48   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 11/24] nbd: Add types for extended headers Eric Blake
2023-06-12 16:11   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` Eric Blake [this message]
2023-06-08 18:26   ` [Libguestfs] [PATCH v4 12/24] nbd: Prepare for 64-bit request effect lengths Eric Blake
2023-06-16 18:16   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 13/24] nbd/server: Refactor handling of request payload Eric Blake
2023-06-08 18:29   ` [Libguestfs] " Eric Blake
2023-06-16 18:29     ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 14/24] nbd/server: Prepare to receive extended header requests Eric Blake
2023-06-16 18:35   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 15/24] nbd/server: Prepare to send extended header replies Eric Blake
2023-06-16 18:48   ` Vladimir Sementsov-Ogievskiy
2023-08-04 19:28     ` Eric Blake
2023-08-07 17:20       ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 16/24] nbd/server: Support 64-bit block status Eric Blake
2023-06-27 13:23   ` Vladimir Sementsov-Ogievskiy
2023-08-04 19:36     ` Eric Blake
2023-08-07 17:28       ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 17/24] nbd/server: Enable initial support for extended headers Eric Blake
2023-06-27 13:26   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 18/24] nbd/client: Plumb errp through nbd_receive_replies Eric Blake
2023-06-08 19:10   ` [Libguestfs] " Eric Blake
2023-06-27 13:31   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 19/24] nbd/client: Initial support for extended headers Eric Blake
2023-06-27 14:22   ` Vladimir Sementsov-Ogievskiy
2023-08-07 19:20     ` Eric Blake
2023-06-08 13:56 ` [PATCH v4 20/24] nbd/client: Accept 64-bit block status chunks Eric Blake
2023-06-27 14:50   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 21/24] nbd/client: Request extended headers during negotiation Eric Blake
2023-06-27 14:55   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 22/24] nbd/server: Refactor list of negotiated meta contexts Eric Blake
2023-06-27 15:11   ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 23/24] nbd/server: Prepare for per-request filtering of BLOCK_STATUS Eric Blake
2023-06-08 19:15   ` [Libguestfs] " Eric Blake
2023-06-27 15:19     ` Vladimir Sementsov-Ogievskiy
2023-06-08 13:56 ` [PATCH v4 24/24] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS Eric Blake
2023-06-08 19:19   ` [Libguestfs] " Eric Blake
2023-06-27 19:42   ` Vladimir Sementsov-Ogievskiy
2023-08-07 20:23     ` [Libguestfs] " Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230608135653.2918540-13-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=libguestfs@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).