OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

nbd: Prepare for 64-bit request effect lengths

Browse Source 
Widen the length field of NBDRequest to 64-bits, although we can
assert that all current uses are still under 32 bits: either because
of NBD_MAX_BUFFER_SIZE which is even smaller (and where size_t can
still be appropriate, even on 32-bit platforms), or because nothing
ever puts us into NBD_MODE_EXTENDED yet (and while future patches will
allow larger transactions, the lengths in play here are still capped
at 32-bit).  There are no semantic changes, other than a typo fix in a
couple of error messages.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230829175826.377251-23-eblake@redhat.com>
[eblake: fix assertion bug in nbd_co_send_simple_reply]
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
This commit is contained in:
Eric Blake 2023-08-29 12:58:31 -05:00
parent d95ffb6fe6
commit b257845932
6 changed files with 45 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
block/nbd.c
View File

@ -1305,10 +1305,11 @@ nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
NBDRequest request = { NBDRequest request = {
.type = NBD_CMD_WRITE_ZEROES, .type = NBD_CMD_WRITE_ZEROES,
.from = offset, .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)); assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
if (!(s->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) { if (!(s->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
@ -1355,10 +1356,11 @@ nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
NBDRequest request = { NBDRequest request = {
.type = NBD_CMD_TRIM, .type = NBD_CMD_TRIM,
.from = offset, .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)); assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
if (!(s->info.flags & NBD_FLAG_SEND_TRIM) || !bytes) { if (!(s->info.flags & NBD_FLAG_SEND_TRIM) || !bytes) {
@ -1380,8 +1382,7 @@ static int coroutine_fn GRAPH_RDLOCK nbd_client_co_block_status(
NBDRequest request = { NBDRequest request = {
.type = NBD_CMD_BLOCK_STATUS, .type = NBD_CMD_BLOCK_STATUS,
.from = offset, .from = offset,
.len = MIN(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment), .len = MIN(bytes, s->info.size - offset),
MIN(bytes, s->info.size - offset)),
.flags = NBD_CMD_FLAG_REQ_ONE, .flags = NBD_CMD_FLAG_REQ_ONE,
}; };
@ -1391,6 +1392,10 @@ static int coroutine_fn GRAPH_RDLOCK nbd_client_co_block_status(
*file = bs; *file = bs;
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID; 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 * Work around the fact that the block layer doesn't do
@ -1955,6 +1960,14 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
bs->bl.max_pwrite_zeroes = max; bs->bl.max_pwrite_zeroes = max;
bs->bl.max_transfer = 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 && if (s->info.opt_block &&
s->info.opt_block > bs->bl.opt_transfer) { s->info.opt_block > bs->bl.opt_transfer) {
bs->bl.opt_transfer = s->info.opt_block; bs->bl.opt_transfer = s->info.opt_block;

2
block/trace-events
View File

@ -167,7 +167,7 @@ iscsi_xcopy(void *src_lun, uint64_t src_off, void *dst_lun, uint64_t dst_off, ui
nbd_parse_blockstatus_compliance(const char *err) "ignoring extra data from non-compliant server: %s" nbd_parse_blockstatus_compliance(const char *err) "ignoring extra data from non-compliant server: %s"
nbd_structured_read_compliance(const char *type) "server sent non-compliant unaligned read %s chunk" nbd_structured_read_compliance(const char *type) "server sent non-compliant unaligned read %s chunk"
nbd_read_reply_entry_fail(int ret, const char *err) "ret = %d, err: %s" nbd_read_reply_entry_fail(int ret, const char *err) "ret = %d, err: %s"
nbd_co_request_fail(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name, int ret, const char *err) "Request failed { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) } ret = %d, err: %s" nbd_co_request_fail(uint64_t from, uint64_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name, int ret, const char *err) "Request failed { .from = %" PRIu64", .len = %" PRIu64 ", .handle = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) } ret = %d, err: %s"
nbd_client_handshake(const char *export_name) "export '%s'" nbd_client_handshake(const char *export_name) "export '%s'"
nbd_client_handshake_success(const char *export_name) "export '%s'" nbd_client_handshake_success(const char *export_name) "export '%s'"
nbd_reconnect_attempt(unsigned in_flight) "in_flight %u" nbd_reconnect_attempt(unsigned in_flight) "in_flight %u"

4
include/block/nbd.h
View File

@ -71,8 +71,8 @@ typedef enum NBDMode {
*/ */
typedef struct NBDRequest { typedef struct NBDRequest {
uint64_t cookie; uint64_t cookie;
uint64_t from; uint64_t from; /* Offset touched by the command */
uint32_t len; uint64_t len; /* Effect length; 32 bit limit without extended headers */
uint16_t flags; /* NBD_CMD_FLAG_* */ uint16_t flags; /* NBD_CMD_FLAG_* */
uint16_t type; /* NBD_CMD_* */ uint16_t type; /* NBD_CMD_* */
NBDMode mode; /* Determines which network representation to use */ NBDMode mode; /* Determines which network representation to use */

1
nbd/client.c
View File

@ -1349,6 +1349,7 @@ int nbd_send_request(QIOChannel *ioc, NBDRequest *request)
uint8_t buf[NBD_REQUEST_SIZE]; uint8_t buf[NBD_REQUEST_SIZE];
assert(request->mode <= NBD_MODE_STRUCTURED); /* TODO handle extended */ assert(request->mode <= NBD_MODE_STRUCTURED); /* TODO handle extended */
assert(request->len <= UINT32_MAX);
trace_nbd_send_request(request->from, request->len, request->cookie, trace_nbd_send_request(request->from, request->len, request->cookie,
request->flags, request->type, request->flags, request->type,
nbd_cmd_lookup(request->type)); nbd_cmd_lookup(request->type));

23
nbd/server.c
View File

@ -1165,7 +1165,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
client->optlen = length; client->optlen = length;
if (length > NBD_MAX_BUFFER_SIZE) { if (length > NBD_MAX_BUFFER_SIZE) {
error_setg(errp, "len (%" PRIu32" ) is larger than max len (%u)", error_setg(errp, "len (%" PRIu32 ") is larger than max len (%u)",
length, NBD_MAX_BUFFER_SIZE); length, NBD_MAX_BUFFER_SIZE);
return -EINVAL; return -EINVAL;
} }
@ -1437,7 +1437,7 @@ static int coroutine_fn nbd_receive_request(NBDClient *client, NBDRequest *reque
request->type = lduw_be_p(buf + 6); request->type = lduw_be_p(buf + 6);
request->cookie = ldq_be_p(buf + 8); request->cookie = ldq_be_p(buf + 8);
request->from = ldq_be_p(buf + 16); request->from = ldq_be_p(buf + 16);
request->len = ldl_be_p(buf + 24); request->len = (uint32_t)ldl_be_p(buf + 24); /* widen 32 to 64 bits */
trace_nbd_receive_request(magic, request->flags, request->type, trace_nbd_receive_request(magic, request->flags, request->type,
request->from, request->len); request->from, request->len);
@ -1887,7 +1887,7 @@ static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
NBDRequest *request, NBDRequest *request,
uint32_t error, uint32_t error,
void *data, void *data,
size_t len, uint64_t len,
Error **errp) Error **errp)
{ {
NBDSimpleReply reply; NBDSimpleReply reply;
@ -1898,6 +1898,7 @@ static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
}; };
assert(!len || !nbd_err); assert(!len || !nbd_err);
assert(len <= NBD_MAX_BUFFER_SIZE);
assert(client->mode < NBD_MODE_STRUCTURED || assert(client->mode < NBD_MODE_STRUCTURED ||
(client->mode == NBD_MODE_STRUCTURED && (client->mode == NBD_MODE_STRUCTURED &&
request->type != NBD_CMD_READ)); request->type != NBD_CMD_READ));
@ -1956,7 +1957,7 @@ static int coroutine_fn nbd_co_send_chunk_read(NBDClient *client,
NBDRequest *request, NBDRequest *request,
uint64_t offset, uint64_t offset,
void *data, void *data,
size_t size, uint64_t size,
bool final, bool final,
Error **errp) Error **errp)
{ {
@ -1968,7 +1969,7 @@ static int coroutine_fn nbd_co_send_chunk_read(NBDClient *client,
{.iov_base = data, .iov_len = size} {.iov_base = data, .iov_len = size}
}; };
assert(size); assert(size && size <= NBD_MAX_BUFFER_SIZE);
trace_nbd_co_send_chunk_read(request->cookie, offset, data, size); trace_nbd_co_send_chunk_read(request->cookie, offset, data, size);
set_be_chunk(client, iov, 3, final ? NBD_REPLY_FLAG_DONE : 0, set_be_chunk(client, iov, 3, final ? NBD_REPLY_FLAG_DONE : 0,
NBD_REPLY_TYPE_OFFSET_DATA, request); NBD_REPLY_TYPE_OFFSET_DATA, request);
@ -2011,13 +2012,14 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
NBDRequest *request, NBDRequest *request,
uint64_t offset, uint64_t offset,
uint8_t *data, uint8_t *data,
size_t size, uint64_t size,
Error **errp) Error **errp)
{ {
int ret = 0; int ret = 0;
NBDExport *exp = client->exp; NBDExport *exp = client->exp;
size_t progress = 0; size_t progress = 0;
assert(size <= NBD_MAX_BUFFER_SIZE);
while (progress < size) { while (progress < size) {
int64_t pnum; int64_t pnum;
int status = blk_co_block_status_above(exp->common.blk, NULL, int status = blk_co_block_status_above(exp->common.blk, NULL,
@ -2347,7 +2349,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
request->type == NBD_CMD_CACHE) request->type == NBD_CMD_CACHE)
{ {
if (request->len > NBD_MAX_BUFFER_SIZE) { 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); request->len, NBD_MAX_BUFFER_SIZE);
return -EINVAL; return -EINVAL;
} }
@ -2363,6 +2365,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
} }
if (request->type == NBD_CMD_WRITE) { 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", if (nbd_read(client->ioc, req->data, request->len, "CMD_WRITE data",
errp) < 0) errp) < 0)
{ {
@ -2384,7 +2387,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *
} }
if (request->from > client->exp->size || if (request->from > client->exp->size ||
request->len > client->exp->size - request->from) { 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, ", Size: %" PRIu64, request->from, request->len,
client->exp->size); client->exp->size);
return (request->type == NBD_CMD_WRITE || return (request->type == NBD_CMD_WRITE ||
@ -2446,6 +2449,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
NBDExport *exp = client->exp; NBDExport *exp = client->exp;
assert(request->type == NBD_CMD_READ); assert(request->type == NBD_CMD_READ);
assert(request->len <= NBD_MAX_BUFFER_SIZE);
/* XXX: NBD Protocol only documents use of FUA with WRITE */ /* XXX: NBD Protocol only documents use of FUA with WRITE */
if (request->flags & NBD_CMD_FLAG_FUA) { if (request->flags & NBD_CMD_FLAG_FUA) {
@ -2496,6 +2500,7 @@ static coroutine_fn int nbd_do_cmd_cache(NBDClient *client, NBDRequest *request,
NBDExport *exp = client->exp; NBDExport *exp = client->exp;
assert(request->type == NBD_CMD_CACHE); assert(request->type == NBD_CMD_CACHE);
assert(request->len <= NBD_MAX_BUFFER_SIZE);
ret = blk_co_preadv(exp->common.blk, request->from, request->len, ret = blk_co_preadv(exp->common.blk, request->from, request->len,
NULL, BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH); NULL, BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH);
@ -2529,6 +2534,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (request->flags & NBD_CMD_FLAG_FUA) { if (request->flags & NBD_CMD_FLAG_FUA) {
flags |= BDRV_REQ_FUA; flags |= BDRV_REQ_FUA;
} }
assert(request->len <= NBD_MAX_BUFFER_SIZE);
ret = blk_co_pwrite(exp->common.blk, request->from, request->len, data, ret = blk_co_pwrite(exp->common.blk, request->from, request->len, data,
flags); flags);
return nbd_send_generic_reply(client, request, ret, return nbd_send_generic_reply(client, request, ret,
@ -2572,6 +2578,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
return nbd_send_generic_reply(client, request, -EINVAL, return nbd_send_generic_reply(client, request, -EINVAL,
"need non-zero length", errp); "need non-zero length", errp);
} }
assert(request->len <= UINT32_MAX);
if (client->export_meta.count) { if (client->export_meta.count) {
bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE; bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE;
int contexts_remaining = client->export_meta.count; int contexts_remaining = client->export_meta.count;

14
nbd/trace-events
View File

@ -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_loop_ret(int ret, const char *error) "NBD loop returned %d: %s"
nbd_client_clear_queue(void) "Clearing NBD queue" nbd_client_clear_queue(void) "Clearing NBD queue"
nbd_client_clear_socket(void) "Clearing NBD socket" 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_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 " }" 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,18 +60,18 @@ nbd_negotiate_options_check_option(uint32_t option, const char *name) "Checking
nbd_negotiate_begin(void) "Beginning negotiation" 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_new_style_size_flags(uint64_t size, unsigned flags) "advertising size %" PRIu64 " and flags 0x%x"
nbd_negotiate_success(void) "Negotiation succeeded" 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_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_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" nbd_co_send_simple_reply(uint64_t cookie, uint32_t error, const char *errname, uint64_t len) "Send simple reply: cookie = %" PRIu64 ", error = %" PRIu32 " (%s), len = %" PRIu64
nbd_co_send_chunk_done(uint64_t cookie) "Send structured reply done: cookie = %" PRIu64 nbd_co_send_chunk_done(uint64_t cookie) "Send structured reply done: cookie = %" PRIu64
nbd_co_send_chunk_read(uint64_t cookie, uint64_t offset, void *data, size_t size) "Send structured read data reply: cookie = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu" nbd_co_send_chunk_read(uint64_t cookie, uint64_t offset, void *data, uint64_t size) "Send structured read data reply: cookie = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %" PRIu64
nbd_co_send_chunk_read_hole(uint64_t cookie, uint64_t offset, size_t size) "Send structured read hole reply: cookie = %" PRIu64 ", offset = %" PRIu64 ", len = %zu" nbd_co_send_chunk_read_hole(uint64_t cookie, uint64_t offset, uint64_t size) "Send structured read hole reply: cookie = %" PRIu64 ", offset = %" PRIu64 ", len = %" PRIu64
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_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_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_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_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, 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_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" nbd_trip(void) "Reading request"
# client-connection.c # client-connection.c