2:53 p.m.
Upcoming additions to support NBD 64-bit effect lengths allow for the
possibility to distinguish between payload length (capped at 32M) and
effect length (up to 63 bits). Without that extension, only the
NBD_CMD_WRITE request has a payload; but with the extension, it makes
sense to allow at least NBD_CMD_BLOCK_STATUS to have both a payload
and effect length (where the payload is a limited-size struct that in
turns gives the real effect length as well as a subset of known ids
for which status is requested). Other future NBD commands may also
have a request payload, so the 64-bit extension introduces a new
NBD_CMD_FLAG_PAYLOAD_LEN that distinguishes between whether the header
length is a payload length or an effect length, rather than
hard-coding the decision based on the command. Note that we do not
support the payload version of BLOCK_STATUS yet.
For this patch, no semantic change is intended for a compliant client.
For a non-compliant client, it is possible that the error behavior
changes (a different message, a change on whether the connection is
killed or remains alive for the next command, or so forth), but all
errors should still be handled gracefully.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
nbd/server.c | 55 +++++++++++++++++++++++++++++++++---------------
nbd/trace-events | 1 +
2 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index cf38a104d9a..5812a773ace 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2316,6 +2316,8 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req,
NBDRequest *
Error **errp)
{
NBDClient *client = req->client;
+ bool extended_with_payload;
+ int payload_len = 0;
int valid_flags;
int ret;
@@ -2329,27 +2331,41 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData
*req, NBDRequest *
trace_nbd_co_receive_request_decode_type(request->handle, request->type,
nbd_cmd_lookup(request->type));
- if (request->type != NBD_CMD_WRITE) {
- /* No payload, we are ready to read the next request. */
- req->complete = true;
- }
-
if (request->type == NBD_CMD_DISC) {
/* Special case: we're going to disconnect without a reply,
* whether or not flags, from, or len are bogus */
+ req->complete = true;
return -EIO;
}
+ /* Payload and buffer handling. */
+ extended_with_payload = client->header_style >= NBD_HEADER_EXTENDED &&
+ (request->flags & NBD_CMD_FLAG_PAYLOAD_LEN);
if (request->type == NBD_CMD_READ || request->type == NBD_CMD_WRITE ||
- request->type == NBD_CMD_CACHE)
- {
+ request->type == NBD_CMD_CACHE || extended_with_payload) {
if (request->len > NBD_MAX_BUFFER_SIZE) {
error_setg(errp, "len (%" PRIu64" ) is larger than max len
(%u)",
request->len, NBD_MAX_BUFFER_SIZE);
return -EINVAL;
}
- if (request->type != NBD_CMD_CACHE) {
+ if (request->type == NBD_CMD_WRITE || extended_with_payload) {
+ payload_len = request->len;
+ if (request->type != NBD_CMD_WRITE) {
+ /*
+ * For now, we don't support payloads on other
+ * commands; but we can keep the connection alive.
+ */
+ request->len = 0;
+ } else if (client->header_style >= NBD_HEADER_EXTENDED &&
+ !extended_with_payload) {
+ /* The client is noncompliant. Trace it, but proceed. */
+ trace_nbd_co_receive_ext_payload_compliance(request->from,
+ request->len);
+ }
+ }
+
+ if (request->type == NBD_CMD_WRITE || request->type == NBD_CMD_READ) {
req->data = blk_try_blockalign(client->exp->common.blk,
request->len);
if (req->data == NULL) {
@@ -2359,18 +2375,20 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData
*req, NBDRequest *
}
}
- if (request->type == NBD_CMD_WRITE) {
- assert(request->len <= NBD_MAX_BUFFER_SIZE);
- if (nbd_read(client->ioc, req->data, request->len, "CMD_WRITE
data",
- errp) < 0)
- {
+ if (payload_len) {
+ if (req->data) {
+ ret = nbd_read(client->ioc, req->data, payload_len,
+ "CMD_WRITE data", errp);
+ } else {
+ ret = nbd_drop(client->ioc, payload_len, errp);
+ }
+ if (ret < 0) {
return -EIO;
}
- req->complete = true;
-
trace_nbd_co_receive_request_payload_received(request->handle,
- request->len);
+ payload_len);
}
+ req->complete = true;
/* Sanity checks. */
if (client->exp->nbdflags & NBD_FLAG_READ_ONLY &&
@@ -2400,7 +2418,10 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req,
NBDRequest *
client->check_align);
}
valid_flags = NBD_CMD_FLAG_FUA;
- if (request->type == NBD_CMD_READ &&
+ if (request->type == NBD_CMD_WRITE &&
+ client->header_style >= NBD_HEADER_EXTENDED) {
+ valid_flags |= NBD_CMD_FLAG_PAYLOAD_LEN;
+ } else if (request->type == NBD_CMD_READ &&
client->header_style >= NBD_HEADER_STRUCTURED) {
valid_flags |= NBD_CMD_FLAG_DF;
} else if (request->type == NBD_CMD_WRITE_ZEROES) {
diff --git a/nbd/trace-events b/nbd/trace-events
index e2c1d68688d..adf5666e207 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -71,6 +71,7 @@ nbd_co_send_extents(uint64_t handle, unsigned int extents, uint32_t id,
uint64_t
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, uint64_t len) "Payload
received: handle = %" PRIu64 ", len = %" PRIu64
+nbd_co_receive_ext_payload_compliance(uint64_t from, uint64_t len) "client sent
non-compliant write without payload flag: from=0x%" PRIx64 ", len=0x%"
PRIx64
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.40.1