From 25bfe4a95b02b6fefafdfa1651c50a4d0c5bc87b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 23 Jul 2019 14:45:44 +0100 Subject: [PATCH 06/14] nbd/client: Support qemu-img convert from unaligned size RH-Author: Max Reitz Message-id: <20190723144546.23701-6-mreitz@redhat.com> Patchwork-id: 89651 O-Subject: [RHEL-8.1.0 qemu-kvm PATCH 5/7] nbd/client: Support qemu-img convert from unaligned size Bugzilla: 1678979 RH-Acked-by: Kevin Wolf RH-Acked-by: Stefano Garzarella RH-Acked-by: John Snow From: Eric Blake If an NBD server advertises a size that is not a multiple of a sector, the block layer rounds up that size, even though we set info.size to the exact byte value sent by the server. The block layer then proceeds to let us read or query block status on the hole that it added past EOF, which the NBD server is unlikely to be happy with. Fortunately, qemu as a server never advertizes an unaligned size, so we generally don't run into this problem; but the nbdkit server makes it easy to test: $ printf %1000d 1 > f1 $ ~/nbdkit/nbdkit -fv file f1 & pid=$! $ qemu-img convert -f raw nbd://localhost:10809 f2 $ kill $pid $ qemu-img compare f1 f2 Pre-patch, the server attempts a 1024-byte read, which nbdkit rightfully rejects as going beyond its advertised 1000 byte size; the conversion fails and the output files differ (not even the first sector is copied, because qemu-img does not follow ddrescue's habit of trying smaller reads to get as much information as possible in spite of errors). Post-patch, the client's attempts to read (and query block status, for new enough nbdkit) are properly truncated to the server's length, with sane handling of the hole the block layer forced on us. Although f2 ends up as a larger file (1024 bytes instead of 1000), qemu-img compare shows the two images to have identical contents for display to the guest. I didn't add iotests coverage since I didn't want to add a dependency on nbdkit in iotests. I also did NOT patch write, trim, or write zeroes - these commands continue to fail (usually with ENOSPC, but whatever the server chose), because we really can't write to the end of the file, and because 'qemu-img convert' is the most common case where we care about being tolerant (which is read-only). Perhaps we could truncate the request if the client is writing zeros to the tail, but that seems like more work, especially if the block layer is fixed in 4.1 to track byte-accurate sizing (in which case this patch would be reverted as unnecessary). Signed-off-by: Eric Blake Message-Id: <20190329042750.14704-5-eblake@redhat.com> Tested-by: Richard W.M. Jones (cherry picked from commit 9cf638508c0090b33ada4155c7cbb684e08e5ee9) Signed-off-by: Max Reitz Signed-off-by: Danilo C. L. de Paula --- block/nbd-client.c | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index 80d3625..6b33fe3 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -790,6 +790,25 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset, if (!bytes) { return 0; } + /* + * Work around the fact that the block layer doesn't do + * byte-accurate sizing yet - if the read exceeds the server's + * advertised size because the block layer rounded size up, then + * truncate the request to the server and tail-pad with zero. + */ + if (offset >= client->info.size) { + assert(bytes < BDRV_SECTOR_SIZE); + qemu_iovec_memset(qiov, 0, 0, bytes); + return 0; + } + if (offset + bytes > client->info.size) { + uint64_t slop = offset + bytes - client->info.size; + + assert(slop < BDRV_SECTOR_SIZE); + qemu_iovec_memset(qiov, bytes - slop, 0, slop); + request.len -= slop; + } + ret = nbd_co_send_request(bs, &request, NULL); if (ret < 0) { return ret; @@ -904,7 +923,8 @@ int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs, .from = offset, .len = MIN(MIN_NON_ZERO(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment), - client->info.max_block), bytes), + client->info.max_block), + MIN(bytes, client->info.size - offset)), .flags = NBD_CMD_FLAG_REQ_ONE, }; @@ -913,6 +933,23 @@ int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs, return BDRV_BLOCK_DATA; } + /* + * Work around the fact that the block layer doesn't do + * byte-accurate sizing yet - if the status request exceeds the + * server's advertised size because the block layer rounded size + * up, we truncated the request to the server (above), or are + * called on just the hole. + */ + if (offset >= client->info.size) { + *pnum = bytes; + assert(bytes < BDRV_SECTOR_SIZE); + /* Intentionally don't report offset_valid for the hole */ + return BDRV_BLOCK_ZERO; + } + + if (client->info.min_block) { + assert(QEMU_IS_ALIGNED(request.len, client->info.min_block)); + } ret = nbd_co_send_request(bs, &request, NULL); if (ret < 0) { return ret; -- 1.8.3.1