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 <ebl...@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