diff mbox series

[v2,2/2] iotests/244: Test preallocation for data-file-raw

Message ID 20210326145509.163455-3-mreitz@redhat.com
State New
Headers show
Series qcow2: Force preallocation with data-file-raw | expand

Commit Message

Max Reitz March 26, 2021, 2:55 p.m. UTC
Three test cases:
(1) Adding a qcow2 (metadata) file to an existing data file, see whether
    we can read the existing data through the qcow2 image.
(2) Append data to the data file, grow the qcow2 image accordingly, see
    whether we can read the new data through the qcow2 image.
(3) At runtime, add a backing image to a freshly created qcow2 image
    with an external data file (with data-file-raw).  Reading data from
    the qcow2 image must return the same result as reading data from the
    data file, so everything in the backing image must be ignored.
    (This did not use to be the case, because without the L2 tables
    preallocated, all clusters would appear as unallocated, and so the
    qcow2 driver would fall through to the backing file.)

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/244     | 104 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/244.out |  59 +++++++++++++++++++++
 2 files changed, 163 insertions(+)

Comments

Eric Blake March 26, 2021, 3:17 p.m. UTC | #1
On 3/26/21 9:55 AM, Max Reitz wrote:
> Three test cases:
> (1) Adding a qcow2 (metadata) file to an existing data file, see whether
>     we can read the existing data through the qcow2 image.
> (2) Append data to the data file, grow the qcow2 image accordingly, see
>     whether we can read the new data through the qcow2 image.
> (3) At runtime, add a backing image to a freshly created qcow2 image
>     with an external data file (with data-file-raw).  Reading data from
>     the qcow2 image must return the same result as reading data from the
>     data file, so everything in the backing image must be ignored.
>     (This did not use to be the case, because without the L2 tables
>     preallocated, all clusters would appear as unallocated, and so the
>     qcow2 driver would fall through to the backing file.)
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/244     | 104 +++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/244.out |  59 +++++++++++++++++++++
>  2 files changed, 163 insertions(+)
> 

> +
> +# We cannot use qemu-img to create the qcow2 image, because it would
> +# clear the data file.  Use the blockdev-create job instead, which will
> +# only format the qcow2 image file.

Well, perhaps we could use qemu-img to create a qcow2 pointing to a
temporary file, then rewrite it to point to the real data file, but that
feels hackish, and your approach worked.  And besides, while we have
qemu-img rebase -u to rewrite the backing file, I don't know if we have
a similar qemu-img command for rewriting the data file.

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz March 26, 2021, 8:06 p.m. UTC | #2
On 26.03.21 16:17, Eric Blake wrote:
> On 3/26/21 9:55 AM, Max Reitz wrote:
>> Three test cases:
>> (1) Adding a qcow2 (metadata) file to an existing data file, see whether
>>      we can read the existing data through the qcow2 image.
>> (2) Append data to the data file, grow the qcow2 image accordingly, see
>>      whether we can read the new data through the qcow2 image.
>> (3) At runtime, add a backing image to a freshly created qcow2 image
>>      with an external data file (with data-file-raw).  Reading data from
>>      the qcow2 image must return the same result as reading data from the
>>      data file, so everything in the backing image must be ignored.
>>      (This did not use to be the case, because without the L2 tables
>>      preallocated, all clusters would appear as unallocated, and so the
>>      qcow2 driver would fall through to the backing file.)
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/244     | 104 +++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/244.out |  59 +++++++++++++++++++++
>>   2 files changed, 163 insertions(+)
>>
> 
>> +
>> +# We cannot use qemu-img to create the qcow2 image, because it would
>> +# clear the data file.  Use the blockdev-create job instead, which will
>> +# only format the qcow2 image file.
> 
> Well, perhaps we could use qemu-img to create a qcow2 pointing to a
> temporary file, then rewrite it to point to the real data file, but that
> feels hackish, and your approach worked.  And besides, while we have
> qemu-img rebase -u to rewrite the backing file, I don't know if we have
> a similar qemu-img command for rewriting the data file.

Yes, we do, but I decided this would be the cleanest way.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!  I’ll decide on Monday whether to include this in 6.0.  I think 
it would be small enough for rc1, but it’s also no regression, so... 
Well, I’ll see then.

Max
diff mbox series

Patch

diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244
index a46b441627..3e61fa25bb 100755
--- a/tests/qemu-iotests/244
+++ b/tests/qemu-iotests/244
@@ -38,6 +38,7 @@  trap "_cleanup; exit \$status" 0 1 2 3 15
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.qemu
 
 _supported_fmt qcow2
 _supported_proto file
@@ -267,6 +268,109 @@  case $result in
         ;;
 esac
 
