Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 00/14] qemu patches for NBD_OPT_EXTENDED_HEADERS
Available at https://repo.or.cz/qemu/ericb.git/shortlog/refs/tags/exthdr-v1 Patch 14 is optional; I'm including it now because I tested with it, but I'm also okay with dropping it based on RFC discussion. Eric Blake (14): nbd/server: Minor cleanups qemu-io: Utilize 64-bit status during map qemu-io: Allow larger write zeroes under no fallback nbd/client: Add safety check on chunk payload length nbd/server: Prepare for alternate-size headers nbd: Prepare for 64-bit requests nbd: Add types for extended headers nbd/server: Initial support for extended headers nbd/server: Support 64-bit block status nbd/client: Initial support for extended headers nbd/client: Accept 64-bit hole chunks nbd/client: Accept 64-bit block status chunks nbd/client: Request extended headers during negotiation do not apply: nbd/server: Send 64-bit hole chunk docs/interop/nbd.txt | 1 + include/block/nbd.h | 94 +++++-- nbd/nbd-internal.h | 8 +- block/nbd.c | 102 +++++-- nbd/client-connection.c | 1 + nbd/client.c | 150 +++++++--- nbd/common.c | 10 +- nbd/server.c | 262 +++++++++++++----- qemu-io-cmds.c | 16 +- qemu-nbd.c | 2 + block/trace-events | 1 + nbd/trace-events | 9 +- tests/qemu-iotests/223.out | 4 + tests/qemu-iotests/233.out | 1 + tests/qemu-iotests/241 | 8 +- tests/qemu-iotests/307 | 2 +- tests/qemu-iotests/307.out | 5 + .../tests/nbd-qemu-allocation.out | 1 + 18 files changed, 486 insertions(+), 191 deletions(-) -- 2.33.1
Spelling fixes, grammar improvements and consistent spacing, noticed
while preparing other patches in this file.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
nbd/server.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 4630dd732250..f302e1cbb03e 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2085,11 +2085,10 @@ static void
nbd_extent_array_convert_to_be(NBDExtentArray *ea)
* Add extent to NBDExtentArray. If extent can't be added (no available
space),
* return -1.
* For safety, when returning -1 for the first time, .can_add is set to false,
- * further call to nbd_extent_array_add() will crash.
- * (to avoid the situation, when after failing to add an extent (returned -1),
- * user miss this failure and add another extent, which is successfully added
- * (array is full, but new extent may be squashed into the last one), then we
- * have invalid array with skipped extent)
+ * and further calls to nbd_extent_array_add() will crash.
+ * (this avoids the situation where a caller ignores failure to add one extent,
+ * where adding another extent that would squash into the last array entry
+ * would result in an incorrect range reported to the client)
*/
static int nbd_extent_array_add(NBDExtentArray *ea,
uint32_t length, uint32_t flags)
@@ -2288,7 +2287,7 @@ static int nbd_co_receive_request(NBDRequestData *req,
NBDRequest *request,
assert(client->recv_coroutine == qemu_coroutine_self());
ret = nbd_receive_request(client, request, errp);
if (ret < 0) {
- return ret;
+ return ret;
}
trace_nbd_co_receive_request_decode_type(request->handle,
request->type,
@@ -2648,7 +2647,7 @@ static coroutine_fn void nbd_trip(void *opaque)
}
if (ret < 0) {
- /* It wans't -EIO, so, according to nbd_co_receive_request()
+ /* It wasn't -EIO, so, according to nbd_co_receive_request()
* semantics, we should return the error to the client. */
Error *export_err = local_err;
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 02/14] qemu-io: Utilize 64-bit status during map
The block layer has supported 64-bit block status from drivers since
commit 86a3d5c688 ("block: Add .bdrv_co_block_status() callback",
v2.12) and friends, with individual driver callbacks responsible for
capping things where necessary. Artificially capping things below 2G
in the qemu-io 'map' command, added in commit d6a644bbfe ("block:
Make
bdrv_is_allocated() byte-based", v2.10) is thus no longer necessary.
One way to test this is with qemu-nbd as server on a raw file larger
than 4G (the entire file should show as allocated), plus 'qemu-io -f
raw -c map nbd://localhost --trace=nbd_\*' as client. Prior to this
patch, the NBD_CMD_BLOCK_STATUS requests are fragmented at 0x7ffffe00
distances; with this patch, the fragmenting changes to 0x7fffffff
(since the NBD protocol is currently still limited to 32-bit
transactions - see block/nbd.c:nbd_client_co_block_status). Then in
later patches, once I add an NBD extension for a 64-bit block status,
the same map command completes with just one NBD_CMD_BLOCK_STATUS.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
qemu-io-cmds.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 46593d632d8f..954955c12fb9 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1993,11 +1993,9 @@ static int map_is_allocated(BlockDriverState *bs, int64_t
offset,
int64_t bytes, int64_t *pnum)
{
int64_t num;
- int num_checked;
int ret, firstret;
- num_checked = MIN(bytes, BDRV_REQUEST_MAX_BYTES);
- ret = bdrv_is_allocated(bs, offset, num_checked, &num);
+ ret = bdrv_is_allocated(bs, offset, bytes, &num);
if (ret < 0) {
return ret;
}
@@ -2009,8 +2007,7 @@ static int map_is_allocated(BlockDriverState *bs, int64_t
offset,
offset += num;
bytes -= num;
- num_checked = MIN(bytes, BDRV_REQUEST_MAX_BYTES);
- ret = bdrv_is_allocated(bs, offset, num_checked, &num);
+ ret = bdrv_is_allocated(bs, offset, bytes, &num);
if (ret == firstret && num) {
*pnum += num;
} else {
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 03/14] qemu-io: Allow larger write zeroes under no fallback
When writing zeroes can fall back to a slow write, permitting an
overly large request can become an amplification denial of service
attack in triggering a large amount of work from a small request. But
the whole point of the no fallback flag is to quickly determine if
writing an entire device to zero can be done quickly (such as when it
is already known that the device started with zero contents); in those
cases, artificially capping things at 2G in qemu-io itself doesn't
help us.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
qemu-io-cmds.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 954955c12fb9..45a957093369 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -603,10 +603,6 @@ static int do_co_pwrite_zeroes(BlockBackend *blk, int64_t
offset,
.done = false,
};
- if (bytes > INT_MAX) {
- return -ERANGE;
- }
-
co = qemu_coroutine_create(co_pwrite_zeroes_entry, &data);
bdrv_coroutine_enter(blk_bs(blk), co);
while (!data.done) {
@@ -1160,8 +1156,9 @@ static int write_f(BlockBackend *blk, int argc, char
**argv)
if (count < 0) {
print_cvtnum_err(count, argv[optind]);
return count;
- } else if (count > BDRV_REQUEST_MAX_BYTES) {
- printf("length cannot exceed %" PRIu64 ", given
%s\n",
+ } else if (count > BDRV_REQUEST_MAX_BYTES &&
+ !(flags & BDRV_REQ_NO_FALLBACK)) {
+ printf("length cannot exceed %" PRIu64 " without -n,
given %s\n",
(uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]);
return -EINVAL;
}
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 04/14] nbd/client: Add safety check on chunk payload length
Our existing use of structured replies either reads into a qiov capped
at 32M (NBD_CMD_READ) or caps allocation to 1000 bytes (see
NBD_MAX_MALLOC_PAYLOAD in block/nbd.c). But the existing length
checks are rather late; if we encounter a buggy (or malicious) server
that sends a super-large payload length, we should drop the connection
right then rather than assuming the layer on top will be careful.
This becomes more important when we permit 64-bit lengths which are
even more likely to have the potential for attempted denial of service
abuse.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
nbd/client.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/nbd/client.c b/nbd/client.c
index 30d5383cb195..8f137c2320bb 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1412,6 +1412,18 @@ static int nbd_receive_structured_reply_chunk(QIOChannel
*ioc,
chunk->handle = be64_to_cpu(chunk->handle);
chunk->length = be32_to_cpu(chunk->length);
+ /*
+ * Because we use BLOCK_STATUS with REQ_ONE, and cap READ requests
+ * at 32M, no valid server should send us payload larger than
+ * this. Even if we stopped using REQ_ONE, sane servers will cap
+ * the number of extents they return for block status.
+ */
+ if (chunk->length > NBD_MAX_BUFFER_SIZE +
sizeof(NBDStructuredReadData)) {
+ error_setg(errp, "server chunk %" PRIu32 " (%s) payload
is too long",
+ chunk->type, nbd_rep_lookup(chunk->type));
+ return -EINVAL;
+ }
+
return 0;
}
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 05/14] nbd/server: Prepare for alternate-size headers
An upcoming NBD extension wants to add the ability to do 64-bit
requests. As part of that extension, the size of the reply headers
will change in order to permit a 64-bit length in the reply for
symmetry [*]. Additionally, where the reply header is currently 16
bytes for simple reply, and 20 bytes for structured reply; with the
extension enabled, both reply type headers will be 32 bytes. Since we
are already wired up to use iovecs, it is easiest to allow for this
change in header size by splitting each structured reply across two
iovecs, one for the header (which will become variable-length in a
future patch according to client negotiation), and the other for the
payload, and removing the header from the payload struct definitions.
Interestingly, the client side code never utilized the packed types,
so only the server code needs to be updated.
[*] Note that on the surface, this is because some server might permit
a 4G+ NBD_CMD_READ and need to reply with that much data in one
transaction. But even though the extended reply length is widened to
64 bits, we will still never send a reply payload larger than just
over 32M (the maximum buffer we allow in NBD_CMD_READ; and we cap the
number of extents we are willing to report in NBD_CMD_BLOCK_STATUS).
Where 64-bit fields really matter in the extension is in a later patch
adding 64-bit support into a counterpart for REPLY_TYPE_BLOCK_STATUS.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
include/block/nbd.h | 10 +++----
nbd/server.c | 64 ++++++++++++++++++++++++++++-----------------
2 files changed, 45 insertions(+), 29 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 78d101b77488..3d0689b69367 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2020 Red Hat, Inc.
+ * Copyright (C) 2016-2021 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <anthony at codemonkey.ws>
*
* Network Block Device
@@ -95,28 +95,28 @@ typedef union NBDReply {
/* Header of chunk for NBD_REPLY_TYPE_OFFSET_DATA */
typedef struct NBDStructuredReadData {
- NBDStructuredReplyChunk h; /* h.length >= 9 */
+ /* header's .length >= 9 */
uint64_t offset;
/* At least one byte of data payload follows, calculated from h.length */
} QEMU_PACKED NBDStructuredReadData;
/* Complete chunk for NBD_REPLY_TYPE_OFFSET_HOLE */
typedef struct NBDStructuredReadHole {
- NBDStructuredReplyChunk h; /* h.length == 12 */
+ /* header's length == 12 */
uint64_t offset;
uint32_t length;
} QEMU_PACKED NBDStructuredReadHole;
/* Header of all NBD_REPLY_TYPE_ERROR* errors */
typedef struct NBDStructuredError {
- NBDStructuredReplyChunk h; /* h.length >= 6 */
+ /* header's length >= 6 */
uint32_t error;
uint16_t message_length;
} QEMU_PACKED NBDStructuredError;
/* Header of NBD_REPLY_TYPE_BLOCK_STATUS */
typedef struct NBDStructuredMeta {
- NBDStructuredReplyChunk h; /* h.length >= 12 (at least one extent) */
+ /* header's length >= 12 (at least one extent) */
uint32_t context_id;
/* extents follows */
} QEMU_PACKED NBDStructuredMeta;
diff --git a/nbd/server.c b/nbd/server.c
index f302e1cbb03e..64845542fd6b 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1869,9 +1869,12 @@ static int coroutine_fn nbd_co_send_iov(NBDClient
*client, struct iovec *iov,
return ret;
}
-static inline void set_be_simple_reply(NBDSimpleReply *reply, uint64_t error,
- uint64_t handle)
+static inline void set_be_simple_reply(NBDClient *client, struct iovec *iov,
+ uint64_t error, uint64_t handle)
{
+ NBDSimpleReply *reply = iov->iov_base;
+
+ iov->iov_len = sizeof(*reply);
stl_be_p(&reply->magic, NBD_SIMPLE_REPLY_MAGIC);
stl_be_p(&reply->error, error);
stq_be_p(&reply->handle, handle);
@@ -1884,23 +1887,27 @@ static int nbd_co_send_simple_reply(NBDClient *client,
size_t len,
Error **errp)
{
- NBDSimpleReply reply;
+ NBDReply hdr;
int nbd_err = system_errno_to_nbd_errno(error);
struct iovec iov[] = {
- {.iov_base = &reply, .iov_len = sizeof(reply)},
+ {.iov_base = &hdr},
{.iov_base = data, .iov_len = len}
};
trace_nbd_co_send_simple_reply(handle, nbd_err, nbd_err_lookup(nbd_err),
len);
- set_be_simple_reply(&reply, nbd_err, handle);
+ set_be_simple_reply(client, &iov[0], nbd_err, handle);
return nbd_co_send_iov(client, iov, len ? 2 : 1, errp);
}
-static inline void set_be_chunk(NBDStructuredReplyChunk *chunk, uint16_t flags,
- uint16_t type, uint64_t handle, uint32_t
length)
+static inline void set_be_chunk(NBDClient *client, struct iovec *iov,
+ uint16_t flags, uint16_t type,
+ uint64_t handle, uint32_t length)
{
+ NBDStructuredReplyChunk *chunk = iov->iov_base;
+
+ iov->iov_len = sizeof(*chunk);
stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC);
stw_be_p(&chunk->flags, flags);
stw_be_p(&chunk->type, type);
@@ -1912,13 +1919,14 @@ static int coroutine_fn
nbd_co_send_structured_done(NBDClient *client,
uint64_t handle,
Error **errp)
{
- NBDStructuredReplyChunk chunk;
+ NBDReply hdr;
struct iovec iov[] = {
- {.iov_base = &chunk, .iov_len = sizeof(chunk)},
+ {.iov_base = &hdr},
};
trace_nbd_co_send_structured_done(handle);
- set_be_chunk(&chunk, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_NONE, handle,
0);
+ set_be_chunk(client, &iov[0], NBD_REPLY_FLAG_DONE,
+ NBD_REPLY_TYPE_NONE, handle, 0);
return nbd_co_send_iov(client, iov, 1, errp);
}
@@ -1931,20 +1939,21 @@ static int coroutine_fn
nbd_co_send_structured_read(NBDClient *client,
bool final,
Error **errp)
{
+ NBDReply hdr;
NBDStructuredReadData chunk;
struct iovec iov[] = {
+ {.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
{.iov_base = data, .iov_len = size}
};
assert(size);
trace_nbd_co_send_structured_read(handle, offset, data, size);
- set_be_chunk(&chunk.h, final ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_OFFSET_DATA, handle,
- sizeof(chunk) - sizeof(chunk.h) + size);
+ set_be_chunk(client, &iov[0], final ? NBD_REPLY_FLAG_DONE : 0,
+ NBD_REPLY_TYPE_OFFSET_DATA, handle, iov[1].iov_len + size);
stq_be_p(&chunk.offset, offset);
- return nbd_co_send_iov(client, iov, 2, errp);
+ return nbd_co_send_iov(client, iov, 3, errp);
}
static int coroutine_fn nbd_co_send_structured_error(NBDClient *client,
@@ -1953,9 +1962,11 @@ static int coroutine_fn
nbd_co_send_structured_error(NBDClient *client,
const char *msg,
Error **errp)
{
+ NBDReply hdr;
NBDStructuredError chunk;
int nbd_err = system_errno_to_nbd_errno(error);
struct iovec iov[] = {
+ {.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
{.iov_base = (char *)msg, .iov_len = msg ? strlen(msg) : 0},
};
@@ -1963,12 +1974,12 @@ static int coroutine_fn
nbd_co_send_structured_error(NBDClient *client,
assert(nbd_err);
trace_nbd_co_send_structured_error(handle, nbd_err,
nbd_err_lookup(nbd_err), msg ? msg :
"");
- set_be_chunk(&chunk.h, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_ERROR,
handle,
- sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len);
+ set_be_chunk(client, &iov[0], NBD_REPLY_FLAG_DONE,
+ NBD_REPLY_TYPE_ERROR, handle, iov[1].iov_len +
iov[2].iov_len);
stl_be_p(&chunk.error, nbd_err);
- stw_be_p(&chunk.message_length, iov[1].iov_len);
+ stw_be_p(&chunk.message_length, iov[2].iov_len);
- return nbd_co_send_iov(client, iov, 1 + !!iov[1].iov_len, errp);
+ return nbd_co_send_iov(client, iov, 2 + !!iov[1].iov_len, errp);
}
/* Do a sparse read and send the structured reply to the client.
@@ -2006,19 +2017,22 @@ static int coroutine_fn
nbd_co_send_sparse_read(NBDClient *client,
assert(pnum && pnum <= size - progress);
final = progress + pnum == size;
if (status & BDRV_BLOCK_ZERO) {
+ NBDReply hdr;
NBDStructuredReadHole chunk;
struct iovec iov[] = {
+ {.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
};
trace_nbd_co_send_structured_read_hole(handle, offset + progress,
pnum);
- set_be_chunk(&chunk.h, final ? NBD_REPLY_FLAG_DONE : 0,
+ set_be_chunk(client, &iov[0],
+ final ? NBD_REPLY_FLAG_DONE : 0,
NBD_REPLY_TYPE_OFFSET_HOLE,
- handle, sizeof(chunk) - sizeof(chunk.h));
+ handle, iov[1].iov_len);
stq_be_p(&chunk.offset, offset + progress);
stl_be_p(&chunk.length, pnum);
- ret = nbd_co_send_iov(client, iov, 1, errp);
+ ret = nbd_co_send_iov(client, iov, 2, errp);
} else {
ret = blk_pread(exp->common.blk, offset + progress,
data + progress, pnum);
@@ -2182,8 +2196,10 @@ static int nbd_co_send_extents(NBDClient *client,
uint64_t handle,
NBDExtentArray *ea,
bool last, uint32_t context_id, Error **errp)
{
+ NBDReply hdr;
NBDStructuredMeta chunk;
struct iovec iov[] = {
+ {.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
{.iov_base = ea->extents, .iov_len = ea->count *
sizeof(ea->extents[0])}
};
@@ -2192,12 +2208,12 @@ static int nbd_co_send_extents(NBDClient *client,
uint64_t handle,
trace_nbd_co_send_extents(handle, ea->count, context_id,
ea->total_length,
last);
- set_be_chunk(&chunk.h, last ? NBD_REPLY_FLAG_DONE : 0,
+ set_be_chunk(client, &iov[0], last ? NBD_REPLY_FLAG_DONE : 0,
NBD_REPLY_TYPE_BLOCK_STATUS,
- handle, sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len);
+ handle, iov[1].iov_len + iov[2].iov_len);
stl_be_p(&chunk.context_id, context_id);
- return nbd_co_send_iov(client, iov, 2, errp);
+ return nbd_co_send_iov(client, iov, 3, errp);
}
/* Get block status from the exported device and send it to the client */
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 06/14] nbd: Prepare for 64-bit requests
Widen the length field of NBDRequest to 64-bits, although we can
assert that all current uses are still under 32 bits. Move the
request magic number to nbd.h, to live alongside the reply magic
number. Add a bool that will eventually track whether the client
successfully negotiated extended headers with the server, allowing the
nbd driver to pass larger requests along where possible; although in
this patch it always remains false for no semantic change yet.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
include/block/nbd.h | 19 +++++++++++--------
nbd/nbd-internal.h | 3 +--
block/nbd.c | 35 ++++++++++++++++++++++++-----------
nbd/client.c | 8 +++++---
nbd/server.c | 11 ++++++++---
nbd/trace-events | 8 ++++----
6 files changed, 53 insertions(+), 31 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 3d0689b69367..732314aaba11 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -52,17 +52,16 @@ typedef struct NBDOptionReplyMetaContext {
/* Transmission phase structs
*
- * Note: these are _NOT_ the same as the network representation of an NBD
- * request and reply!
+ * Note: NBDRequest is _NOT_ the same as the network representation of an NBD
+ * request!
*/
-struct NBDRequest {
+typedef struct NBDRequest {
uint64_t handle;
uint64_t from;
- uint32_t len;
+ uint64_t len; /* Must fit 32 bits unless extended headers negotiated */
uint16_t flags; /* NBD_CMD_FLAG_* */
- uint16_t type; /* NBD_CMD_* */
-};
-typedef struct NBDRequest NBDRequest;
+ uint16_t type; /* NBD_CMD_* */
+} NBDRequest;
typedef struct NBDSimpleReply {
uint32_t magic; /* NBD_SIMPLE_REPLY_MAGIC */
@@ -235,6 +234,9 @@ enum {
*/
#define NBD_MAX_STRING_SIZE 4096
+/* Transmission request structure */
+#define NBD_REQUEST_MAGIC 0x25609513
+
/* Two types of reply structures */
#define NBD_SIMPLE_REPLY_MAGIC 0x67446698
#define NBD_STRUCTURED_REPLY_MAGIC 0x668e33ef
@@ -293,6 +295,7 @@ struct NBDExportInfo {
/* In-out fields, set by client before nbd_receive_negotiate() and
* updated by server results during nbd_receive_negotiate() */
bool structured_reply;
+ bool extended_headers;
bool base_allocation; /* base:allocation context for NBD_CMD_BLOCK_STATUS
*/
/* Set by server results during nbd_receive_negotiate() and
@@ -322,7 +325,7 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds
*tlscreds,
Error **errp);
int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info,
Error **errp);
-int nbd_send_request(QIOChannel *ioc, NBDRequest *request);
+int nbd_send_request(QIOChannel *ioc, NBDRequest *request, bool ext_hdr);
int coroutine_fn nbd_receive_reply(BlockDriverState *bs, QIOChannel *ioc,
NBDReply *reply, Error **errp);
int nbd_client(int fd);
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 1b2141ab4b2d..0016793ff4b1 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -1,7 +1,7 @@
/*
* NBD Internal Declarations
*
- * Copyright (C) 2016 Red Hat, Inc.
+ * Copyright (C) 2016-2021 Red Hat, Inc.
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -45,7 +45,6 @@
#define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124)
#define NBD_INIT_MAGIC 0x4e42444d41474943LL /* ASCII
"NBDMAGIC" */
-#define NBD_REQUEST_MAGIC 0x25609513
#define NBD_OPTS_MAGIC 0x49484156454F5054LL /* ASCII
"IHAVEOPT" */
#define NBD_CLIENT_MAGIC 0x0000420281861253LL
#define NBD_REP_MAGIC 0x0003e889045565a9LL
diff --git a/block/nbd.c b/block/nbd.c
index 5ef462db1b7f..3e9875241bec 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -2,7 +2,7 @@
* QEMU Block driver for NBD
*
* Copyright (c) 2019 Virtuozzo International GmbH.
- * Copyright (C) 2016 Red Hat, Inc.
+ * Copyright (C) 2016-2021 Red Hat, Inc.
* Copyright (C) 2008 Bull S.A.S.
* Author: Laurent Vivier <Laurent.Vivier at bull.net>
*
@@ -300,7 +300,7 @@ int coroutine_fn
nbd_co_do_establish_connection(BlockDriverState *bs,
*/
NBDRequest request = { .type = NBD_CMD_DISC };
- nbd_send_request(s->ioc, &request);
+ nbd_send_request(s->ioc, &request, s->info.extended_headers);
yank_unregister_function(BLOCKDEV_YANK_INSTANCE(s->bs->node_name),
nbd_yank, bs);
@@ -470,7 +470,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
if (qiov) {
qio_channel_set_cork(s->ioc, true);
- rc = nbd_send_request(s->ioc, request);
+ rc = nbd_send_request(s->ioc, request, s->info.extended_headers);
if (nbd_client_connected(s) && rc >= 0) {
if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
NULL) < 0) {
@@ -481,7 +481,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
}
qio_channel_set_cork(s->ioc, false);
} else {
- rc = nbd_send_request(s->ioc, request);
+ rc = nbd_send_request(s->ioc, request, s->info.extended_headers);
}
err:
@@ -1252,10 +1252,11 @@ static int nbd_client_co_pwrite_zeroes(BlockDriverState
*bs, int64_t offset,
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.extended_headers);
assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
if (!(s->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
@@ -1302,10 +1303,11 @@ static int nbd_client_co_pdiscard(BlockDriverState *bs,
int64_t offset,
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.extended_headers);
assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
if (!(s->info.flags & NBD_FLAG_SEND_TRIM) || !bytes) {
@@ -1327,8 +1329,7 @@ static int coroutine_fn 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,
};
@@ -1338,6 +1339,10 @@ static int coroutine_fn nbd_client_co_block_status(
*file = bs;
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
}
+ if (!s->info.extended_headers) {
+ 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
@@ -1415,7 +1420,7 @@ static void nbd_client_close(BlockDriverState *bs)
NBDRequest request = { .type = NBD_CMD_DISC };
if (s->ioc) {
- nbd_send_request(s->ioc, &request);
+ nbd_send_request(s->ioc, &request, s->info.extended_headers);
}
nbd_teardown_connection(bs);
@@ -1877,6 +1882,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.extended_headers) {
+ 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 8f137c2320bb..aa162b9d08d5 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2019 Red Hat, Inc.
+ * Copyright (C) 2016-2021 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori <anthony at codemonkey.ws>
*
* Network Block Device Client Side
@@ -1221,7 +1221,7 @@ int nbd_receive_export_list(QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
if (nbd_drop(ioc, 124, NULL) == 0) {
NBDRequest request = { .type = NBD_CMD_DISC };
- nbd_send_request(ioc, &request);
+ nbd_send_request(ioc, &request, false);
}
break;
default:
@@ -1345,10 +1345,12 @@ int nbd_disconnect(int fd)
#endif /* __linux__ */
-int nbd_send_request(QIOChannel *ioc, NBDRequest *request)
+int nbd_send_request(QIOChannel *ioc, NBDRequest *request, bool ext_hdr)
{
uint8_t buf[NBD_REQUEST_SIZE];
+ assert(!ext_hdr);
+ assert(request->len <= UINT32_MAX);
trace_nbd_send_request(request->from, request->len,
request->handle,
request->flags, request->type,
nbd_cmd_lookup(request->type));
diff --git a/nbd/server.c b/nbd/server.c
index 64845542fd6b..4306fa7b426c 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1436,7 +1436,7 @@ static int nbd_receive_request(NBDClient *client,
NBDRequest *request,
request->type = lduw_be_p(buf + 6);
request->handle = 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);
@@ -2324,7 +2324,7 @@ static int nbd_co_receive_request(NBDRequestData *req,
NBDRequest *request,
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;
}
@@ -2340,6 +2340,7 @@ static int nbd_co_receive_request(NBDRequestData *req,
NBDRequest *request,
}
if (request->type == NBD_CMD_WRITE) {
+ assert(request->len <= UINT32_MAX);
if (nbd_read(client->ioc, req->data, request->len,
"CMD_WRITE data",
errp) < 0)
{
@@ -2361,7 +2362,7 @@ static int nbd_co_receive_request(NBDRequestData *req,
NBDRequest *request,
}
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 ||
@@ -2424,6 +2425,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 <= INT_MAX);
/* XXX: NBD Protocol only documents use of FUA with WRITE */
if (request->flags & NBD_CMD_FLAG_FUA) {
@@ -2475,6 +2477,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 <= INT_MAX);
ret = blk_co_preadv(exp->common.blk, request->from, request->len,
NULL, BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH);
@@ -2508,6 +2511,7 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
if (request->flags & NBD_CMD_FLAG_FUA) {
flags |= BDRV_REQ_FUA;
}
+ assert(request->len <= INT_MAX);
ret = blk_pwrite(exp->common.blk, request->from, data,
request->len,
flags);
return nbd_send_generic_reply(client, request->handle, ret,
@@ -2551,6 +2555,7 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
return nbd_send_generic_reply(client, request->handle, -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 c4919a2dd581..d18da8b0b743 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 handle, uint16_t flags,
uint16_t type, const char *name) "Sending request to server: { .from =
%" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64
", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s)
}"
+nbd_send_request(uint64_t from, uint64_t len, uint64_t handle, uint16_t flags,
uint16_t type, const char *name) "Sending request to server: { .from =
%" PRIu64", .len = %" PRIu64 ", .handle = %" PRIu64
", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s)
}"
nbd_receive_simple_reply(int32_t error, const char *errname, uint64_t handle)
"Got simple reply: { .error = %" PRId32 " (%s), handle = %"
PRIu64" }"
nbd_receive_structured_reply_chunk(uint16_t flags, uint16_t type, const char
*name, uint64_t handle, uint32_t length) "Got structured reply chunk: {
flags = 0x%" PRIx16 ", type = %d (%s), handle = %" 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 handle, uint32_t error, const char *errname,
int len) "Send simple reply: handle = %" PRIu64 ", error =
%" PRIu32 " (%s), len = %d"
@@ -70,6 +70,6 @@ nbd_co_send_structured_read_hole(uint64_t handle, uint64_t
offset, size_t size)
nbd_co_send_extents(uint64_t handle, unsigned int extents, uint32_t id,
uint64_t length, int last) "Send block status reply: handle = %"
PRIu64 ", extents = %u, context = %d (extents cover %" PRIu64 "
bytes, last chunk = %d)"
nbd_co_send_structured_error(uint64_t handle, int err, const char *errname,
const char *msg) "Send structured error reply: handle = %" PRIu64
", error = %d (%s), msg = '%s'"
nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char
*name) "Decoding type: handle = %" PRIu64 ", type = %"
PRIu16 " (%s)"
-nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len)
"Payload received: handle = %" 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 handle, uint64_t len)
"Payload received: handle = %" 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"
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 07/14] nbd: Add types for extended headers
Add the constants and structs necessary for later patches to start
implementing the NBD_OPT_EXTENDED_HEADERS extension in both the client
and server. This patch does not change any existing behavior, but
merely sets the stage.
This patch does not change the status quo that neither the client nor
server use a packed-struct representation for the request header.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
docs/interop/nbd.txt | 1 +
include/block/nbd.h | 67 +++++++++++++++++++++++++++++++++++---------
nbd/common.c | 10 +++++--
3 files changed, 62 insertions(+), 16 deletions(-)
diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
index bdb0f2a41aca..6229ea573c04 100644
--- a/docs/interop/nbd.txt
+++ b/docs/interop/nbd.txt
@@ -68,3 +68,4 @@ NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:",
NBD_CMD_CACHE
* 4.2: NBD_FLAG_CAN_MULTI_CONN for shareable read-only exports,
NBD_CMD_FLAG_FAST_ZERO
* 5.2: NBD_CMD_BLOCK_STATUS for "qemu:allocation-depth"
+* 7.0: NBD_OPT_EXTENDED_HEADERS
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 732314aaba11..5f9d86a86352 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -69,6 +69,14 @@ typedef struct NBDSimpleReply {
uint64_t handle;
} QEMU_PACKED NBDSimpleReply;
+typedef struct NBDSimpleReplyExt {
+ uint32_t magic; /* NBD_SIMPLE_REPLY_EXT_MAGIC */
+ uint32_t error;
+ uint64_t handle;
+ uint64_t _pad1; /* Must be 0 */
+ uint64_t _pad2; /* Must be 0 */
+} QEMU_PACKED NBDSimpleReplyExt;
+
/* Header of all structured replies */
typedef struct NBDStructuredReplyChunk {
uint32_t magic; /* NBD_STRUCTURED_REPLY_MAGIC */
@@ -78,9 +86,20 @@ typedef struct NBDStructuredReplyChunk {
uint32_t length; /* length of payload */
} QEMU_PACKED NBDStructuredReplyChunk;
+typedef struct NBDStructuredReplyChunkExt {
+ uint32_t magic; /* NBD_STRUCTURED_REPLY_EXT_MAGIC */
+ uint16_t flags; /* combination of NBD_REPLY_FLAG_* */
+ uint16_t type; /* NBD_REPLY_TYPE_* */
+ uint64_t handle; /* request handle */
+ uint64_t length; /* length of payload */
+ uint64_t _pad; /* Must be 0 */
+} QEMU_PACKED NBDStructuredReplyChunkExt;
+
typedef union NBDReply {
NBDSimpleReply simple;
+ NBDSimpleReplyExt simple_ext;
NBDStructuredReplyChunk structured;
+ NBDStructuredReplyChunkExt structured_ext;
struct {
/* @magic and @handle fields have the same offset and size both in
* simple reply and structured reply chunk, so let them be accessible
@@ -106,6 +125,13 @@ typedef struct NBDStructuredReadHole {
uint32_t length;
} QEMU_PACKED NBDStructuredReadHole;
+/* Complete chunk for NBD_REPLY_TYPE_OFFSET_HOLE_EXT */
+typedef struct NBDStructuredReadHoleExt {
+ /* header's length == 16 */
+ uint64_t offset;
+ uint64_t length;
+} QEMU_PACKED NBDStructuredReadHoleExt;
+
/* Header of all NBD_REPLY_TYPE_ERROR* errors */
typedef struct NBDStructuredError {
/* header's length >= 6 */
@@ -113,19 +139,26 @@ typedef struct NBDStructuredError {
uint16_t message_length;
} QEMU_PACKED NBDStructuredError;
-/* Header of NBD_REPLY_TYPE_BLOCK_STATUS */
+/* Header of NBD_REPLY_TYPE_BLOCK_STATUS, NBD_REPLY_TYPE_BLOCK_STATUS_EXT */
typedef struct NBDStructuredMeta {
- /* header's length >= 12 (at least one extent) */
+ /* header's length >= 12 narrow, or >= 20 extended (at least one
extent) */
uint32_t context_id;
- /* extents follows */
+ /* extents[] follows: NBDExtent for narrow, NBDExtentExt for extended */
} QEMU_PACKED NBDStructuredMeta;
-/* Extent chunk for NBD_REPLY_TYPE_BLOCK_STATUS */
+/* Extent array for NBD_REPLY_TYPE_BLOCK_STATUS */
typedef struct NBDExtent {
uint32_t length;
uint32_t flags; /* NBD_STATE_* */
} QEMU_PACKED NBDExtent;
+/* Extent array for NBD_REPLY_TYPE_BLOCK_STATUS_EXT */
+typedef struct NBDExtentExt {
+ uint64_t length;
+ uint32_t flags; /* NBD_STATE_* */
+ uint32_t _pad; /* Must be 0 */
+} QEMU_PACKED NBDExtentExt;
+
/* Transmission (export) flags: sent from server to client during handshake,
but describe what will happen during transmission */
enum {
@@ -178,6 +211,7 @@ enum {
#define NBD_OPT_STRUCTURED_REPLY (8)
#define NBD_OPT_LIST_META_CONTEXT (9)
#define NBD_OPT_SET_META_CONTEXT (10)
+#define NBD_OPT_EXTENDED_HEADERS (11)
/* Option reply types. */
#define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value))
@@ -234,12 +268,15 @@ enum {
*/
#define NBD_MAX_STRING_SIZE 4096
-/* Transmission request structure */
+/* Two types of request structures, a given client will only use 1 */
#define NBD_REQUEST_MAGIC 0x25609513
+#define NBD_REQUEST_EXT_MAGIC 0x21e41c71
-/* Two types of reply structures */
-#define NBD_SIMPLE_REPLY_MAGIC 0x67446698
-#define NBD_STRUCTURED_REPLY_MAGIC 0x668e33ef
+/* Four types of reply structures, a given client will only use 2 */
+#define NBD_SIMPLE_REPLY_MAGIC 0x67446698
+#define NBD_STRUCTURED_REPLY_MAGIC 0x668e33ef
+#define NBD_SIMPLE_REPLY_EXT_MAGIC 0x60d12fd6
+#define NBD_STRUCTURED_REPLY_EXT_MAGIC 0x6e8a278c
/* Structured reply flags */
#define NBD_REPLY_FLAG_DONE (1 << 0) /* This reply-chunk is last
*/
@@ -247,12 +284,14 @@ enum {
/* Structured reply types */
#define NBD_REPLY_ERR(value) ((1 << 15) | (value))
-#define NBD_REPLY_TYPE_NONE 0
-#define NBD_REPLY_TYPE_OFFSET_DATA 1
-#define NBD_REPLY_TYPE_OFFSET_HOLE 2
-#define NBD_REPLY_TYPE_BLOCK_STATUS 5
-#define NBD_REPLY_TYPE_ERROR NBD_REPLY_ERR(1)
-#define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_ERR(2)
+#define NBD_REPLY_TYPE_NONE 0
+#define NBD_REPLY_TYPE_OFFSET_DATA 1
+#define NBD_REPLY_TYPE_OFFSET_HOLE 2
+#define NBD_REPLY_TYPE_OFFSET_HOLE_EXT 3
+#define NBD_REPLY_TYPE_BLOCK_STATUS 5
+#define NBD_REPLY_TYPE_BLOCK_STATUS_EXT 6
+#define NBD_REPLY_TYPE_ERROR NBD_REPLY_ERR(1)
+#define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_ERR(2)
/* Extent flags for base:allocation in NBD_REPLY_TYPE_BLOCK_STATUS */
#define NBD_STATE_HOLE (1 << 0)
diff --git a/nbd/common.c b/nbd/common.c
index ddfe7d118371..08eaed4cb3c2 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -79,6 +79,8 @@ const char *nbd_opt_lookup(uint32_t opt)
return "list meta context";
case NBD_OPT_SET_META_CONTEXT:
return "set meta context";
+ case NBD_OPT_EXTENDED_HEADERS:
+ return "extended headers";
default:
return "<unknown>";
}
@@ -168,9 +170,13 @@ const char *nbd_reply_type_lookup(uint16_t type)
case NBD_REPLY_TYPE_OFFSET_DATA:
return "data";
case NBD_REPLY_TYPE_OFFSET_HOLE:
- return "hole";
+ return "hole (32-bit)";
+ case NBD_REPLY_TYPE_OFFSET_HOLE_EXT:
+ return "hole (64-bit)";
case NBD_REPLY_TYPE_BLOCK_STATUS:
- return "block status";
+ return "block status (32-bit)";
+ case NBD_REPLY_TYPE_BLOCK_STATUS_EXT:
+ return "block status (64-bit)";
case NBD_REPLY_TYPE_ERROR:
return "generic error";
case NBD_REPLY_TYPE_ERROR_OFFSET:
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 08/14] nbd/server: Initial support for extended headers
We have no reason to send NBD_REPLY_TYPE_OFFSET_HOLE_EXT since we
already cap NBD_CMD_READ to 32M. For NBD_CMD_WRITE_ZEROES and
NBD_CMD_TRIM, the block layer already supports 64-bit operations
without any effort on our part. For NBD_CMD_BLOCK_STATUS, the
client's length is a hint; the easiest approach is to truncate our
answer back to 32 bits, letting us delay the effort of implementing
NBD_REPLY_TYPE_BLOCK_STATUS_EXT to a separate patch.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
nbd/nbd-internal.h | 5 ++-
nbd/server.c | 106 ++++++++++++++++++++++++++++++++++-----------
2 files changed, 85 insertions(+), 26 deletions(-)
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 0016793ff4b1..875b6204c28c 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -35,8 +35,11 @@
* https://github.com/yoe/nbd/blob/master/doc/proto.md
*/
-/* Size of all NBD_OPT_*, without payload */
+/* Size of all compact NBD_CMD_*, without payload */
#define NBD_REQUEST_SIZE (4 + 2 + 2 + 8 + 8 + 4)
+/* Size of all extended NBD_CMD_*, without payload */
+#define NBD_REQUEST_EXT_SIZE (4 + 2 + 2 + 8 + 8 + 8)
+
/* Size of all NBD_REP_* sent in answer to most NBD_OPT_*, without payload */
#define NBD_REPLY_SIZE (4 + 4 + 8)
/* Size of reply to NBD_OPT_EXPORT_NAME */
diff --git a/nbd/server.c b/nbd/server.c
index 4306fa7b426c..0e496f60ffbd 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -142,6 +142,7 @@ struct NBDClient {
uint32_t check_align; /* If non-zero, check for aligned client requests */
bool structured_reply;
+ bool extended_headers;
NBDExportMetaContexts export_meta;
uint32_t opt; /* Current option being negotiated */
@@ -1275,6 +1276,19 @@ static int nbd_negotiate_options(NBDClient *client, Error
**errp)
errp);
break;
+ case NBD_OPT_EXTENDED_HEADERS:
+ if (length) {
+ ret = nbd_reject_length(client, false, errp);
+ } else if (client->extended_headers) {
+ ret = nbd_negotiate_send_rep_err(
+ client, NBD_REP_ERR_INVALID, errp,
+ "extended headers already negotiated");
+ } else {
+ ret = nbd_negotiate_send_rep(client, NBD_REP_ACK, errp);
+ client->extended_headers = true;
+ }
+ break;
+
default:
ret = nbd_opt_drop(client, NBD_REP_ERR_UNSUP, errp,
"Unsupported option %" PRIu32
" (%s)",
@@ -1410,11 +1424,13 @@ nbd_read_eof(NBDClient *client, void *buffer, size_t
size, Error **errp)
static int nbd_receive_request(NBDClient *client, NBDRequest *request,
Error **errp)
{
- uint8_t buf[NBD_REQUEST_SIZE];
- uint32_t magic;
+ uint8_t buf[NBD_REQUEST_EXT_SIZE];
+ uint32_t magic, expect;
int ret;
+ size_t size = client->extended_headers ? NBD_REQUEST_EXT_SIZE
+ : NBD_REQUEST_SIZE;
- ret = nbd_read_eof(client, buf, sizeof(buf), errp);
+ ret = nbd_read_eof(client, buf, size, errp);
if (ret < 0) {
return ret;
}
@@ -1422,13 +1438,21 @@ static int nbd_receive_request(NBDClient *client,
NBDRequest *request,
return -EIO;
}
- /* Request
- [ 0 .. 3] magic (NBD_REQUEST_MAGIC)
- [ 4 .. 5] flags (NBD_CMD_FLAG_FUA, ...)
- [ 6 .. 7] type (NBD_CMD_READ, ...)
- [ 8 .. 15] handle
- [16 .. 23] from
- [24 .. 27] len
+ /*
+ * Compact request
+ * [ 0 .. 3] magic (NBD_REQUEST_MAGIC)
+ * [ 4 .. 5] flags (NBD_CMD_FLAG_FUA, ...)
+ * [ 6 .. 7] type (NBD_CMD_READ, ...)
+ * [ 8 .. 15] handle
+ * [16 .. 23] from
+ * [24 .. 27] len
+ * Extended request
+ * [ 0 .. 3] magic (NBD_REQUEST_EXT_MAGIC)
+ * [ 4 .. 5] flags (NBD_CMD_FLAG_FUA, ...)
+ * [ 6 .. 7] type (NBD_CMD_READ, ...)
+ * [ 8 .. 15] handle
+ * [16 .. 23] from
+ * [24 .. 31] len
*/
magic = ldl_be_p(buf);
@@ -1436,12 +1460,18 @@ static int nbd_receive_request(NBDClient *client,
NBDRequest *request,
request->type = lduw_be_p(buf + 6);
request->handle = ldq_be_p(buf + 8);
request->from = ldq_be_p(buf + 16);
- request->len = ldl_be_p(buf + 24); /* widen 32 to 64 bits */
+ if (client->extended_headers) {
+ request->len = ldq_be_p(buf + 24);
+ expect = NBD_REQUEST_EXT_MAGIC;
+ } else {
+ request->len = ldl_be_p(buf + 24); /* widen 32 to 64 bits */
+ expect = NBD_REQUEST_MAGIC;
+ }
trace_nbd_receive_request(magic, request->flags, request->type,
request->from, request->len);
- if (magic != NBD_REQUEST_MAGIC) {
+ if (magic != expect) {
error_setg(errp, "invalid magic (got 0x%" PRIx32
")", magic);
return -EINVAL;
}
@@ -1872,12 +1902,22 @@ static int coroutine_fn nbd_co_send_iov(NBDClient
*client, struct iovec *iov,
static inline void set_be_simple_reply(NBDClient *client, struct iovec *iov,
uint64_t error, uint64_t handle)
{
- NBDSimpleReply *reply = iov->iov_base;
+ if (client->extended_headers) {
+ NBDSimpleReplyExt *reply = iov->iov_base;
- iov->iov_len = sizeof(*reply);
- stl_be_p(&reply->magic, NBD_SIMPLE_REPLY_MAGIC);
- stl_be_p(&reply->error, error);
- stq_be_p(&reply->handle, handle);
+ iov->iov_len = sizeof(*reply);
+ stl_be_p(&reply->magic, NBD_SIMPLE_REPLY_EXT_MAGIC);
+ stl_be_p(&reply->error, error);
+ stq_be_p(&reply->handle, handle);
+ reply->_pad1 = reply->_pad2 = 0;
+ } else {
+ NBDSimpleReply *reply = iov->iov_base;
+
+ iov->iov_len = sizeof(*reply);
+ stl_be_p(&reply->magic, NBD_SIMPLE_REPLY_MAGIC);
+ stl_be_p(&reply->error, error);
+ stq_be_p(&reply->handle, handle);
+ }
}
static int nbd_co_send_simple_reply(NBDClient *client,
@@ -1905,14 +1945,26 @@ static inline void set_be_chunk(NBDClient *client,
struct iovec *iov,
uint16_t flags, uint16_t type,
uint64_t handle, uint32_t length)
{
- NBDStructuredReplyChunk *chunk = iov->iov_base;
+ if (client->extended_headers) {
+ NBDStructuredReplyChunkExt *chunk = iov->iov_base;
- iov->iov_len = sizeof(*chunk);
- stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC);
- stw_be_p(&chunk->flags, flags);
- stw_be_p(&chunk->type, type);
- stq_be_p(&chunk->handle, handle);
- stl_be_p(&chunk->length, length);
+ iov->iov_len = sizeof(*chunk);
+ stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_EXT_MAGIC);
+ stw_be_p(&chunk->flags, flags);
+ stw_be_p(&chunk->type, type);
+ stq_be_p(&chunk->handle, handle);
+ stq_be_p(&chunk->length, length);
+ chunk->_pad = 0;
+ } else {
+ NBDStructuredReplyChunk *chunk = iov->iov_base;
+
+ iov->iov_len = sizeof(*chunk);
+ stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC);
+ stw_be_p(&chunk->flags, flags);
+ stw_be_p(&chunk->type, type);
+ stq_be_p(&chunk->handle, handle);
+ stl_be_p(&chunk->length, length);
+ }
}
static int coroutine_fn nbd_co_send_structured_done(NBDClient *client,
@@ -2555,7 +2607,11 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
return nbd_send_generic_reply(client, request->handle, -EINVAL,
"need non-zero length",
errp);
}
- assert(request->len <= UINT32_MAX);
+ if (request->len > UINT32_MAX) {
+ /* For now, truncate our response to a 32 bit window */
+ request->len = QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
+ client->check_align ?: 1);
+ }
if (client->export_meta.count) {
bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE;
int contexts_remaining = client->export_meta.count;
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 09/14] nbd/server: Support 64-bit block status
The previous patch handled extended headers by truncating large block
status requests from the client back to 32 bits. But this is not
ideal; for cases where we can truly determine the status of the entire
image quickly (for example, when reporting the entire image as
non-sparse because we lack the ability to probe for holes), this
causes more network traffic for the client to iterate through 4G
chunks than for us to just report the entire image at once. For ease
of implementation, if extended headers were negotiated, then we always
reply with 64-bit block status replies, even when the result could
have fit in the older 32-bit block status chunk (clients supporting
extended headers have to be prepared for either chunk type, so
temporarily reverting this patch proves whether a client is
compliant).
Note that we previously had some interesting size-juggling on call
chains, such as:
nbd_co_send_block_status(uint32_t length)
-> blockstatus_to_extends(uint32_t bytes)
-> bdrv_block_status_above(bytes, &uint64_t num)
-> nbd_extent_array_add(uint64_t num)
-> store num in 32-bit length
But we were lucky that it never overflowed: bdrv_block_status_above
never sets num larger than bytes, and we had previously been capping
'bytes' at 32 bits (either by the protocol, or in the previous patch
with an explicit truncation). This patch adds some assertions that
ensure we continue to avoid overflowing 32 bits for a narrow client,
while fully utilizing 64-bits all the way through when the client
understands that.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
nbd/server.c | 72 ++++++++++++++++++++++++++++++++++------------------
1 file changed, 48 insertions(+), 24 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 0e496f60ffbd..7e6140350797 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2106,20 +2106,26 @@ static int coroutine_fn
nbd_co_send_sparse_read(NBDClient *client,
}
typedef struct NBDExtentArray {
- NBDExtent *extents;
+ union {
+ NBDExtent *narrow;
+ NBDExtentExt *extents;
+ };
unsigned int nb_alloc;
unsigned int count;
uint64_t total_length;
+ bool extended; /* Whether 64-bit extents are allowed */
bool can_add;
bool converted_to_be;
} NBDExtentArray;
-static NBDExtentArray *nbd_extent_array_new(unsigned int nb_alloc)
+static NBDExtentArray *nbd_extent_array_new(unsigned int nb_alloc,
+ bool extended)
{
NBDExtentArray *ea = g_new0(NBDExtentArray, 1);
ea->nb_alloc = nb_alloc;
- ea->extents = g_new(NBDExtent, nb_alloc);
+ ea->extents = g_new(NBDExtentExt, nb_alloc);
+ ea->extended = extended;
ea->can_add = true;
return ea;
@@ -2133,17 +2139,31 @@ static void nbd_extent_array_free(NBDExtentArray *ea)
G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
/* Further modifications of the array after conversion are abandoned */
-static void nbd_extent_array_convert_to_be(NBDExtentArray *ea)
+static void nbd_extent_array_convert_to_be(NBDExtentArray *ea,
+ struct iovec *iov)
{
int i;
assert(!ea->converted_to_be);
+ assert(iov->iov_base == ea->extents);
ea->can_add = false;
ea->converted_to_be = true;
- for (i = 0; i < ea->count; i++) {
- ea->extents[i].flags = cpu_to_be32(ea->extents[i].flags);
- ea->extents[i].length = cpu_to_be32(ea->extents[i].length);
+ if (ea->extended) {
+ for (i = 0; i < ea->count; i++) {
+ ea->extents[i].length = cpu_to_be64(ea->extents[i].length);
+ ea->extents[i].flags = cpu_to_be32(ea->extents[i].flags);
+ assert(ea->extents[i]._pad == 0);
+ }
+ iov->iov_len = ea->count * sizeof(ea->extents[0]);
+ } else {
+ /* Conversion reduces memory usage, order of iteration matters */
+ for (i = 0; i < ea->count; i++) {
+ assert(ea->extents[i].length <= UINT32_MAX);
+ ea->narrow[i].length = cpu_to_be32(ea->extents[i].length);
+ ea->narrow[i].flags = cpu_to_be32(ea->extents[i].flags);
+ }
+ iov->iov_len = ea->count * sizeof(ea->narrow[0]);
}
}
@@ -2157,19 +2177,23 @@ static void
nbd_extent_array_convert_to_be(NBDExtentArray *ea)
* would result in an incorrect range reported to the client)
*/
static int nbd_extent_array_add(NBDExtentArray *ea,
- uint32_t length, uint32_t flags)
+ uint64_t length, uint32_t flags)
{
assert(ea->can_add);
if (!length) {
return 0;
}
+ if (!ea->extended) {
+ assert(length <= UINT32_MAX);
+ }
/* Extend previous extent if flags are the same */
if (ea->count > 0 && flags == ea->extents[ea->count -
1].flags) {
- uint64_t sum = (uint64_t)length + ea->extents[ea->count -
1].length;
+ uint64_t sum = length + ea->extents[ea->count - 1].length;
- if (sum <= UINT32_MAX) {
+ assert(sum >= length);
+ if (sum <= UINT32_MAX || ea->extended) {
ea->extents[ea->count - 1].length = sum;
ea->total_length += length;
return 0;
@@ -2182,7 +2206,7 @@ static int nbd_extent_array_add(NBDExtentArray *ea,
}
ea->total_length += length;
- ea->extents[ea->count] = (NBDExtent) {.length = length, .flags =
flags};
+ ea->extents[ea->count] = (NBDExtentExt) {.length = length, .flags =
flags};
ea->count++;
return 0;
@@ -2253,15 +2277,16 @@ static int nbd_co_send_extents(NBDClient *client,
uint64_t handle,
struct iovec iov[] = {
{.iov_base = &hdr},
{.iov_base = &chunk, .iov_len = sizeof(chunk)},
- {.iov_base = ea->extents, .iov_len = ea->count *
sizeof(ea->extents[0])}
+ {.iov_base = ea->extents}
};
- nbd_extent_array_convert_to_be(ea);
+ nbd_extent_array_convert_to_be(ea, &iov[2]);
trace_nbd_co_send_extents(handle, ea->count, context_id,
ea->total_length,
last);
set_be_chunk(client, &iov[0], last ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_BLOCK_STATUS,
+ client->extended_headers ? NBD_REPLY_TYPE_BLOCK_STATUS_EXT
+ : NBD_REPLY_TYPE_BLOCK_STATUS,
handle, iov[1].iov_len + iov[2].iov_len);
stl_be_p(&chunk.context_id, context_id);
@@ -2271,13 +2296,14 @@ static int nbd_co_send_extents(NBDClient *client,
uint64_t handle,
/* Get block status from the exported device and send it to the client */
static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
BlockDriverState *bs, uint64_t offset,
- uint32_t length, bool dont_fragment,
+ uint64_t length, bool dont_fragment,
bool last, uint32_t context_id,
Error **errp)
{
int ret;
unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_EXTENTS;
- g_autoptr(NBDExtentArray) ea = nbd_extent_array_new(nb_extents);
+ g_autoptr(NBDExtentArray) ea + nbd_extent_array_new(nb_extents,
client->extended_headers);
if (context_id == NBD_META_ID_BASE_ALLOCATION) {
ret = blockstatus_to_extents(bs, offset, length, ea);
@@ -2304,7 +2330,8 @@ static void bitmap_to_extents(BdrvDirtyBitmap *bitmap,
bdrv_dirty_bitmap_lock(bitmap);
for (start = offset;
- bdrv_dirty_bitmap_next_dirty_area(bitmap, start, end, INT32_MAX,
+ bdrv_dirty_bitmap_next_dirty_area(bitmap, start, end,
+ es->extended ? INT64_MAX :
INT32_MAX,
&dirty_start, &dirty_count);
start = dirty_start + dirty_count)
{
@@ -2326,11 +2353,12 @@ static void bitmap_to_extents(BdrvDirtyBitmap *bitmap,
static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle,
BdrvDirtyBitmap *bitmap, uint64_t offset,
- uint32_t length, bool dont_fragment, bool last,
+ uint64_t length, bool dont_fragment, bool last,
uint32_t context_id, Error **errp)
{
unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_EXTENTS;
- g_autoptr(NBDExtentArray) ea = nbd_extent_array_new(nb_extents);
+ g_autoptr(NBDExtentArray) ea + nbd_extent_array_new(nb_extents,
client->extended_headers);
bitmap_to_extents(bitmap, offset, length, ea);
@@ -2607,11 +2635,7 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
return nbd_send_generic_reply(client, request->handle, -EINVAL,
"need non-zero length",
errp);
}
- if (request->len > UINT32_MAX) {
- /* For now, truncate our response to a 32 bit window */
- request->len = QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
- client->check_align ?: 1);
- }
+ assert(client->extended_headers || 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;
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 10/14] nbd/client: Initial support for extended headers
Update the client code to be able to send an extended request, and
parse an extended header from the server. Note that since we reject
any structured reply with a too-large payload, we can always normalize
a valid header back into the compact form, so that the caller need not
deal with two branches of a union. Still, until a later patch lets
the client negotiate extended headers, the code added here should not
be reached. Note that because of the different magic numbers, it is
just as easy to trace and then tolerate a non-compliant server sending
the wrong header reply as it would be to insist that the server is
compliant.
The only caller to nbd_receive_reply() always passed NULL for errp;
since we are changing the signature anyways, I decided to sink the
decision to ignore errors one layer lower.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
include/block/nbd.h | 2 +-
block/nbd.c | 3 +-
nbd/client.c | 112 +++++++++++++++++++++++++++++++-------------
nbd/trace-events | 1 +
4 files changed, 84 insertions(+), 34 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 5f9d86a86352..d489c67d98dc 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -366,7 +366,7 @@ int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo
*info,
Error **errp);
int nbd_send_request(QIOChannel *ioc, NBDRequest *request, bool ext_hdr);
int coroutine_fn nbd_receive_reply(BlockDriverState *bs, QIOChannel *ioc,
- NBDReply *reply, Error **errp);
+ NBDReply *reply, bool ext_hdrs);
int nbd_client(int fd);
int nbd_disconnect(int fd);
int nbd_errno_to_system_errno(int err);
diff --git a/block/nbd.c b/block/nbd.c
index 3e9875241bec..da5e6ac2d9a5 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -401,7 +401,8 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s,
uint64_t handle)
/* We are under mutex and handle is 0. We have to do the dirty work. */
assert(s->reply.handle == 0);
- ret = nbd_receive_reply(s->bs, s->ioc, &s->reply, NULL);
+ ret = nbd_receive_reply(s->bs, s->ioc, &s->reply,
+ s->info.extended_headers);
if (ret <= 0) {
ret = ret ? ret : -EIO;
nbd_channel_error(s, ret);
diff --git a/nbd/client.c b/nbd/client.c
index aa162b9d08d5..f1aa5256c8bf 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1347,22 +1347,28 @@ int nbd_disconnect(int fd)
int nbd_send_request(QIOChannel *ioc, NBDRequest *request, bool ext_hdr)
{
- uint8_t buf[NBD_REQUEST_SIZE];
+ uint8_t buf[NBD_REQUEST_EXT_SIZE];
+ size_t len;
- assert(!ext_hdr);
- assert(request->len <= UINT32_MAX);
trace_nbd_send_request(request->from, request->len,
request->handle,
request->flags, request->type,
nbd_cmd_lookup(request->type));
- stl_be_p(buf, NBD_REQUEST_MAGIC);
+ stl_be_p(buf, ext_hdr ? NBD_REQUEST_EXT_MAGIC : NBD_REQUEST_MAGIC);
stw_be_p(buf + 4, request->flags);
stw_be_p(buf + 6, request->type);
stq_be_p(buf + 8, request->handle);
stq_be_p(buf + 16, request->from);
- stl_be_p(buf + 24, request->len);
+ if (ext_hdr) {
+ stq_be_p(buf + 24, request->len);
+ len = NBD_REQUEST_EXT_SIZE;
+ } else {
+ assert(request->len <= UINT32_MAX);
+ stl_be_p(buf + 24, request->len);
+ len = NBD_REQUEST_SIZE;
+ }
- return nbd_write(ioc, buf, sizeof(buf), NULL);
+ return nbd_write(ioc, buf, len, NULL);
}
/* nbd_receive_simple_reply
@@ -1370,49 +1376,69 @@ int nbd_send_request(QIOChannel *ioc, NBDRequest
*request, bool ext_hdr)
* Payload is not read (payload is possible for CMD_READ, but here we even
* don't know whether it take place or not).
*/
-static int nbd_receive_simple_reply(QIOChannel *ioc, NBDSimpleReply *reply,
+static int nbd_receive_simple_reply(QIOChannel *ioc, NBDReply *reply,
Error **errp)
{
int ret;
+ size_t len;
- assert(reply->magic == NBD_SIMPLE_REPLY_MAGIC);
+ if (reply->magic == NBD_SIMPLE_REPLY_MAGIC) {
+ len = sizeof(reply->simple);
+ } else {
+ assert(reply->magic == NBD_SIMPLE_REPLY_EXT_MAGIC);
+ len = sizeof(reply->simple_ext);
+ }
ret = nbd_read(ioc, (uint8_t *)reply + sizeof(reply->magic),
- sizeof(*reply) - sizeof(reply->magic), "reply",
errp);
+ len - sizeof(reply->magic), "reply", errp);
if (ret < 0) {
return ret;
}
- reply->error = be32_to_cpu(reply->error);
- reply->handle = be64_to_cpu(reply->handle);
+ /* error and handle occupy same space between forms */
+ reply->simple.error = be32_to_cpu(reply->simple.error);
+ reply->simple.handle = be64_to_cpu(reply->handle);
+ if (reply->magic == NBD_SIMPLE_REPLY_EXT_MAGIC) {
+ if (reply->simple_ext._pad1 || reply->simple_ext._pad2) {
+ error_setg(errp, "Server used non-zero padding in extended
header");
+ return -EINVAL;
+ }
+ reply->magic = NBD_SIMPLE_REPLY_MAGIC;
+ }
return 0;
}
/* nbd_receive_structured_reply_chunk
* Read structured reply chunk except magic field (which should be already
- * read).
+ * read). Normalize into the compact form.
* Payload is not read.
*/
-static int nbd_receive_structured_reply_chunk(QIOChannel *ioc,
- NBDStructuredReplyChunk *chunk,
+static int nbd_receive_structured_reply_chunk(QIOChannel *ioc, NBDReply *chunk,
Error **errp)
{
int ret;
+ size_t len;
+ uint64_t payload_len;
- assert(chunk->magic == NBD_STRUCTURED_REPLY_MAGIC);
+ if (chunk->magic == NBD_STRUCTURED_REPLY_MAGIC) {
+ len = sizeof(chunk->structured);
+ } else {
+ assert(chunk->magic == NBD_STRUCTURED_REPLY_EXT_MAGIC);
+ len = sizeof(chunk->structured_ext);
+ }
ret = nbd_read(ioc, (uint8_t *)chunk + sizeof(chunk->magic),
- sizeof(*chunk) - sizeof(chunk->magic), "structured
chunk",
+ len - sizeof(chunk->magic), "structured chunk",
errp);
if (ret < 0) {
return ret;
}
- chunk->flags = be16_to_cpu(chunk->flags);
- chunk->type = be16_to_cpu(chunk->type);
- chunk->handle = be64_to_cpu(chunk->handle);
- chunk->length = be32_to_cpu(chunk->length);
+ /* flags, type, and handle occupy same space between forms */
+ chunk->structured.flags = be16_to_cpu(chunk->structured.flags);
+ chunk->structured.type = be16_to_cpu(chunk->structured.type);
+ chunk->structured.handle = be64_to_cpu(chunk->structured.handle);
/*
* Because we use BLOCK_STATUS with REQ_ONE, and cap READ requests
@@ -1420,11 +1446,23 @@ static int nbd_receive_structured_reply_chunk(QIOChannel
*ioc,
* this. Even if we stopped using REQ_ONE, sane servers will cap
* the number of extents they return for block status.
*/
- if (chunk->length > NBD_MAX_BUFFER_SIZE +
sizeof(NBDStructuredReadData)) {
+ if (chunk->magic == NBD_STRUCTURED_REPLY_MAGIC) {
+ payload_len = be32_to_cpu(chunk->structured.length);
+ } else {
+ payload_len = be64_to_cpu(chunk->structured_ext.length);
+ if (chunk->structured_ext._pad) {
+ error_setg(errp, "Server used non-zero padding in extended
header");
+ return -EINVAL;
+ }
+ chunk->magic = NBD_STRUCTURED_REPLY_MAGIC;
+ }
+ if (payload_len > NBD_MAX_BUFFER_SIZE + sizeof(NBDStructuredReadData)) {
error_setg(errp, "server chunk %" PRIu32 " (%s) payload
is too long",
- chunk->type, nbd_rep_lookup(chunk->type));
+ chunk->structured.type,
+ nbd_rep_lookup(chunk->structured.type));
return -EINVAL;
}
+ chunk->structured.length = payload_len;
return 0;
}
@@ -1471,30 +1509,36 @@ nbd_read_eof(BlockDriverState *bs, QIOChannel *ioc, void
*buffer, size_t size,
/* nbd_receive_reply
*
- * Decreases bs->in_flight while waiting for a new reply. This yield is
where
- * we wait indefinitely and the coroutine must be able to be safely reentered
- * for nbd_client_attach_aio_context().
+ * Wait for a new reply. If this yields, the coroutine must be able to be
+ * safely reentered for nbd_client_attach_aio_context(). @ext_hdrs determines
+ * which reply magic we are expecting, although this normalizes the result
+ * so that the caller only has to work with compact headers.
*
* Returns 1 on success
- * 0 on eof, when no data was read (errp is not set)
- * negative errno on failure (errp is set)
+ * 0 on eof, when no data was read
+ * negative errno on failure
*/
int coroutine_fn nbd_receive_reply(BlockDriverState *bs, QIOChannel *ioc,
- NBDReply *reply, Error **errp)
+ NBDReply *reply, bool ext_hdrs)
{
int ret;
const char *type;
- ret = nbd_read_eof(bs, ioc, &reply->magic, sizeof(reply->magic),
errp);
+ ret = nbd_read_eof(bs, ioc, &reply->magic, sizeof(reply->magic),
NULL);
if (ret <= 0) {
return ret;
}
reply->magic = be32_to_cpu(reply->magic);
+ /* Diagnose but accept wrong-width header */
switch (reply->magic) {
case NBD_SIMPLE_REPLY_MAGIC:
- ret = nbd_receive_simple_reply(ioc, &reply->simple, errp);
+ case NBD_SIMPLE_REPLY_EXT_MAGIC:
+ if (ext_hdrs != (reply->magic == NBD_SIMPLE_REPLY_EXT_MAGIC)) {
+ trace_nbd_receive_wrong_header(reply->magic);
+ }
+ ret = nbd_receive_simple_reply(ioc, reply, NULL);
if (ret < 0) {
break;
}
@@ -1503,7 +1547,11 @@ int coroutine_fn nbd_receive_reply(BlockDriverState *bs,
QIOChannel *ioc,
reply->handle);
break;
case NBD_STRUCTURED_REPLY_MAGIC:
- ret = nbd_receive_structured_reply_chunk(ioc,
&reply->structured, errp);
+ case NBD_STRUCTURED_REPLY_EXT_MAGIC:
+ if (ext_hdrs != (reply->magic == NBD_STRUCTURED_REPLY_EXT_MAGIC)) {
+ trace_nbd_receive_wrong_header(reply->magic);
+ }
+ ret = nbd_receive_structured_reply_chunk(ioc, reply, NULL);
if (ret < 0) {
break;
}
@@ -1514,7 +1562,7 @@ int coroutine_fn nbd_receive_reply(BlockDriverState *bs,
QIOChannel *ioc,
reply->structured.length);
break;
default:
- error_setg(errp, "invalid magic (got 0x%" PRIx32
")", reply->magic);
+ trace_nbd_receive_wrong_header(reply->magic);
return -EINVAL;
}
if (ret < 0) {
diff --git a/nbd/trace-events b/nbd/trace-events
index d18da8b0b743..ad63e565fd6e 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -34,6 +34,7 @@ nbd_client_clear_socket(void) "Clearing NBD socket"
nbd_send_request(uint64_t from, uint64_t len, uint64_t handle, uint16_t flags,
uint16_t type, const char *name) "Sending request to server: { .from =
%" PRIu64", .len = %" PRIu64 ", .handle = %" PRIu64
", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s)
}"
nbd_receive_simple_reply(int32_t error, const char *errname, uint64_t handle)
"Got simple reply: { .error = %" PRId32 " (%s), handle = %"
PRIu64" }"
nbd_receive_structured_reply_chunk(uint16_t flags, uint16_t type, const char
*name, uint64_t handle, uint32_t length) "Got structured reply chunk: {
flags = 0x%" PRIx16 ", type = %d (%s), handle = %" PRIu64 ",
length = %" PRIu32 " }"
+nbd_receive_wrong_header(uint32_t magic) "Server sent unexpected magic
0x%" PRIx32
# common.c
nbd_unknown_error(int err) "Squashing unexpected error %d to EINVAL"
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 11/14] nbd/client: Accept 64-bit hole chunks
Although our read requests are sized such that servers need not send
an extended hole chunk, we still have to be prepared for it to be
fully compliant if we request extended headers. We can also tolerate
a non-compliant server sending the new chunk even when it should not.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
block/nbd.c | 26 ++++++++++++++++++++------
block/trace-events | 1 +
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index da5e6ac2d9a5..c5dea864ebb6 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -518,20 +518,26 @@ static inline uint64_t payload_advance64(uint8_t
**payload)
static int nbd_parse_offset_hole_payload(BDRVNBDState *s,
NBDStructuredReplyChunk *chunk,
- uint8_t *payload, uint64_t
orig_offset,
+ uint8_t *payload, bool wide,
+ uint64_t orig_offset,
QEMUIOVector *qiov, Error **errp)
{
uint64_t offset;
- uint32_t hole_size;
+ uint64_t hole_size;
+ size_t len = wide ? sizeof(hole_size) : sizeof(uint32_t);
- if (chunk->length != sizeof(offset) + sizeof(hole_size)) {
+ if (chunk->length != sizeof(offset) + len) {
error_setg(errp, "Protocol error: invalid payload for "
"NBD_REPLY_TYPE_OFFSET_HOLE");
return -EINVAL;
}
offset = payload_advance64(&payload);
- hole_size = payload_advance32(&payload);
+ if (wide) {
+ hole_size = payload_advance64(&payload);
+ } else {
+ hole_size = payload_advance32(&payload);
+ }
if (!hole_size || offset < orig_offset || hole_size > qiov->size
||
offset > orig_offset + qiov->size - hole_size) {
@@ -544,6 +550,7 @@ static int nbd_parse_offset_hole_payload(BDRVNBDState *s,
trace_nbd_structured_read_compliance("hole");
}
+ assert(hole_size <= SIZE_MAX);
qemu_iovec_memset(qiov, offset - orig_offset, 0, hole_size);
return 0;
@@ -1037,9 +1044,16 @@ static int nbd_co_receive_cmdread_reply(BDRVNBDState *s,
uint64_t handle,
* in qiov
*/
break;
+ case NBD_REPLY_TYPE_OFFSET_HOLE_EXT:
+ if (!s->info.extended_headers) {
+ trace_nbd_extended_headers_compliance("hole_ext");
+ }
+ /* fallthrough */
case NBD_REPLY_TYPE_OFFSET_HOLE:
- ret = nbd_parse_offset_hole_payload(s, &reply.structured,
payload,
- offset, qiov, &local_err);
+ ret = nbd_parse_offset_hole_payload(
+ s, &reply.structured, payload,
+ chunk->type == NBD_REPLY_TYPE_OFFSET_HOLE_EXT,
+ offset, qiov, &local_err);
if (ret < 0) {
nbd_channel_error(s, ret);
nbd_iter_channel_error(&iter, ret, &local_err);
diff --git a/block/trace-events b/block/trace-events
index 549090d453e7..ee65da204dde 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -168,6 +168,7 @@ iscsi_xcopy(void *src_lun, uint64_t src_off, void *dst_lun,
uint64_t dst_off, ui
# nbd.c
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_extended_headers_compliance(const char *type) "server sent
non-compliant %s chunk without extended headers"
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_client_handshake(const char *export_name) "export '%s'"
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 12/14] nbd/client: Accept 64-bit block status chunks
Because we use NBD_CMD_FLAG_REQ_ONE with NBD_CMD_BLOCK_STATUS, a
client in narrow mode should not be able to provoke a server into
sending a block status result larger than the client's 32-bit request.
But in extended mode, a 64-bit status request must be able to handle a
64-bit status result, once a future patch enables the client
requesting extended mode. We can also tolerate a non-compliant server
sending the new chunk even when it should not.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
block/nbd.c | 38 +++++++++++++++++++++++++++-----------
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index c5dea864ebb6..bd4a9c407bde 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -563,13 +563,15 @@ static int nbd_parse_offset_hole_payload(BDRVNBDState *s,
*/
static int nbd_parse_blockstatus_payload(BDRVNBDState *s,
NBDStructuredReplyChunk *chunk,
- uint8_t *payload, uint64_t
orig_length,
- NBDExtent *extent, Error **errp)
+ uint8_t *payload, bool wide,
+ uint64_t orig_length,
+ NBDExtentExt *extent, Error **errp)
{
uint32_t context_id;
+ size_t len = wide ? sizeof(*extent) : sizeof(NBDExtent);
/* The server succeeded, so it must have sent [at least] one extent */
- if (chunk->length < sizeof(context_id) + sizeof(*extent)) {
+ if (chunk->length < sizeof(context_id) + len) {
error_setg(errp, "Protocol error: invalid payload for "
"NBD_REPLY_TYPE_BLOCK_STATUS");
return -EINVAL;
@@ -584,8 +586,16 @@ static int nbd_parse_blockstatus_payload(BDRVNBDState *s,
return -EINVAL;
}
- extent->length = payload_advance32(&payload);
- extent->flags = payload_advance32(&payload);
+ if (wide) {
+ extent->length = payload_advance64(&payload);
+ extent->flags = payload_advance32(&payload);
+ if (payload_advance32(&payload) != 0) {
+ trace_nbd_parse_blockstatus_compliance("non-zero extent
padding");
+ }
+ } else {
+ extent->length = payload_advance32(&payload);
+ extent->flags = payload_advance32(&payload);
+ }
if (extent->length == 0) {
error_setg(errp, "Protocol error: server sent status chunk with
"
@@ -625,7 +635,7 @@ static int nbd_parse_blockstatus_payload(BDRVNBDState *s,
* connection; just ignore trailing extents, and clamp things to
* the length of our request.
*/
- if (chunk->length > sizeof(context_id) + sizeof(*extent)) {
+ if (chunk->length > sizeof(context_id) + len) {
trace_nbd_parse_blockstatus_compliance("more than one
extent");
}
if (extent->length > orig_length) {
@@ -1081,7 +1091,7 @@ static int nbd_co_receive_cmdread_reply(BDRVNBDState *s,
uint64_t handle,
static int nbd_co_receive_blockstatus_reply(BDRVNBDState *s,
uint64_t handle, uint64_t length,
- NBDExtent *extent,
+ NBDExtentExt *extent,
int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
@@ -1098,6 +1108,11 @@ static int nbd_co_receive_blockstatus_reply(BDRVNBDState
*s,
assert(nbd_reply_is_structured(&reply));
switch (chunk->type) {
+ case NBD_REPLY_TYPE_BLOCK_STATUS_EXT:
+ if (!s->info.extended_headers) {
+
trace_nbd_extended_headers_compliance("block_status_ext");
+ }
+ /* fallthrough */
case NBD_REPLY_TYPE_BLOCK_STATUS:
if (received) {
nbd_channel_error(s, -EINVAL);
@@ -1106,9 +1121,10 @@ static int nbd_co_receive_blockstatus_reply(BDRVNBDState
*s,
}
received = true;
- ret = nbd_parse_blockstatus_payload(s, &reply.structured,
- payload, length, extent,
- &local_err);
+ ret = nbd_parse_blockstatus_payload(
+ s, &reply.structured, payload,
+ chunk->type == NBD_REPLY_TYPE_BLOCK_STATUS_EXT,
+ length, extent, &local_err);
if (ret < 0) {
nbd_channel_error(s, ret);
nbd_iter_channel_error(&iter, ret, &local_err);
@@ -1337,7 +1353,7 @@ static int coroutine_fn nbd_client_co_block_status(
int64_t *pnum, int64_t *map, BlockDriverState **file)
{
int ret, request_ret;
- NBDExtent extent = { 0 };
+ NBDExtentExt extent = { 0 };
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
Error *local_err = NULL;
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 13/14] nbd/client: Request extended headers during negotiation
All the pieces are in place for a client to finally request extended
headers. Note that we must not request extended headers when qemu-nbd
is used to connect to the kernel module (as nbd.ko does not expect
them), but there is no harm in all other clients requesting them.
Extended headers do not make a difference to the information collected
during 'qemu-nbd --list', but probing for it gives us one more piece
of information in that output. Update the iotests affected by the new
line of output.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
nbd/client-connection.c | 1 +
nbd/client.c | 26 ++++++++++++++++---
qemu-nbd.c | 2 ++
tests/qemu-iotests/223.out | 4 +++
tests/qemu-iotests/233.out | 1 +
tests/qemu-iotests/241 | 8 +++---
tests/qemu-iotests/307 | 2 +-
tests/qemu-iotests/307.out | 5 ++++
.../tests/nbd-qemu-allocation.out | 1 +
9 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/nbd/client-connection.c b/nbd/client-connection.c
index 695f85575414..d8b9ae230264 100644
--- a/nbd/client-connection.c
+++ b/nbd/client-connection.c
@@ -87,6 +87,7 @@ NBDClientConnection *nbd_client_connection_new(const
SocketAddress *saddr,
.initial_info.request_sizes = true,
.initial_info.structured_reply = true,
+ .initial_info.extended_headers = true,
.initial_info.base_allocation = true,
.initial_info.x_dirty_bitmap = g_strdup(x_dirty_bitmap),
.initial_info.name = g_strdup(export_name ?: "")
diff --git a/nbd/client.c b/nbd/client.c
index f1aa5256c8bf..0e227255d59b 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -882,8 +882,8 @@ static int nbd_list_meta_contexts(QIOChannel *ioc,
static int nbd_start_negotiate(AioContext *aio_context, QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
const char *hostname, QIOChannel **outioc,
- bool structured_reply, bool *zeroes,
- Error **errp)
+ bool structured_reply, bool *ext_hdrs,
+ bool *zeroes, Error **errp)
{
ERRP_GUARD();
uint64_t magic;
@@ -960,6 +960,15 @@ static int nbd_start_negotiate(AioContext *aio_context,
QIOChannel *ioc,
if (fixedNewStyle) {
int result = 0;
+ if (ext_hdrs && *ext_hdrs) {
+ result = nbd_request_simple_option(ioc,
+ NBD_OPT_EXTENDED_HEADERS,
+ false, errp);
+ if (result < 0) {
+ return -EINVAL;
+ }
+ *ext_hdrs = result == 1;
+ }
if (structured_reply) {
result = nbd_request_simple_option(ioc,
NBD_OPT_STRUCTURED_REPLY,
@@ -970,6 +979,9 @@ static int nbd_start_negotiate(AioContext *aio_context,
QIOChannel *ioc,
}
return 2 + result;
} else {
+ if (ext_hdrs) {
+ *ext_hdrs = false;
+ }
return 1;
}
} else if (magic == NBD_CLIENT_MAGIC) {
@@ -977,6 +989,9 @@ static int nbd_start_negotiate(AioContext *aio_context,
QIOChannel *ioc,
error_setg(errp, "Server does not support STARTTLS");
return -EINVAL;
}
+ if (ext_hdrs) {
+ *ext_hdrs = false;
+ }
return 0;
} else {
error_setg(errp, "Bad server magic received: 0x%" PRIx64,
magic);
@@ -1030,7 +1045,8 @@ int nbd_receive_negotiate(AioContext *aio_context,
QIOChannel *ioc,
trace_nbd_receive_negotiate_name(info->name);
result = nbd_start_negotiate(aio_context, ioc, tlscreds, hostname, outioc,
- info->structured_reply, &zeroes, errp);
+ info->structured_reply,
+ &info->extended_headers, &zeroes,
errp);
info->structured_reply = false;
info->base_allocation = false;
@@ -1147,10 +1163,11 @@ int nbd_receive_export_list(QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
int ret = -1;
NBDExportInfo *array = NULL;
QIOChannel *sioc = NULL;
+ bool ext_hdrs;
*info = NULL;
result = nbd_start_negotiate(NULL, ioc, tlscreds, hostname, &sioc,
true,
- NULL, errp);
+ &ext_hdrs, NULL, errp);
if (tlscreds && sioc) {
ioc = sioc;
}
@@ -1179,6 +1196,7 @@ int nbd_receive_export_list(QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
array[count - 1].name = name;
array[count - 1].description = desc;
array[count - 1].structured_reply = result == 3;
+ array[count - 1].extended_headers = ext_hdrs;
}
for (i = 0; i < count; i++) {
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c6c20df68a4d..ecf47b7e1250 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -237,6 +237,8 @@ static int qemu_nbd_client_list(SocketAddress *saddr,
QCryptoTLSCreds *tls,
printf(" opt block: %u\n", list[i].opt_block);
printf(" max block: %u\n", list[i].max_block);
}
+ printf(" transaction size: %s\n",
+ list[i].extended_headers ? "64-bit" :
"32-bit");
if (list[i].n_contexts) {
printf(" available meta contexts: %d\n",
list[i].n_contexts);
for (j = 0; j < list[i].n_contexts; j++) {
diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
index e58ea5abbd5a..36de28ccd12f 100644
--- a/tests/qemu-iotests/223.out
+++ b/tests/qemu-iotests/223.out
@@ -76,6 +76,7 @@ exports available: 2
min block: 1
opt block: 4096
max block: 33554432
+ transaction size: 64-bit
available meta contexts: 2
base:allocation
qemu:dirty-bitmap:b
@@ -86,6 +87,7 @@ exports available: 2
min block: 1
opt block: 4096
max block: 33554432
+ transaction size: 64-bit
available meta contexts: 2
base:allocation
qemu:dirty-bitmap:b2
@@ -177,6 +179,7 @@ exports available: 2
min block: 1
opt block: 4096
max block: 33554432
+ transaction size: 64-bit
available meta contexts: 2
base:allocation
qemu:dirty-bitmap:b
@@ -187,6 +190,7 @@ exports available: 2
min block: 1
opt block: 4096
max block: 33554432
+ transaction size: 64-bit
available meta contexts: 2
base:allocation
qemu:dirty-bitmap:b2
diff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out
index 4b1f6a0e1513..b04cc38bd11c 100644
--- a/tests/qemu-iotests/233.out
+++ b/tests/qemu-iotests/233.out
@@ -43,6 +43,7 @@ exports available: 1
min block: 1
opt block: 4096
max block: 33554432
+ transaction size: 64-bit
available meta contexts: 1
base:allocation
diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241
index c962c8b6075d..23307ca2d829 100755
--- a/tests/qemu-iotests/241
+++ b/tests/qemu-iotests/241
@@ -3,7 +3,7 @@
#
# Test qemu-nbd vs. unaligned images
#
-# Copyright (C) 2018-2019 Red Hat, Inc.
+# Copyright (C) 2018-2021 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -58,7 +58,7 @@ echo
nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE"
-$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
+$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '^ *\(size\|min\)'
$QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map
$QEMU_IO -f raw -c map "$TEST_IMG"
nbd_server_stop
@@ -71,7 +71,7 @@ echo
# sector alignment, here at the server.
nbd_server_start_unix_socket "$TEST_IMG_FILE" 2>
"$TEST_DIR/server.log"
-$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
+$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '^ *\(size\|min\)'
$QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map
$QEMU_IO -f raw -c map "$TEST_IMG"
nbd_server_stop
@@ -84,7 +84,7 @@ echo
# Now force sector alignment at the client.
nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE"
-$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
+$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '^ *\(size\|min\)'
$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
$QEMU_IO -c map "$TEST_IMG"
nbd_server_stop
diff --git a/tests/qemu-iotests/307 b/tests/qemu-iotests/307
index b429b5aa50a4..64ce250d82de 100755
--- a/tests/qemu-iotests/307
+++ b/tests/qemu-iotests/307
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
# group: rw quick export
#
-# Copyright (C) 2020 Red Hat, Inc.
+# Copyright (C) 2020-2021 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
diff --git a/tests/qemu-iotests/307.out b/tests/qemu-iotests/307.out
index ec8d2be0e0a4..343ddc0a5c16 100644
--- a/tests/qemu-iotests/307.out
+++ b/tests/qemu-iotests/307.out
@@ -19,6 +19,7 @@ exports available: 1
min block: XXX
opt block: XXX
max block: XXX
+ transaction size: 64-bit
available meta contexts: 1
base:allocation
@@ -47,6 +48,7 @@ exports available: 1
min block: XXX
opt block: XXX
max block: XXX
+ transaction size: 64-bit
available meta contexts: 1
base:allocation
@@ -78,6 +80,7 @@ exports available: 2
min block: XXX
opt block: XXX
max block: XXX
+ transaction size: 64-bit
available meta contexts: 1
base:allocation
export: 'export1'
@@ -87,6 +90,7 @@ exports available: 2
min block: XXX
opt block: XXX
max block: XXX
+ transaction size: 64-bit
available meta contexts: 1
base:allocation
@@ -113,6 +117,7 @@ exports available: 1
min block: XXX
opt block: XXX
max block: XXX
+ transaction size: 64-bit
available meta contexts: 1
base:allocation
diff --git a/tests/qemu-iotests/tests/nbd-qemu-allocation.out
b/tests/qemu-iotests/tests/nbd-qemu-allocation.out
index 0bf1abb06338..f30b4bed2144 100644
--- a/tests/qemu-iotests/tests/nbd-qemu-allocation.out
+++ b/tests/qemu-iotests/tests/nbd-qemu-allocation.out
@@ -21,6 +21,7 @@ exports available: 1
min block: 1
opt block: 4096
max block: 33554432
+ transaction size: 64-bit
available meta contexts: 2
base:allocation
qemu:allocation-depth
--
2.33.1
Eric Blake
2021-Dec-03 23:15 UTC
[Libguestfs] [PATCH 14/14] do not apply: nbd/server: Send 64-bit hole chunk
Since we cap NBD_CMD_READ requests to 32M, we never have a reason to
send a 64-bit chunk type for a hole; but it is worth producing these
for interoperability testing of clients that want extended headers.
---
nbd/server.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 7e6140350797..4369a9a8ff08 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2071,19 +2071,29 @@ static int coroutine_fn
nbd_co_send_sparse_read(NBDClient *client,
if (status & BDRV_BLOCK_ZERO) {
NBDReply hdr;
NBDStructuredReadHole chunk;
+ NBDStructuredReadHoleExt chunk_ext;
struct iovec iov[] = {
{.iov_base = &hdr},
- {.iov_base = &chunk, .iov_len = sizeof(chunk)},
+ {.iov_base = client->extended_headers ? &chunk_ext
+ : (void *) &chunk,
+ .iov_len = client->extended_headers ? sizeof(chunk_ext)
+ : sizeof(chunk)},
};
trace_nbd_co_send_structured_read_hole(handle, offset + progress,
pnum);
set_be_chunk(client, &iov[0],
final ? NBD_REPLY_FLAG_DONE : 0,
- NBD_REPLY_TYPE_OFFSET_HOLE,
+ client->extended_headers ?
NBD_REPLY_TYPE_OFFSET_HOLE_EXT
+ : NBD_REPLY_TYPE_OFFSET_HOLE,
handle, iov[1].iov_len);
- stq_be_p(&chunk.offset, offset + progress);
- stl_be_p(&chunk.length, pnum);
+ if (client->extended_headers) {
+ stq_be_p(&chunk_ext.offset, offset + progress);
+ stq_be_p(&chunk_ext.length, pnum);
+ } else {
+ stq_be_p(&chunk.offset, offset + progress);
+ stl_be_p(&chunk.length, pnum);
+ }
ret = nbd_co_send_iov(client, iov, 2, errp);
} else {
ret = blk_pread(exp->common.blk, offset + progress,
--
2.33.1