[PATCH 2/3] iotests: Test qemu-img checksum

Nir Soffer posted 3 patches 3 years, 5 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
[PATCH 2/3] iotests: Test qemu-img checksum
Posted by Nir Soffer 3 years, 5 months ago
Add simple tests creating an image with all kinds of extents, different
formats, different backing chain, different protocol, and different
image options. Since all images have the same guest visible content they
must have the same checksum.

To help debugging in case of failures, the output includes a json map of
every test image.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 tests/qemu-iotests/tests/qemu-img-checksum    | 149 ++++++++++++++++++
 .../qemu-iotests/tests/qemu-img-checksum.out  |  74 +++++++++
 2 files changed, 223 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/qemu-img-checksum
 create mode 100644 tests/qemu-iotests/tests/qemu-img-checksum.out

diff --git a/tests/qemu-iotests/tests/qemu-img-checksum b/tests/qemu-iotests/tests/qemu-img-checksum
new file mode 100755
index 0000000000..3a85ba33f2
--- /dev/null
+++ b/tests/qemu-iotests/tests/qemu-img-checksum
@@ -0,0 +1,149 @@
+#!/usr/bin/env python3
+# group: rw auto quick
+#
+# Test cases for qemu-img checksum.
+#
+# Copyright (C) 2022 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/>.
+
+import re
+
+import iotests
+
+from iotests import (
+    filter_testfiles,
+    qemu_img,
+    qemu_img_log,
+    qemu_io,
+    qemu_nbd_popen,
+)
+
+
+def checksum_available():
+    out = qemu_img("--help").stdout
+    return re.search(r"\bchecksum .+ filename\b", out) is not None
+
+
+if not checksum_available():
+    iotests.notrun("checksum command not available")
+
+iotests.script_initialize(
+    supported_fmts=["raw", "qcow2"],
+    supported_cache_modes=["none", "writeback"],
+    supported_protocols=["file", "nbd"],
+    required_fmts=["raw", "qcow2"],
+)
+
+print()
+print("=== Test images ===")
+print()
+
+disk_raw = iotests.file_path('raw')
+qemu_img("create", "-f", "raw", disk_raw, "10m")
+qemu_io("-f", "raw",
+        "-c", "write -P 0x1 0 2m",      # data
+        "-c", "write -P 0x0 2m 2m",     # data with zeroes
+        "-c", "write -z 4m 2m",         # zero allocated
+        "-c", "write -z -u 6m 2m",      # zero hole
+                                        # unallocated
+        disk_raw)
+print(filter_testfiles(disk_raw))
+qemu_img_log("map", "--output", "json", disk_raw)
+
+disk_qcow2 = iotests.file_path('qcow2')
+qemu_img("create", "-f", "qcow2", disk_qcow2, "10m")
+qemu_io("-f", "qcow2",
+        "-c", "write -P 0x1 0 2m",      # data
+        "-c", "write -P 0x0 2m 2m",     # data with zeroes
+        "-c", "write -z 4m 2m",         # zero allocated
+        "-c", "write -z -u 6m 2m",      # zero hole
+                                        # unallocated
+        disk_qcow2)
+print(filter_testfiles(disk_qcow2))
+qemu_img_log("map", "--output", "json", disk_qcow2)
+
+disk_compressed = iotests.file_path('compressed')
+qemu_img("convert", "-f", "qcow2", "-O", "qcow2", "-c",
+         disk_qcow2, disk_compressed)
+print(filter_testfiles(disk_compressed))
+qemu_img_log("map", "--output", "json", disk_compressed)
+
+disk_base = iotests.file_path('base')
+qemu_img("create", "-f", "raw", disk_base, "10m")
+qemu_io("-f", "raw",
+        "-c", "write -P 0x1 0 2m",
+        "-c", "write -P 0x0 2m 2m",
+        disk_base)
+print(filter_testfiles(disk_base))
+qemu_img_log("map", "--output", "json", disk_base)
+
+disk_top = iotests.file_path('top')
+qemu_img("create", "-f", "qcow2", "-b", disk_base, "-F", "raw",
+         disk_top)
+qemu_io("-f", "qcow2",
+        "-c", "write -z 4m 2m",
+        "-c", "write -z -u 6m 2m",
+        disk_top)
+print(filter_testfiles(disk_top))
+qemu_img_log("map", "--output", "json", disk_top)
+
+print()
+print("=== Checksums - file ===")
+print()
+
+qemu_img_log("checksum", disk_raw)
+qemu_img_log("checksum", disk_qcow2)
+qemu_img_log("checksum", disk_compressed)
+qemu_img_log("checksum", disk_top)
+qemu_img_log("checksum", disk_base)
+
+print()
+print("=== Checksums - nbd ===")
+print()
+
+nbd_sock = iotests.file_path("nbd.sock", base_dir=iotests.sock_dir)
+nbd_uri = f"nbd+unix:///{{}}?socket={nbd_sock}"
+
+with qemu_nbd_popen("-k", nbd_sock, "-f", "raw", "-x", "raw", disk_raw):
+    qemu_img_log("checksum", nbd_uri.format("raw"))
+
+with qemu_nbd_popen("-k", nbd_sock, "-f", "qcow2", "-x", "qcow2", disk_qcow2):
+    qemu_img_log("checksum", nbd_uri.format("qcow2"))
+
+with qemu_nbd_popen("-k", nbd_sock, "-f", "qcow2", "-x", "compressed", disk_compressed):
+    qemu_img_log("checksum", nbd_uri.format("compressed"))
+
+with qemu_nbd_popen("-k", nbd_sock, "-f", "raw", "-x", "base", disk_base):
+    qemu_img_log("checksum", nbd_uri.format("base"))
+
+with qemu_nbd_popen("-k", nbd_sock, "-f", "qcow2", "-x", "top", disk_top):
+    qemu_img_log("checksum", nbd_uri.format("top"))
+
+print()
+print("=== Command line options ===")
+print()
+
+qemu_img_log("checksum", "-f", "qcow2", disk_top)
+qemu_img_log("checksum", "-T", "none", disk_top)
+
+out = qemu_img("checksum", "-p", disk_top).stdout
+last = out.splitlines()[-1]  # Filter progress lines.
+print(filter_testfiles(last))
+
+print()
+print("=== Incorrect usage ===")
+print()
+
+qemu_img_log("checksum", "-f", "qcow2", disk_raw, check=False)
diff --git a/tests/qemu-iotests/tests/qemu-img-checksum.out b/tests/qemu-iotests/tests/qemu-img-checksum.out
new file mode 100644
index 0000000000..2cff03439f
--- /dev/null
+++ b/tests/qemu-iotests/tests/qemu-img-checksum.out
@@ -0,0 +1,74 @@
+
+=== Test images ===
+
+TEST_DIR/PID-raw
+[{ "start": 0, "length": 4194304, "depth": 0, "present": true, "zero": false, "data": true, "offset": 0},
+{ "start": 4194304, "length": 6291456, "depth": 0, "present": true, "zero": true, "data": false, "offset": 4194304}]
+
+TEST_DIR/PID-qcow2
+[{ "start": 0, "length": 4194304, "depth": 0, "present": true, "zero": false, "data": true, "offset": 327680},
+{ "start": 4194304, "length": 4194304, "depth": 0, "present": true, "zero": true, "data": false},
+{ "start": 8388608, "length": 2097152, "depth": 0, "present": false, "zero": true, "data": false}]
+
+TEST_DIR/PID-compressed
+[{ "start": 0, "length": 2097152, "depth": 0, "present": true, "zero": false, "data": true},
+{ "start": 2097152, "length": 8388608, "depth": 0, "present": false, "zero": true, "data": false}]
+
+TEST_DIR/PID-base
+[{ "start": 0, "length": 4194304, "depth": 0, "present": true, "zero": false, "data": true, "offset": 0},
+{ "start": 4194304, "length": 6291456, "depth": 0, "present": true, "zero": true, "data": false, "offset": 4194304}]
+
+TEST_DIR/PID-top
+[{ "start": 0, "length": 4194304, "depth": 1, "present": true, "zero": false, "data": true, "offset": 0},
+{ "start": 4194304, "length": 4194304, "depth": 0, "present": true, "zero": true, "data": false},
+{ "start": 8388608, "length": 2097152, "depth": 1, "present": true, "zero": true, "data": false, "offset": 8388608}]
+
+
+=== Checksums - file ===
+
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  TEST_DIR/PID-raw
+
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  TEST_DIR/PID-qcow2
+
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  TEST_DIR/PID-compressed
+
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  TEST_DIR/PID-top
+
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  TEST_DIR/PID-base
+
+
+=== Checksums - nbd ===
+
+Start NBD server
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  nbd+unix:///raw?socket=SOCK_DIR/PID-nbd.sock
+
+Kill NBD server
+Start NBD server
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  nbd+unix:///qcow2?socket=SOCK_DIR/PID-nbd.sock
+
+Kill NBD server
+Start NBD server
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  nbd+unix:///compressed?socket=SOCK_DIR/PID-nbd.sock
+
+Kill NBD server
+Start NBD server
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  nbd+unix:///base?socket=SOCK_DIR/PID-nbd.sock
+
+Kill NBD server
+Start NBD server
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  nbd+unix:///top?socket=SOCK_DIR/PID-nbd.sock
+
+Kill NBD server
+
+=== Command line options ===
+
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  TEST_DIR/PID-top
+
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  TEST_DIR/PID-top
+
+57cd8ef0cfad106d737f8fb0de3a0306a8a1a41db7bf7c0c36e2dfe75ee9bd26  TEST_DIR/PID-top
+
+=== Incorrect usage ===
+
+qemu-img: Could not open 'TEST_DIR/PID-raw': Image is not in qcow2 format
+
-- 
2.37.2
Re: [PATCH 2/3] iotests: Test qemu-img checksum
Posted by Hanna Reitz 3 years, 3 months ago
On 01.09.22 16:32, Nir Soffer wrote:
> Add simple tests creating an image with all kinds of extents, different
> formats, different backing chain, different protocol, and different
> image options. Since all images have the same guest visible content they
> must have the same checksum.
>
> To help debugging in case of failures, the output includes a json map of
> every test image.
>
> Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> ---
>   tests/qemu-iotests/tests/qemu-img-checksum    | 149 ++++++++++++++++++
>   .../qemu-iotests/tests/qemu-img-checksum.out  |  74 +++++++++
>   2 files changed, 223 insertions(+)
>   create mode 100755 tests/qemu-iotests/tests/qemu-img-checksum
>   create mode 100644 tests/qemu-iotests/tests/qemu-img-checksum.out
>
> diff --git a/tests/qemu-iotests/tests/qemu-img-checksum b/tests/qemu-iotests/tests/qemu-img-checksum
> new file mode 100755
> index 0000000000..3a85ba33f2
> --- /dev/null
> +++ b/tests/qemu-iotests/tests/qemu-img-checksum
> @@ -0,0 +1,149 @@
> +#!/usr/bin/env python3
> +# group: rw auto quick
> +#
> +# Test cases for qemu-img checksum.
> +#
> +# Copyright (C) 2022 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/>.
> +
> +import re
> +
> +import iotests
> +
> +from iotests import (
> +    filter_testfiles,
> +    qemu_img,
> +    qemu_img_log,
> +    qemu_io,
> +    qemu_nbd_popen,
> +)
> +
> +
> +def checksum_available():
> +    out = qemu_img("--help").stdout
> +    return re.search(r"\bchecksum .+ filename\b", out) is not None
> +
> +
> +if not checksum_available():
> +    iotests.notrun("checksum command not available")
> +
> +iotests.script_initialize(
> +    supported_fmts=["raw", "qcow2"],
> +    supported_cache_modes=["none", "writeback"],

It doesn’t work with writeback, though, because it uses -T none below.

Which by the way is a heavy cost, because I usually run tests in tmpfs, 
where this won’t work.  Is there any way of not doing the -T none below?

> +    supported_protocols=["file", "nbd"],
> +    required_fmts=["raw", "qcow2"],
> +)
> +
> +print()
> +print("=== Test images ===")
> +print()
> +
> +disk_raw = iotests.file_path('raw')
> +qemu_img("create", "-f", "raw", disk_raw, "10m")
> +qemu_io("-f", "raw",
> +        "-c", "write -P 0x1 0 2m",      # data
> +        "-c", "write -P 0x0 2m 2m",     # data with zeroes
> +        "-c", "write -z 4m 2m",         # zero allocated
> +        "-c", "write -z -u 6m 2m",      # zero hole
> +                                        # unallocated
> +        disk_raw)
> +print(filter_testfiles(disk_raw))
> +qemu_img_log("map", "--output", "json", disk_raw)
> +
> +disk_qcow2 = iotests.file_path('qcow2')
> +qemu_img("create", "-f", "qcow2", disk_qcow2, "10m")
> +qemu_io("-f", "qcow2",
> +        "-c", "write -P 0x1 0 2m",      # data
> +        "-c", "write -P 0x0 2m 2m",     # data with zeroes
> +        "-c", "write -z 4m 2m",         # zero allocated
> +        "-c", "write -z -u 6m 2m",      # zero hole
> +                                        # unallocated
> +        disk_qcow2)
> +print(filter_testfiles(disk_qcow2))
> +qemu_img_log("map", "--output", "json", disk_qcow2)