+echo
+echo '=== Preallocation with data-file-raw ==='
+
+echo
+echo '--- Using a non-zeroed data file ---'
+
+# Using data-file-raw must enforce at least metadata preallocation so
+# that it does not matter whether one reads the raw file or the qcow2
+# file
+
+# Pre-create the data file, write some data.  Real-world use cases for
+# this are adding a qcow2 metadata file to a block device (i.e., using
+# the device as the data file) or adding qcow2 features to pre-existing
+# raw images (e.g. because the user now wants persistent dirty bitmaps).
+truncate -s 1M "$TEST_IMG.data"
+$QEMU_IO -f raw -c 'write -P 42 0 1M' "$TEST_IMG.data" | _filter_qemu_io
+
+# We cannot use qemu-img to create the qcow2 image, because it would
+# clear the data file.  Use the blockdev-create job instead, which will
+# only format the qcow2 image file.
+touch "$TEST_IMG"
+_launch_qemu \
+    -blockdev file,node-name=data,filename="$TEST_IMG.data" \
+    -blockdev file,node-name=meta,filename="$TEST_IMG"
+
+_send_qemu_cmd $QEMU_HANDLE '{ "execute": "qmp_capabilities" }' 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    '{ "execute": "blockdev-create",
+       "arguments": {
+           "job-id": "create",
+           "options": {
+               "driver": "qcow2",
+               "size": '"$((1 * 1024 * 1024))"',
+               "file": "meta",
+               "data-file": "data",
+               "data-file-raw": true
+           } } }' \
+    '"status": "concluded"'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    '{ "execute": "job-dismiss", "arguments": { "id": "create" } }' \
+    'return'
+
+_cleanup_qemu
+
+echo
+echo 'Comparing pattern:'
+
+# Reading from either the qcow2 file or the data file should return
+# the same result:
+$QEMU_IO -f raw -c 'read -P 42 0 1M' "$TEST_IMG.data" | _filter_qemu_io
+$QEMU_IO -f $IMGFMT -c 'read -P 42 0 1M' "$TEST_IMG" | _filter_qemu_io
+
+# For good measure
+$QEMU_IMG compare -f raw "$TEST_IMG.data" "$TEST_IMG"
+
+echo
+echo '--- Truncation (growing) ---'
+
+# Append some new data to the raw file, then resize the qcow2 image
+# accordingly and see whether the new data is visible.  Technically
+# that is not allowed, but it is reasonable behavior, so test it.
+truncate -s 2M "$TEST_IMG.data"
+$QEMU_IO -f raw -c 'write -P 84 1M 1M' "$TEST_IMG.data" | _filter_qemu_io
+
+$QEMU_IMG resize "$TEST_IMG" 2M
+
+echo
+echo 'Comparing pattern:'
+
+$QEMU_IO -f raw -c 'read -P 42 0 1M' -c 'read -P 84 1M 1M' "$TEST_IMG.data" \
+    | _filter_qemu_io
+$QEMU_IO -f $IMGFMT -c 'read -P 42 0 1M' -c 'read -P 84 1M 1M' "$TEST_IMG" \
+    | _filter_qemu_io
+
+$QEMU_IMG compare -f raw "$TEST_IMG.data" "$TEST_IMG"
+
+echo
+echo '--- Giving a backing file at runtime ---'
+
+# qcow2 files with data-file-raw cannot have backing files given by
+# their image header, but qemu will allow you to set a backing node at
+# runtime -- it should not have any effect, though (because reading
+# from the qcow2 node should return the same data as reading from the
+# raw node).
+
+_make_test_img -o "data_file=$TEST_IMG.data,data_file_raw=on" 1M
+TEST_IMG="$TEST_IMG.base" _make_test_img 1M
+
+# Write something that is not zero into the base image
+$QEMU_IO -c 'write -P 42 0 1M' "$TEST_IMG.base" | _filter_qemu_io
+
+echo
+echo 'Comparing qcow2 image and raw data file:'
+
+# $TEST_IMG and $TEST_IMG.data must show the same data at all times;
+# that is, the qcow2 node must not fall through to the backing image
+# at any point
+$QEMU_IMG compare --image-opts \
+    "driver=raw,file.filename=$TEST_IMG.data"  \
+    "file.filename=$TEST_IMG,backing.file.filename=$TEST_IMG.base"
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/244.out b/tests/qemu-iotests/244.out
index 1a3ae31dde..99f56ac18c 100644
--- a/tests/qemu-iotests/244.out
+++ b/tests/qemu-iotests/244.out
@@ -137,4 +137,63 @@  wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 Success: qemu-io failed, so the data file was flushed
+
+=== Preallocation with data-file-raw ===
+
+--- Using a non-zeroed data file ---
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{ "execute": "qmp_capabilities" }
+{"return": {}}
+{ "execute": "blockdev-create",
+       "arguments": {
+           "job-id": "create",
+           "options": {
+               "driver": "IMGFMT",
+               "size": 1048576,
+               "file": "meta",
+               "data-file": "data",
+               "data-file-raw": true
+           } } }
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "create"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "create"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "create"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "create"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "create"}}
+{ "execute": "job-dismiss", "arguments": { "id": "create" } }
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "create"}}
+{"return": {}}
+
+Comparing pattern:
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Images are identical.
+
+--- Truncation (growing) ---
+wrote 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image resized.
+
+Comparing pattern:
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Images are identical.
+
+--- Giving a backing file at runtime ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 data_file=TEST_DIR/t.IMGFMT.data data_file_raw=on
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Comparing qcow2 image and raw data file:
+Images are identical.
 *** done