[PATCH v2 6/6] iotests: Add test 291 to for qemu-img bitmap coverage

Eric Blake posted 6 patches 5 years, 9 months ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
There is a newer version of this series
[PATCH v2 6/6] iotests: Add test 291 to for qemu-img bitmap coverage
Posted by Eric Blake 5 years, 9 months ago
Add a new test covering the 'qemu-img bitmap' subcommand, as well as
'qemu-img convert --bitmaps', both added in recent patches.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/291     | 103 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/291.out |  78 ++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 182 insertions(+)
 create mode 100755 tests/qemu-iotests/291
 create mode 100644 tests/qemu-iotests/291.out

diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
new file mode 100755
index 000000000000..77713c0cfea7
--- /dev/null
+++ b/tests/qemu-iotests/291
@@ -0,0 +1,103 @@
+#!/usr/bin/env bash
+#
+# Test qemu-img bitmap handling
+#
+# Copyright (C) 2018-2020 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    nbd_server_stop
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.nbd
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+_require_command QEMU_NBD
+
+echo
+echo "=== Initial image setup ==="
+echo
+
+# Create backing image with one bitmap
+TEST_IMG="$TEST_IMG.base" _make_test_img 10M
+$QEMU_IMG bitmap --add -f $IMGFMT "$TEST_IMG.base" b0
+$QEMU_IO -c 'w 3M 1M' -f $IMGFMT "$TEST_IMG.base" | _filter_qemu_io
+# Create initial image and populate two bitmaps: one active, one inactive.
+_make_test_img -b "$TEST_IMG.base" -F $IMGFMT 10M
+$QEMU_IO -c 'w 0 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG bitmap --add -g 512k -f $IMGFMT "$TEST_IMG" b1
+$QEMU_IMG bitmap --add --disabled -f $IMGFMT "$TEST_IMG" b2
+$QEMU_IO -c 'w 3M 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG bitmap --clear -f $IMGFMT "$TEST_IMG" b1
+$QEMU_IO -c 'w 1M 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG bitmap --disable -f $IMGFMT "$TEST_IMG" b1
+$QEMU_IMG bitmap --enable -f $IMGFMT "$TEST_IMG" b2
+$QEMU_IO -c 'w 2M 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "=== Bitmap preservation not possible to non-qcow2 ==="
+echo
+
+mv "$TEST_IMG" "$TEST_IMG.orig"
+$QEMU_IMG convert --bitmaps -O raw "$TEST_IMG.orig" "$TEST_IMG"
+
+echo
+echo "=== Convert with bitmap preservation ==="
+echo
+
+# Only bitmaps from the active layer are copied
+$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG.orig" "$TEST_IMG"
+$QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
+# But we can also merge in bitmaps from other layers
+$QEMU_IMG bitmap --add --disabled -f $IMGFMT "$TEST_IMG" b0
+$QEMU_IMG bitmap --add -f $IMGFMT "$TEST_IMG" tmp
+$QEMU_IMG bitmap --merge b0 -b "$TEST_IMG.base" -F $IMGFMT "$TEST_IMG" tmp
+$QEMU_IMG bitmap --merge tmp "$TEST_IMG" b0
+$QEMU_IMG bitmap --remove -f $IMGFMT "$TEST_IMG" tmp
+$QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
+
+echo
+echo "=== Check bitmap contents ==="
+echo
+
+IMG="driver=nbd,server.type=unix,server.path=$nbd_unix_socket"
+nbd_server_start_unix_socket -r -f qcow2 -B b0 "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+    "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b0" | _filter_qemu_img_map
+nbd_server_start_unix_socket -r -f qcow2 -B b1 "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+    "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b1" | _filter_qemu_img_map
+nbd_server_start_unix_socket -r -f qcow2 -B b2 "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+    "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b2" | _filter_qemu_img_map
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
new file mode 100644
index 000000000000..d716c0c7cc0b
--- /dev/null
+++ b/tests/qemu-iotests/291.out
@@ -0,0 +1,78 @@
+QA output created by 291
+
+=== Initial image setup ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=10485760
+wrote 1048576/1048576 bytes at offset 3145728
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=10485760 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 3145728
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 2097152
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Bitmap preservation not possible to non-qcow2 ===
+
+qemu-img: Format driver 'raw' does not support bitmaps
+
+=== Convert with bitmap preservation ===
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 10 MiB (10485760 bytes)
+disk size: 4.39 MiB
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+    bitmaps:
+        [0]:
+            flags:
+            name: b1
+            granularity: 524288
+        [1]:
+            flags:
+                [0]: auto
+            name: b2
+            granularity: 65536
+    refcount bits: 16
+    corrupt: false
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 10 MiB (10485760 bytes)
+disk size: 4.49 MiB
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+    bitmaps:
+        [0]:
+            flags:
+            name: b1
+            granularity: 524288
+        [1]:
+            flags:
+                [0]: auto
+            name: b2
+            granularity: 65536
+        [2]:
+            flags:
+            name: b0
+            granularity: 65536
+    refcount bits: 16
+    corrupt: false
+
+=== Check bitmap contents ===
+
+[{ "start": 0, "length": 3145728, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 3145728, "length": 1048576, "depth": 0, "zero": false, "data": false},
+{ "start": 4194304, "length": 6291456, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 1048576, "length": 1048576, "depth": 0, "zero": false, "data": false},
+{ "start": 2097152, "length": 8388608, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 2097152, "length": 1048576, "depth": 0, "zero": false, "data": false},
+{ "start": 3145728, "length": 7340032, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 435dccd5af90..8e9b9513a091 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -297,3 +297,4 @@
 288 quick
 289 rw quick
 290 rw auto quick
+291 rw quick
-- 
2.26.2


Re: [PATCH v2 6/6] iotests: Add test 291 to for qemu-img bitmap coverage
Posted by Max Reitz 5 years, 9 months ago
On 21.04.20 23:20, Eric Blake wrote:
> Add a new test covering the 'qemu-img bitmap' subcommand, as well as
> 'qemu-img convert --bitmaps', both added in recent patches.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/qemu-iotests/291     | 103 +++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/291.out |  78 ++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 182 insertions(+)
>  create mode 100755 tests/qemu-iotests/291
>  create mode 100644 tests/qemu-iotests/291.out
> 
> diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
> new file mode 100755
> index 000000000000..77713c0cfea7
> --- /dev/null
> +++ b/tests/qemu-iotests/291

[...]

> +echo
> +echo "=== Bitmap preservation not possible to non-qcow2 ==="
> +echo
> +
> +mv "$TEST_IMG" "$TEST_IMG.orig"

“mv” doesn’t work images with external data files.

(ORIG_IMG=$TEST_IMG; TEST_IMG="$TEST_IMG".orig should work)

> +$QEMU_IMG convert --bitmaps -O raw "$TEST_IMG.orig" "$TEST_IMG"
> +
> +echo
> +echo "=== Convert with bitmap preservation ==="
> +echo
> +
> +# Only bitmaps from the active layer are copied

That’s kind of obvious when you think about (whenever an image is
attached to a VM, only the active layer’s bitmaps are visible, not those
from the backing chain), but maybe this should be noted in the
documentation?

> +$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG.orig" "$TEST_IMG"
> +$QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
> +# But we can also merge in bitmaps from other layers
> +$QEMU_IMG bitmap --add --disabled -f $IMGFMT "$TEST_IMG" b0
> +$QEMU_IMG bitmap --add -f $IMGFMT "$TEST_IMG" tmp
> +$QEMU_IMG bitmap --merge b0 -b "$TEST_IMG.base" -F $IMGFMT "$TEST_IMG" tmp
> +$QEMU_IMG bitmap --merge tmp "$TEST_IMG" b0
> +$QEMU_IMG bitmap --remove -f $IMGFMT "$TEST_IMG" tmp

Why do we need tmp here?  Can’t we just merge base’s b0 directly into
$TEST_IMG’s b0?

[...]

> diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
> new file mode 100644
> index 000000000000..d716c0c7cc0b
> --- /dev/null
> +++ b/tests/qemu-iotests/291.out

[...]

> +=== Check bitmap contents ===
> +
> +[{ "start": 0, "length": 3145728, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
> +{ "start": 3145728, "length": 1048576, "depth": 0, "zero": false, "data": false},
> +{ "start": 4194304, "length": 6291456, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
> +[{ "start": 0, "length": 1048576, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
> +{ "start": 1048576, "length": 1048576, "depth": 0, "zero": false, "data": false},
> +{ "start": 2097152, "length": 8388608, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
> +[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
> +{ "start": 2097152, "length": 1048576, "depth": 0, "zero": false, "data": false},
> +{ "start": 3145728, "length": 7340032, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]

Am I looking at this wrong or does the bitmap data seem to be inverted?
 Everywhere where I’d expect the bitmaps to be cleared, this map reports
data=true, whereas where I’d expect them to be set, it reports data=false.

I suppose that’s intentional, but can you explain this behavior to me?

Max

> +*** done

Re: [PATCH v2 6/6] iotests: Add test 291 to for qemu-img bitmap coverage
Posted by Eric Blake 5 years, 9 months ago
On 5/4/20 8:05 AM, Max Reitz wrote:
> On 21.04.20 23:20, Eric Blake wrote:
>> Add a new test covering the 'qemu-img bitmap' subcommand, as well as
>> 'qemu-img convert --bitmaps', both added in recent patches.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>

>> +echo
>> +echo "=== Bitmap preservation not possible to non-qcow2 ==="
>> +echo
>> +
>> +mv "$TEST_IMG" "$TEST_IMG.orig"
> 
> “mv” doesn’t work images with external data files.
> 
> (ORIG_IMG=$TEST_IMG; TEST_IMG="$TEST_IMG".orig should work)

Good idea.

> 
>> +$QEMU_IMG convert --bitmaps -O raw "$TEST_IMG.orig" "$TEST_IMG"
>> +
>> +echo
>> +echo "=== Convert with bitmap preservation ==="
>> +echo
>> +
>> +# Only bitmaps from the active layer are copied
> 
> That’s kind of obvious when you think about (whenever an image is
> attached to a VM, only the active layer’s bitmaps are visible, not those
> from the backing chain), but maybe this should be noted in the
> documentation?

As part of integrating bitmaps with external snapshots, libvirt actually 
depends on being able to see bitmaps from the backing chain - but as 
bitmaps are always referenced as a 'node name, bitmap name' tuple, this 
is indeed doable.

> 
>> +$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG.orig" "$TEST_IMG"
>> +$QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
>> +# But we can also merge in bitmaps from other layers
>> +$QEMU_IMG bitmap --add --disabled -f $IMGFMT "$TEST_IMG" b0
>> +$QEMU_IMG bitmap --add -f $IMGFMT "$TEST_IMG" tmp
>> +$QEMU_IMG bitmap --merge b0 -b "$TEST_IMG.base" -F $IMGFMT "$TEST_IMG" tmp
>> +$QEMU_IMG bitmap --merge tmp "$TEST_IMG" b0
>> +$QEMU_IMG bitmap --remove -f $IMGFMT "$TEST_IMG" tmp
> 
> Why do we need tmp here?  Can’t we just merge base’s b0 directly into
> $TEST_IMG’s b0?

Yes, we could.  But then I wouldn't cover as many bitmap subcommands. 
Adding a comment about why the example is contrived (for maximal 
coverage) is a good idea.


>> +=== Check bitmap contents ===
>> +
>> +[{ "start": 0, "length": 3145728, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
>> +{ "start": 3145728, "length": 1048576, "depth": 0, "zero": false, "data": false},
>> +{ "start": 4194304, "length": 6291456, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
>> +[{ "start": 0, "length": 1048576, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
>> +{ "start": 1048576, "length": 1048576, "depth": 0, "zero": false, "data": false},
>> +{ "start": 2097152, "length": 8388608, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
>> +[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
>> +{ "start": 2097152, "length": 1048576, "depth": 0, "zero": false, "data": false},
>> +{ "start": 3145728, "length": 7340032, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
> 
> Am I looking at this wrong or does the bitmap data seem to be inverted?
>   Everywhere where I’d expect the bitmaps to be cleared, this map reports
> data=true, whereas where I’d expect them to be set, it reports data=false.
> 
> I suppose that’s intentional, but can you explain this behavior to me?

This is an artifact of how x-dirty-bitmap works (it has the x- prefix 
because it is a hack, but we don't have anything better for reading out 
bitmap contents).  The NBD spec returns block status as a 32-bit value 
for a 'metadata context'; normally, we use context 'base:allocation' 
context where bit 0 is set for holes or clear for allocated, and bit 1 
is set for reads-as-zero or clear for unknown contents (favoring all-0 
as the most-common case).  But with x-dirty-bitmap, we are instead 
abusing NBD to query the 'qemu:dirty-bitmap:FOO' context, where bit 0 is 
set for anywhere the bitmap has a 1, yet feed that information into the 
pre-existing qemu code for handling block status.  So qemu-img map is 
reporting "data":true for what it thinks is the normal 0-for-allocated, 
and "data":false for 1-for-sparse, and we just have to translate that 
back into an understanding of what the bitmap reported.  Yes, a comment 
would be helpful.

I would _really_ love to enhance 'qemu-img map' to output image-specific 
metadata _in addition_ to the existing "zero" and "data" fields (by 
having qemu-img read two NBD contexts at once: both base:allocation and 
qemu:dirty-bitmap:FOO), at which point we can drop the x- prefix and 
avoid the abuse of qemu's internals by overwriting the block_status 
code.  But that's a bigger project.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org