This isn’t how iotests work, generally.  When run with -qcow2 -file, it 
should only test qcow2 on file, not raw on file, not raw on nbd. Perhaps 
this way this test could even support other formats than qcow2 and raw.

Hanna


Re: [PATCH 2/3] iotests: Test qemu-img checksum
Posted by Nir Soffer 3 years, 3 months ago
On Wed, Oct 26, 2022 at 4:31 PM Hanna Reitz <hreitz@redhat.com> wrote:

> On 01.09.22 16:32, Nir Soffer wrote:
> > Add simple tests creating an image with all kinds of extents, different
> > formats, different backing chain, different protocol, and different
> > image options. Since all images have the same guest visible content they
> > must have the same checksum.
> >
> > To help debugging in case of failures, the output includes a json map of
> > every test image.
> >
> > Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> > ---
> >   tests/qemu-iotests/tests/qemu-img-checksum    | 149 ++++++++++++++++++
> >   .../qemu-iotests/tests/qemu-img-checksum.out  |  74 +++++++++
> >   2 files changed, 223 insertions(+)
> >   create mode 100755 tests/qemu-iotests/tests/qemu-img-checksum
> >   create mode 100644 tests/qemu-iotests/tests/qemu-img-checksum.out
> >
> > diff --git a/tests/qemu-iotests/tests/qemu-img-checksum
> b/tests/qemu-iotests/tests/qemu-img-checksum
> > new file mode 100755
> > index 0000000000..3a85ba33f2
> > --- /dev/null
> > +++ b/tests/qemu-iotests/tests/qemu-img-checksum
> > @@ -0,0 +1,149 @@
> > +#!/usr/bin/env python3
> > +# group: rw auto quick
> > +#
> > +# Test cases for qemu-img checksum.
> > +#
> > +# Copyright (C) 2022 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/>.
> > +
> > +import re
> > +
> > +import iotests
> > +
> > +from iotests import (
> > +    filter_testfiles,
> > +    qemu_img,
> > +    qemu_img_log,
> > +    qemu_io,
> > +    qemu_nbd_popen,
> > +)
> > +
> > +
> > +def checksum_available():
> > +    out = qemu_img("--help").stdout
> > +    return re.search(r"\bchecksum .+ filename\b", out) is not None
> > +
> > +
> > +if not checksum_available():
> > +    iotests.notrun("checksum command not available")
> > +
> > +iotests.script_initialize(
> > +    supported_fmts=["raw", "qcow2"],
> > +    supported_cache_modes=["none", "writeback"],
>
> It doesn’t work with writeback, though, because it uses -T none below.
>

Good point


>
> Which by the way is a heavy cost, because I usually run tests in tmpfs,
> where this won’t work.  Is there any way of not doing the -T none below?
>

Testing using tempfs is problematic since you cannot test -T none. In oVirt
we alway use /var/tmp which usually uses something that supports direct I/O.

Do we have a way to specify cache mode in the tests, so we can use -T none
only when the option is set?


>
> > +    supported_protocols=["file", "nbd"],
> > +    required_fmts=["raw", "qcow2"],
> > +)
> > +
> > +print()
> > +print("=== Test images ===")
> > +print()
> > +
> > +disk_raw = iotests.file_path('raw')
> > +qemu_img("create", "-f", "raw", disk_raw, "10m")
> > +qemu_io("-f", "raw",
> > +        "-c", "write -P 0x1 0 2m",      # data
> > +        "-c", "write -P 0x0 2m 2m",     # data with zeroes
> > +        "-c", "write -z 4m 2m",         # zero allocated
> > +        "-c", "write -z -u 6m 2m",      # zero hole
> > +                                        # unallocated
> > +        disk_raw)
> > +print(filter_testfiles(disk_raw))
> > +qemu_img_log("map", "--output", "json", disk_raw)
> > +
> > +disk_qcow2 = iotests.file_path('qcow2')
> > +qemu_img("create", "-f", "qcow2", disk_qcow2, "10m")
> > +qemu_io("-f", "qcow2",
> > +        "-c", "write -P 0x1 0 2m",      # data
> > +        "-c", "write -P 0x0 2m 2m",     # data with zeroes
> > +        "-c", "write -z 4m 2m",         # zero allocated
> > +        "-c", "write -z -u 6m 2m",      # zero hole
> > +                                        # unallocated
> > +        disk_qcow2)
> > +print(filter_testfiles(disk_qcow2))
> > +qemu_img_log("map", "--output", "json", disk_qcow2)
>
> This isn’t how iotests work, generally.  When run with -qcow2 -file, it
> should only test qcow2 on file, not raw on file, not raw on nbd. Perhaps
> this way this test could even support other formats than qcow2 and raw.
>

