From 25bfe4a95b02b6fefafdfa1651c50a4d0c5bc87b Mon Sep 17 00:00:00 2001

From: Max Reitz <mreitz@redhat.com>

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 <mreitz@redhat.com>

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 <kwolf@redhat.com>

RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>

RH-Acked-by: John Snow <jsnow@redhat.com>

From: Eric Blake <eblake@redhat.com>

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 <eblake@redhat.com>

Message-Id: <20190329042750.14704-5-eblake@redhat.com>

Tested-by: Richard W.M. Jones <rjones@redhat.com>

(cherry picked from commit 9cf638508c0090b33ada4155c7cbb684e08e5ee9)

Signed-off-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>

---

 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