[Qemu-block] [PATCH 1/4] qemu-img: Fix preallocation with -S 0 for convert

[Max Reitz]

When passing -S 0 to qemu-img convert, the target image is supposed to
be fully allocated. Right now, this is not the case if the source image
contains areas which bdrv_get_block_status() reports as being zero.

This patch introduces a new ImgConvertBlockStatus named BLK_ZERO_DATA
which is set by convert_iteration_sectors() if an area is detected to be
zero and min_sparse is 0. Simply using BLK_DATA is not optimal, because
knowing an area to be zero allows us to memset() the buffer to be
written directly rather than having to use blk_read().

Other than that, BLK_ZERO_DATA is handled exactly like BLK_DATA.

This patch changes the reference output for iotest 122; contrary to what
it assumed, -S 0 really should allocate everything in the output, not
just areas that are filled with zeros (as opposed to being zeroed).

Signed-off-by: Max Reitz <address@hidden>
---
 qemu-img.c                 | 37 ++++++++++++++++++++++++-------------
 tests/qemu-iotests/122.out |  6 ++----
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index fb9ab1f..809ea3b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1232,6 +1232,10 @@ enum ImgConvertBlockStatus {
     BLK_DATA,
     BLK_ZERO,
     BLK_BACKING_FILE,
+
+    /* Will return zeros when read but must be written as data (i.e. is set for
+     * zeroed areas with min_sparse == 0) */
+    BLK_ZERO_DATA,
 };
 
 typedef struct ImgConvertState {
@@ -1282,7 +1286,13 @@ static int convert_iteration_sectors(ImgConvertState *s, 
int64_t sector_num)
         }
 
         if (ret & BDRV_BLOCK_ZERO) {
-            s->status = BLK_ZERO;
+            if (s->min_sparse) {
+                s->status = BLK_ZERO;
+            } else {
+                /* If min_sparse is 0, this area must be written to the target
+                 * image as data */
+                s->status = BLK_ZERO_DATA;
+            }
         } else if (ret & BDRV_BLOCK_DATA) {
             s->status = BLK_DATA;
         } else if (!s->target_has_backing) {
@@ -1300,7 +1310,7 @@ static int convert_iteration_sectors(ImgConvertState *s, 
int64_t sector_num)
     }
 
     n = MIN(n, s->sector_next_status - sector_num);
-    if (s->status == BLK_DATA) {
+    if (s->status == BLK_DATA || s->status == BLK_ZERO_DATA) {
         n = MIN(n, s->buf_sectors);
     }
 
@@ -1325,10 +1335,6 @@ static int convert_read(ImgConvertState *s, int64_t 
sector_num, int nb_sectors,
     int n;
     int ret;
 
-    if (s->status == BLK_ZERO || s->status == BLK_BACKING_FILE) {
-        return 0;
-    }
-
     assert(nb_sectors <= s->buf_sectors);
     while (nb_sectors > 0) {
         BlockBackend *blk;
@@ -1372,6 +1378,7 @@ static int convert_write(ImgConvertState *s, int64_t 
sector_num, int nb_sectors,
             break;
 
         case BLK_DATA:
+        case BLK_ZERO_DATA:
             /* We must always write compressed clusters as a whole, so don't
              * try to find zeroed parts in the buffer. We can only save the
              * write if the buffer is completely zeroed and we're allowed to
@@ -1466,7 +1473,7 @@ static int convert_do_copy(ImgConvertState *s)
             ret = n;
             goto fail;
         }
-        if (s->status == BLK_DATA) {
+        if (s->status == BLK_DATA || s->status == BLK_ZERO_DATA) {
             s->allocated_sectors += n;
         }
         sector_num += n;
@@ -1486,17 +1493,21 @@ static int convert_do_copy(ImgConvertState *s)
             ret = n;
             goto fail;
         }
-        if (s->status == BLK_DATA) {
+        if (s->status == BLK_DATA || s->status == BLK_ZERO_DATA) {
             allocated_done += n;
             qemu_progress_print(100.0 * allocated_done / s->allocated_sectors,
                                 0);
         }
 
-        ret = convert_read(s, sector_num, n, buf);
-        if (ret < 0) {
-            error_report("error while reading sector %" PRId64
-                         ": %s", sector_num, strerror(-ret));
-            goto fail;
+        if (s->status == BLK_DATA) {
+            ret = convert_read(s, sector_num, n, buf);
+            if (ret < 0) {
+                error_report("error while reading sector %" PRId64
+                             ": %s", sector_num, strerror(-ret));
+                goto fail;
+            }
+        } else if (s->status == BLK_ZERO_DATA) {
+            memset(buf, 0, n * BDRV_SECTOR_SIZE);
         }
 
         ret = convert_write(s, sector_num, n, buf);
diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
index 0068e96..98814de 100644
--- a/tests/qemu-iotests/122.out
+++ b/tests/qemu-iotests/122.out
@@ -112,16 +112,14 @@ read 3145728/3145728 bytes at offset 0
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 63963136/63963136 bytes at offset 3145728
 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
-{ "start": 6291456, "length": 60817408, "depth": 0, "zero": true, "data": 
false}]
+[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, 
"offset": 327680}]
 
 convert -c -S 0:
 read 3145728/3145728 bytes at offset 0
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 63963136/63963136 bytes at offset 3145728
 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true},
-{ "start": 6291456, "length": 60817408, "depth": 0, "zero": true, "data": 
false}]
+[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true}]
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
 wrote 33554432/33554432 bytes at offset 0
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-- 
2.7.1