For this type of tests, running the same code with raw, qcow2 (and other
formats)
and using file or nbd is the best way to test this feature - one test
verifies all the
use cases.

I can change this to use the current format (raw, qcow2, ...), protocol
(file, nbd, ...)
and cache value (none, writeback), but I'm not sure how this can work with
the
out files. The output from nbd is different from file. Maybe we need
different out
files for file and nbd (qemu-img-checksum.file.out,
qemu-img-checksum.nbd.out)?

Nir
Re: [PATCH 2/3] iotests: Test qemu-img checksum
Posted by Hanna Reitz 3 years, 3 months ago
On 30.10.22 18:38, Nir Soffer wrote:
> On Wed, Oct 26, 2022 at 4:31 PM Hanna Reitz <hreitz@redhat.com> wrote:
>
>     On 01.09.22 16:32, Nir Soffer wrote:
>     > Add simple tests creating an image with all kinds of extents,
>     different
>     > formats, different backing chain, different protocol, and different
>     > image options. Since all images have the same guest visible
>     content they
>     > must have the same checksum.
>     >
>     > To help debugging in case of failures, the output includes a
>     json map of
>     > every test image.
>     >
>     > Signed-off-by: Nir Soffer <nsoffer@redhat.com>
>     > ---
>     >   tests/qemu-iotests/tests/qemu-img-checksum    | 149
>     ++++++++++++++++++
>     >   .../qemu-iotests/tests/qemu-img-checksum.out  |  74 +++++++++
>     >   2 files changed, 223 insertions(+)
>     >   create mode 100755 tests/qemu-iotests/tests/qemu-img-checksum
>     >   create mode 100644 tests/qemu-iotests/tests/qemu-img-checksum.out
>     >
>     > diff --git a/tests/qemu-iotests/tests/qemu-img-checksum
>     b/tests/qemu-iotests/tests/qemu-img-checksum
>     > new file mode 100755
>     > index 0000000000..3a85ba33f2
>     > --- /dev/null
>     > +++ b/tests/qemu-iotests/tests/qemu-img-checksum
>     > @@ -0,0 +1,149 @@
>     > +#!/usr/bin/env python3
>     > +# group: rw auto quick
>     > +#
>     > +# Test cases for qemu-img checksum.
>     > +#
>     > +# Copyright (C) 2022 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/>.
>     > +
>     > +import re
>     > +
>     > +import iotests
>     > +
>     > +from iotests import (
>     > +    filter_testfiles,
>     > +    qemu_img,
>     > +    qemu_img_log,
>     > +    qemu_io,
>     > +    qemu_nbd_popen,
>     > +)
>     > +
>     > +
>     > +def checksum_available():
>     > +    out = qemu_img("--help").stdout
>     > +    return re.search(r"\bchecksum .+ filename\b", out) is not None
>     > +
>     > +
>     > +if not checksum_available():
>     > +    iotests.notrun("checksum command not available")
>     > +
>     > +iotests.script_initialize(
>     > +    supported_fmts=["raw", "qcow2"],
>     > +    supported_cache_modes=["none", "writeback"],
>
>     It doesn’t work with writeback, though, because it uses -T none below.
>
>
> Good point
>
>
>     Which by the way is a heavy cost, because I usually run tests in
>     tmpfs,
>     where this won’t work.  Is there any way of not doing the -T none
>     below?
>
>
> Testing using tempfs is problematic since you cannot test -T none.In 
> oVirt
> we alway use /var/tmp which usually uses something that supports 
> direct I/O.
>
> Do we have a way to specify cache mode in the tests, so we can use -T none
> only when the option is set?

`./check` has a `-c` option (e.g. `./check -c none`), which lands in 
`iotests.cachemode`.  That isn’t automatically passed to qemu-img calls, 
but you can do it manually (i.e. `qemu_img_log("checksum", "-T", 
iotests.cachemode, disk_top)` instead of `"-T", "none"`).

>
>     > +    supported_protocols=["file", "nbd"],
>     > +    required_fmts=["raw", "qcow2"],
>     > +)
>     > +
>     > +print()
>     > +print("=== Test images ===")
>     > +print()
>     > +
>     > +disk_raw = iotests.file_path('raw')
>     > +qemu_img("create", "-f", "raw", disk_raw, "10m")
>     > +qemu_io("-f", "raw",
>     > +        "-c", "write -P 0x1 0 2m",      # data
>     > +        "-c", "write -P 0x0 2m 2m",     # data with zeroes
>     > +        "-c", "write -z 4m 2m",         # zero allocated
>     > +        "-c", "write -z -u 6m 2m",      # zero hole
>     > +                                        # unallocated
>     > +        disk_raw)
>     > +print(filter_testfiles(disk_raw))
>     > +qemu_img_log("map", "--output", "json", disk_raw)
>     > +
>     > +disk_qcow2 = iotests.file_path('qcow2')
>     > +qemu_img("create", "-f", "qcow2", disk_qcow2, "10m")
>     > +qemu_io("-f", "qcow2",
>     > +        "-c", "write -P 0x1 0 2m",      # data
>     > +        "-c", "write -P 0x0 2m 2m",     # data with zeroes
>     > +        "-c", "write -z 4m 2m",         # zero allocated
>     > +        "-c", "write -z -u 6m 2m",      # zero hole
>     > +                                        # unallocated
>     > +        disk_qcow2)
>     > +print(filter_testfiles(disk_qcow2))
>     > +qemu_img_log("map", "--output", "json", disk_qcow2)
>
>     This isn’t how iotests work, generally.  When run with -qcow2
>     -file, it
>     should only test qcow2 on file, not raw on file, not raw on nbd.
>     Perhaps
>     this way this test could even support other formats than qcow2 and
>     raw.
>
>
> For this type of tests, running the same code with raw, qcow2 (and 
> other formats)
> and using file or nbd is the best way to test this feature - one test 
> verifies all the
> use cases.

