[v2,1/2] qcow2: Force preallocation with data-file-raw

Commit Message

Max Reitz March 26, 2021, 2:55 p.m. UTC

Setting the qcow2 data-file-raw bit means that you can ignore the
qcow2 metadata when reading from the external data file.  It does not
mean that you have to ignore it, though.  Therefore, the data read must
be the same regardless of whether you interpret the metadata or whether
you ignore it, and thus the L1/L2 tables must all be present and give a
1:1 mapping.

This patch changes 244's output: First, the qcow2 file is larger right
after creation, because of metadata preallocation.  Second, the qemu-img
map output changes: Everything that was not explicitly discarded or
zeroed is now a data area.

Signed-off-by: Max Reitz <[e-mail]>
---
 block/qcow2.c              | 34 ++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/244.out |  9 ++++-----
 2 files changed, 38 insertions(+), 5 deletions(-)

Comments

Eric Blake March 26, 2021, 3:13 p.m. UTC | #1

On 3/26/21 9:55 AM, Max Reitz wrote:
> Setting the qcow2 data-file-raw bit means that you can ignore the
> qcow2 metadata when reading from the external data file.  It does not
> mean that you have to ignore it, though.  Therefore, the data read must
> be the same regardless of whether you interpret the metadata or whether
> you ignore it, and thus the L1/L2 tables must all be present and give a
> 1:1 mapping.
> 
> This patch changes 244's output: First, the qcow2 file is larger right
> after creation, because of metadata preallocation.  Second, the qemu-img
> map output changes: Everything that was not explicitly discarded or
> zeroed is now a data area.
> 
> Signed-off-by: Max Reitz <[e-mail]>
> ---
>  block/qcow2.c              | 34 ++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/244.out |  9 ++++-----
>  2 files changed, 38 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake <[e-mail]>

I think this counts as a bug fix worthy of inclusion in 6.0.

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index 0db1227ac9..9920c756eb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3502,6 +3502,28 @@  qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
         ret = -EINVAL;
         goto out;
     }
+    if (qcow2_opts->data_file_raw &&
+        qcow2_opts->preallocation == PREALLOC_MODE_OFF)
+    {
+        /*
+         * data-file-raw means that "the external data file can be
+         * read as a consistent standalone raw image without looking
+         * at the qcow2 metadata."  It does not say that the metadata
+         * must be ignored, though (and the qcow2 driver in fact does
+         * not ignore it), so the L1/L2 tables must be present and
+         * give a 1:1 mapping, so you get the same result regardless
+         * of whether you look at the metadata or whether you ignore
+         * it.
+         */
+        qcow2_opts->preallocation = PREALLOC_MODE_METADATA;
+
+        /*
+         * Cannot use preallocation with backing files, but giving a
+         * backing file when specifying data_file_raw is an error
+         * anyway.
+         */
+        assert(!qcow2_opts->has_backing_file);
+    }
 
     if (qcow2_opts->data_file) {
         if (version < 3) {
@@ -4237,6 +4259,18 @@  static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
             error_setg_errno(errp, -ret, "Failed to grow the L1 table");
             goto fail;
         }
+
+        if (data_file_is_raw(bs) && prealloc == PREALLOC_MODE_OFF) {
+            /*
+             * When creating a qcow2 image with data-file-raw, we enforce
+             * at least prealloc=metadata, so that the L1/L2 tables are
+             * fully allocated and reading from the data file will return
+             * the same data as reading from the qcow2 image.  When the
+             * image is grown, we must consequently preallocate the
+             * metadata structures to cover the added area.
+             */
+            prealloc = PREALLOC_MODE_METADATA;
+        }
     }
 
     switch (prealloc) {
diff --git a/tests/qemu-iotests/244.out b/tests/qemu-iotests/244.out
index 7269b4295a..1a3ae31dde 100644
--- a/tests/qemu-iotests/244.out
+++ b/tests/qemu-iotests/244.out
@@ -83,7 +83,7 @@  qcow2 file size after I/O: 327680
 === Standalone image with external data file (valid raw) ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=on
-qcow2 file size before I/O: 196616
+qcow2 file size before I/O: 327680
 
 wrote 4194304/4194304 bytes at offset 1048576
 4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -93,11 +93,10 @@  wrote 3145728/3145728 bytes at offset 3145728
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 No errors were found on the image.
 
-[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false},
-{ "start": 1048576, "length": 1048576, "depth": 0, "zero": false, "data": true, "offset": 1048576},
+[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": 0},
 { "start": 2097152, "length": 2097152, "depth": 0, "zero": true, "data": false},
-{ "start": 4194304, "length": 1048576, "depth": 0, "zero": true, "data": false, "offset": 4194304},
-{ "start": 5242880, "length": 61865984, "depth": 0, "zero": true, "data": false}]
+{ "start": 4194304, "length": 2097152, "depth": 0, "zero": true, "data": false, "offset": 4194304},
+{ "start": 6291456, "length": 60817408, "depth": 0, "zero": false, "data": true, "offset": 6291456}]
 
 read 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)