qemu-img: avoid unaligned read requests during convert

in case of large continous areas that share the same allocation status
it happens that the value of s->sector_next_status is unaligned to the
cluster size or even request alignment of the source. Avoid this by
stripping down the s->sector_next_status position to cluster boundaries.

Signed-off-by: Peter Lieven <pl@kamp.de>
Message-Id: <20200901125129.6398-1-pl@kamp.de>
[mreitz: Disable vhdx for 251]
Signed-off-by: Max Reitz <mreitz@redhat.com>

qemu-img.c

@@ -1666,6 +1666,7 @@ enum ImgConvertBlockStatus {
 typedef struct ImgConvertState {
     BlockBackend **src;
     int64_t *src_sectors;
+    int *src_alignment;
     int src_num;
     int64_t total_sectors;
     int64_t allocated_sectors;
@@ -1732,6 +1733,7 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
     if (s->sector_next_status <= sector_num) {
         uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
         int64_t count;
+        int tail;
         BlockDriverState *src_bs = blk_bs(s->src[src_cur]);
         BlockDriverState *base;
 
@@ -1772,6 +1774,16 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
 
         n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
 
+        /*
+         * Avoid that s->sector_next_status becomes unaligned to the source
+         * request alignment and/or cluster size to avoid unnecessary read
+         * cycles.
+         */
+        tail = (sector_num - src_cur_offset + n) % s->src_alignment[src_cur];
+        if (n > tail) {
+            n -= tail;
+        }
+
         if (ret & BDRV_BLOCK_ZERO) {
             s->status = post_backing_zero ? BLK_BACKING_FILE : BLK_ZERO;
         } else if (ret & BDRV_BLOCK_DATA) {
@@ -2410,8 +2422,10 @@ static int img_convert(int argc, char **argv)
 
     s.src = g_new0(BlockBackend *, s.src_num);
     s.src_sectors = g_new(int64_t, s.src_num);
+    s.src_alignment = g_new(int, s.src_num);
 
     for (bs_i = 0; bs_i < s.src_num; bs_i++) {
+        BlockDriverState *src_bs;
         s.src[bs_i] = img_open(image_opts, argv[optind + bs_i],
                                fmt, src_flags, src_writethrough, s.quiet,
                                force_share);
@@ -2426,6 +2440,13 @@ static int img_convert(int argc, char **argv)
             ret = -1;
             goto out;
         }
+        src_bs = blk_bs(s.src[bs_i]);
+        s.src_alignment[bs_i] = DIV_ROUND_UP(src_bs->bl.request_alignment,
+                                             BDRV_SECTOR_SIZE);
+        if (!bdrv_get_info(src_bs, &bdi)) {
+            s.src_alignment[bs_i] = MAX(s.src_alignment[bs_i],
+                                        bdi.cluster_size / BDRV_SECTOR_SIZE);
+        }
         s.total_sectors += s.src_sectors[bs_i];
     }
 
@@ -2708,6 +2729,7 @@ static int img_convert(int argc, char **argv)
         g_free(s.src);
     }
     g_free(s.src_sectors);
+    g_free(s.src_alignment);
 fail_getopt:
     g_free(options);
 

tests/qemu-iotests/251

@@ -46,8 +46,11 @@ if [ "$IMGOPTSSYNTAX" = "true" ]; then
     # We use json:{} filenames here, so we cannot work with additional options.
     _unsupported_fmt $IMGFMT
 else
-    # With VDI, the output is ordered differently.  Just disable it.
-    _unsupported_fmt vdi
+    # - With VDI, the output is ordered differently.  Just disable it.
+    # - VHDX has large clusters; because qemu-img convert tries to
+    #   align the requests to the cluster size, the output is ordered
+    #   differently, so disable it, too.
+    _unsupported_fmt vdi vhdx
 fi