Yes, I see, but that’s a general thing in the iotests.  The design is 
such that tests don’t cycle through their test matrix themselves, but 
that they always only test a single combination of format+protocol on 
each run, and the user is responsible for cycling through the desired 
test matrix.

I’m not saying that was definitely the best design decision, but the 
problem now that if a test cycles through its test matrix by itself, it 
must also ensure that it is run only once when the user cycles through 
the same test matrix.  For example, a reasonable run of the iotests 
consists of `./check -raw`, `./check -qcow2`, and `./check -nbd`.  This 
test here would then run in all three configurations, but do the same 
thing every time (specifically, test all of those configurations every 
time).

So there’s a conflict.  Either the test follows the existing design and 
only tests a single configuration, as iotests are expected to do, or we 
have the question of how to deal with the fact that users will run the 
test suite in multiple configurations, but one run of this test would 
already cover them all.

I’m not sternly against cycling through the possible combinations right 
here in the test, but I want to lay out the problems with that approach.

> I can change this to use the current format (raw, qcow2, ...), 
> protocol (file, nbd, ...)
> and cache value (none, writeback), but I'm not sure how this can work 
> with the
> out files. The output from nbd is different from file. Maybe we need 
> different out
> files for file and nbd (qemu-img-checksum.file.out, 
> qemu-img-checksum.nbd.out)?

