From: Denis V. Lunev <[email protected]> NBD_CMD_CACHE carries no payload in either direction. The request is a header only, nbd_do_cmd_cache() passes a NULL qiov to blk_co_preadv(), and the reply is a bare status. Still the server rejects any effect length above NBD_MAX_BUFFER_SIZE with EINVAL, which forces a client to split a large prefetch into 32 MiB pieces.
The specification does not ask for this. The constraint was renamed from "maximum block size" to "maximum payload size" precisely to separate payload length from effect length, and it says: For commands that do not require a payload in either direction (such as NBD_CMD_TRIM or NBD_CMD_WRITE_ZEROES), the client MAY request an effect length larger than the maximum payload size; the server SHOULD NOT disconnect, but MAY reply with an NBD_EOVERFLOW or NBD_EINVAL error if the oversize request would require too many server resources when compared to the same command with an effect length limited to the maximum payload size (such as an implementation of NBD_CMD_WRITE_ZEROES that utilizes a scratch buffer). We already follow that for NBD_CMD_TRIM and NBD_CMD_WRITE_ZEROES, which carry no length check at all, and our client assumes a server supporting extended headers takes unlimited zero and trim lengths. Handle NBD_CMD_CACHE in the same way. Signed-off-by: Denis V. Lunev <[email protected]> Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]> CC: Eric Blake <[email protected]> CC: Vladimir Sementsov-Ogievskiy <[email protected]> --- nbd/server.c | 4 +--- tests/qemu-iotests/tests/nbd-commands | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 78ec984409..e6c47f8c2e 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -2718,7 +2718,6 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, break; case NBD_CMD_CACHE: - check_length = true; break; case NBD_CMD_WRITE_ZEROES: @@ -2752,7 +2751,7 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, req->complete = true; } if (check_length && request->len > NBD_MAX_BUFFER_SIZE) { - /* READ, WRITE, CACHE */ + /* READ, WRITE */ error_setg(errp, "len (%" PRIu64 ") is larger than max len (%u)", request->len, NBD_MAX_BUFFER_SIZE); return -EINVAL; @@ -2908,7 +2907,6 @@ static coroutine_fn int nbd_do_cmd_cache(NBDClient *client, NBDRequest *request, NBDExport *exp = client->exp; assert(request->type == NBD_CMD_CACHE); - assert(request->len <= NBD_MAX_BUFFER_SIZE); ret = blk_co_preadv(exp->common.blk, request->from, request->len, NULL, BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH); diff --git a/tests/qemu-iotests/tests/nbd-commands b/tests/qemu-iotests/tests/nbd-commands index 4c1cd33db7..cbfc47782f 100755 --- a/tests/qemu-iotests/tests/nbd-commands +++ b/tests/qemu-iotests/tests/nbd-commands @@ -114,10 +114,9 @@ class TestNbdCommands(iotests.QMPTestCase): self.assertEqual(self.block_status()['qemu:allocation-depth'], [(size, DEPTH_BACKING)]) - self.h.cache(maximum, 0) + self.h.cache(size, 0) - self.assertEqual(self.top_extents(), - [(0, maximum, 0), (maximum, size - maximum, 1)]) + self.assertEqual(self.top_extents(), [(0, size, 0)]) qemu_io('-c', f'read -P {pattern} 0 {size}', top) def test_cache_past_end_of_export(self): -- 2.53.0