We already have that for the format (e.g. 178.out.{qcow2,raw}).  If you 
decide to do that, it shouldn’t be too difficult to implement 
(testrunner.py’s `TestRunner.find_reference()`).  Alternatively, it’s 
also possible to basically ignore the reference output and verify the 
expected output right in the test code.

Hanna


Re: [PATCH 2/3] iotests: Test qemu-img checksum
Posted by Nir Soffer 3 years, 2 months ago
On Mon, Nov 7, 2022 at 1:41 PM Hanna Reitz <hreitz@redhat.com> wrote:

> On 30.10.22 18:38, Nir Soffer wrote:
> > On Wed, Oct 26, 2022 at 4:31 PM Hanna Reitz <hreitz@redhat.com> wrote:
> >
> >     On 01.09.22 16:32, Nir Soffer wrote:
> >     > Add simple tests creating an image with all kinds of extents,
> >     different
> >     > formats, different backing chain, different protocol, and different
> >     > image options. Since all images have the same guest visible
> >     content they
> >     > must have the same checksum.
> >     >
> >     > To help debugging in case of failures, the output includes a
> >     json map of
> >     > every test image.
> >     >
> >     > Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> >     > ---
> >     >   tests/qemu-iotests/tests/qemu-img-checksum    | 149
> >     ++++++++++++++++++
> >     >   .../qemu-iotests/tests/qemu-img-checksum.out  |  74 +++++++++
> >     >   2 files changed, 223 insertions(+)
> >     >   create mode 100755 tests/qemu-iotests/tests/qemu-img-checksum
> >     >   create mode 100644 tests/qemu-iotests/tests/qemu-img-checksum.out
> >     >
> >     > diff --git a/tests/qemu-iotests/tests/qemu-img-checksum
> >     b/tests/qemu-iotests/tests/qemu-img-checksum
> >     > new file mode 100755
> >     > index 0000000000..3a85ba33f2
> >     > --- /dev/null
> >     > +++ b/tests/qemu-iotests/tests/qemu-img-checksum
> >     > @@ -0,0 +1,149 @@
> >     > +#!/usr/bin/env python3
> >     > +# group: rw auto quick
> >     > +#
> >     > +# Test cases for qemu-img checksum.
> >     > +#
> >     > +# Copyright (C) 2022 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/>.
> >     > +
> >     > +import re
> >     > +
> >     > +import iotests
> >     > +
> >     > +from iotests import (
> >     > +    filter_testfiles,
> >     > +    qemu_img,
> >     > +    qemu_img_log,
> >     > +    qemu_io,
> >     > +    qemu_nbd_popen,
> >     > +)
> >     > +
> >     > +
> >     > +def checksum_available():
> >     > +    out = qemu_img("--help").stdout
> >     > +    return re.search(r"\bchecksum .+ filename\b", out) is not None
> >     > +
> >     > +
> >     > +if not checksum_available():
> >     > +    iotests.notrun("checksum command not available")
> >     > +
> >     > +iotests.script_initialize(
> >     > +    supported_fmts=["raw", "qcow2"],
> >     > +    supported_cache_modes=["none", "writeback"],
> >
> >     It doesn’t work with writeback, though, because it uses -T none
> below.
> >
> >
> > Good point
> >
> >
> >     Which by the way is a heavy cost, because I usually run tests in
> >     tmpfs,
> >     where this won’t work.  Is there any way of not doing the -T none
> >     below?
> >
> >
> > Testing using tempfs is problematic since you cannot test -T none.In
> > oVirt
> > we alway use /var/tmp which usually uses something that supports
> > direct I/O.
> >
> > Do we have a way to specify cache mode in the tests, so we can use -T
> none
> > only when the option is set?
>
> `./check` has a `-c` option (e.g. `./check -c none`), which lands in
> `iotests.cachemode`.  That isn’t automatically passed to qemu-img calls,
> but you can do it manually (i.e. `qemu_img_log("checksum", "-T",
> iotests.cachemode, disk_top)` instead of `"-T", "none"`).
>

Ok, I will change to use the current cache setting.


> >
> >     > +    supported_protocols=["file", "nbd"],
> >     > +    required_fmts=["raw", "qcow2"],
> >     > +)
> >     > +
> >     > +print()
> >     > +print("=== Test images ===")
> >     > +print()
> >     > +
> >     > +disk_raw = iotests.file_path('raw')
> >     > +qemu_img("create", "-f", "raw", disk_raw, "10m")
> >     > +qemu_io("-f", "raw",
> >     > +        "-c", "write -P 0x1 0 2m",      # data
> >     > +        "-c", "write -P 0x0 2m 2m",     # data with zeroes
> >     > +        "-c", "write -z 4m 2m",         # zero allocated
> >     > +        "-c", "write -z -u 6m 2m",      # zero hole
> >     > +                                        # unallocated
> >     > +        disk_raw)
> >     > +print(filter_testfiles(disk_raw))
> >     > +qemu_img_log("map", "--output", "json", disk_raw)
> >     > +
> >     > +disk_qcow2 = iotests.file_path('qcow2')
> >     > +qemu_img("create", "-f", "qcow2", disk_qcow2, "10m")
> >     > +qemu_io("-f", "qcow2",
> >     > +        "-c", "write -P 0x1 0 2m",      # data
> >     > +        "-c", "write -P 0x0 2m 2m",     # data with zeroes
> >     > +        "-c", "write -z 4m 2m",         # zero allocated
> >     > +        "-c", "write -z -u 6m 2m",      # zero hole
> >     > +                                        # unallocated
> >     > +        disk_qcow2)
> >     > +print(filter_testfiles(disk_qcow2))
> >     > +qemu_img_log("map", "--output", "json", disk_qcow2)
> >
> >     This isn’t how iotests work, generally.  When run with -qcow2
> >     -file, it
> >     should only test qcow2 on file, not raw on file, not raw on nbd.
> >     Perhaps
> >     this way this test could even support other formats than qcow2 and
> >     raw.
> >
> >
> > For this type of tests, running the same code with raw, qcow2 (and
> > other formats)
> > and using file or nbd is the best way to test this feature - one test
> > verifies all the
> > use cases.
>
> Yes, I see, but that’s a general thing in the iotests.  The design is
> such that tests don’t cycle through their test matrix themselves, but
> that they always only test a single combination of format+protocol on
> each run, and the user is responsible for cycling through the desired
> test matrix.
>
> I’m not saying that was definitely the best design decision, but the
> problem now that if a test cycles through its test matrix by itself, it
> must also ensure that it is run only once when the user cycles through
> the same test matrix.  For example, a reasonable run of the iotests
> consists of `./check -raw`, `./check -qcow2`, and `./check -nbd`.  This
> test here would then run in all three configurations, but do the same
> thing every time (specifically, test all of those configurations every
> time).
>
> So there’s a conflict.  Either the test follows the existing design and
> only tests a single configuration, as iotests are expected to do, or we
> have the question of how to deal with the fact that users will run the
> test suite in multiple configurations, but one run of this test would
> already cover them all.
>
> I’m not sternly against cycling through the possible combinations right
> here in the test, but I want to lay out the problems with that approach.
>
> > I can change this to use the current format (raw, qcow2, ...),
> > protocol (file, nbd, ...)
> > and cache value (none, writeback), but I'm not sure how this can work
> > with the
> > out files. The output from nbd is different from file. Maybe we need
> > different out
> > files for file and nbd (qemu-img-checksum.file.out,
> > qemu-img-checksum.nbd.out)?
>
> We already have that for the format (e.g. 178.out.{qcow2,raw}).  If you
> decide to do that, it shouldn’t be too difficult to implement
> (testrunner.py’s `TestRunner.find_reference()`).  Alternatively, it’s
> also possible to basically ignore the reference output and verify the
> expected output right in the test code.
>

In this kind of test checking the output is the right thing since this is
the actual result of the command.

I'll change to use the current format and cache mode - this should work fine
with file protocol, and will simplify the test a lot.

I'm not sure if this will work for NBD but it is less important and can be
added
later. There is nothing related to NBD in the code.

Nir