| Message ID | 20210705091549.178335-7-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: support zstd | expand |
05.07.2021 12:15, Vladimir Sementsov-Ogievskiy wrote: > Instead of qemu_img_log("info", ..) use generic helper img_info_log(). > > img_info_log() has smarter logic. For example it use filter_img_info() > to filter output, which in turns filter a compression type. So it will > help us in future when we implement a possibility to use zstd > compression by default (with help of some runtime config file or maybe > build option). For now to test you should recompile qemu with a small > patch: > > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) > } > } > > + if (!qcow2_opts->has_compression_type && version >= 3) { > + qcow2_opts->has_compression_type = true; > + qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD; > + } > + > if (qcow2_opts->has_compression_type && > qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) { > > Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com> Wow, that was bad idea to insert patch into commit message even with indent: it breaks rpm build for me. So, reword like this: build option). For now to test you should recompile qemu with a small addition into block/qcow2.c before "if (qcow2_opts->has_compression_type": if (!qcow2_opts->has_compression_type && version >= 3) { qcow2_opts->has_compression_type = true; qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD; }
On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote: > Instead of qemu_img_log("info", ..) use generic helper img_info_log(). > > img_info_log() has smarter logic. For example it use filter_img_info() > to filter output, which in turns filter a compression type. So it will > help us in future when we implement a possibility to use zstd > compression by default (with help of some runtime config file or maybe > build option). For now to test you should recompile qemu with a small > patch: > > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) > } > } > > + if (!qcow2_opts->has_compression_type && version >= 3) { > + qcow2_opts->has_compression_type = true; > + qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD; > + } > + > if (qcow2_opts->has_compression_type && > qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) { > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > tests/qemu-iotests/302 | 3 ++- > tests/qemu-iotests/302.out | 7 +++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302 > index 5695af4914..2180dbc896 100755 > --- a/tests/qemu-iotests/302 > +++ b/tests/qemu-iotests/302 > @@ -34,6 +34,7 @@ from iotests import ( > qemu_img_measure, > qemu_io, > qemu_nbd_popen, > + img_info_log, > ) > > iotests.script_initialize(supported_fmts=["qcow2"]) > @@ -99,7 +100,7 @@ with tarfile.open(tar_file, "w") as tar: > nbd_uri) > > iotests.log("=== Converted image info ===") > - qemu_img_log("info", nbd_uri) > + img_info_log(nbd_uri) There’s another `qemu_img_log("info", nbd_uri)` call above this place. We can’t use `img_info_log()` there, because in that case, the image is not in qcow2 format (which is the test’s image format), but `img_info_log()` enforces “-f {imgfmt}”. It would have been nice to have a comment on that somewhere, though. But, well. Reviewed-by: Max Reitz <mreitz@redhat.com> (And speaking in principle, I don’t think I like the broad img_info_log() very much anyway, because I feel like tests should rather only have the actually relevant bits in their reference outputs…) > > iotests.log("=== Converted image check ===") > qemu_img_log("check", nbd_uri) > diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out > index e2f6077e83..3e7c281b91 100644 > --- a/tests/qemu-iotests/302.out > +++ b/tests/qemu-iotests/302.out > @@ -6,14 +6,13 @@ virtual size: 448 KiB (458752 bytes) > disk size: unavailable > > === Converted image info === > -image: nbd+unix:///exp?socket=SOCK_DIR/PID-nbd-sock > -file format: qcow2 > +image: TEST_IMG > +file format: IMGFMT > virtual size: 1 GiB (1073741824 bytes) > -disk size: unavailable > cluster_size: 65536 > Format specific information: > compat: 1.1 > - compression type: zlib > + compression type: COMPRESSION_TYPE > lazy refcounts: false > refcount bits: 16 > corrupt: false
16.07.2021 14:39, Max Reitz wrote: > On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote: >> Instead of qemu_img_log("info", ..) use generic helper img_info_log(). >> >> img_info_log() has smarter logic. For example it use filter_img_info() >> to filter output, which in turns filter a compression type. So it will >> help us in future when we implement a possibility to use zstd >> compression by default (with help of some runtime config file or maybe >> build option). For now to test you should recompile qemu with a small >> patch: >> >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) >> } >> } >> >> + if (!qcow2_opts->has_compression_type && version >= 3) { >> + qcow2_opts->has_compression_type = true; >> + qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD; >> + } >> + >> if (qcow2_opts->has_compression_type && >> qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) { >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> tests/qemu-iotests/302 | 3 ++- >> tests/qemu-iotests/302.out | 7 +++---- >> 2 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302 >> index 5695af4914..2180dbc896 100755 >> --- a/tests/qemu-iotests/302 >> +++ b/tests/qemu-iotests/302 >> @@ -34,6 +34,7 @@ from iotests import ( >> qemu_img_measure, >> qemu_io, >> qemu_nbd_popen, >> + img_info_log, >> ) >> iotests.script_initialize(supported_fmts=["qcow2"]) >> @@ -99,7 +100,7 @@ with tarfile.open(tar_file, "w") as tar: >> nbd_uri) >> iotests.log("=== Converted image info ===") >> - qemu_img_log("info", nbd_uri) >> + img_info_log(nbd_uri) > > There’s another `qemu_img_log("info", nbd_uri)` call above this place. We can’t use `img_info_log()` there, because in that case, the image is not in qcow2 format (which is the test’s image format), but `img_info_log()` enforces “-f {imgfmt}”. It would have been nice to have a comment on that somewhere, though. I'll add a comment. Actually, I only fixed places which breaks when I set zstd by default. That's why some things may be not covered. > > But, well. > > Reviewed-by: Max Reitz <mreitz@redhat.com> > > (And speaking in principle, I don’t think I like the broad img_info_log() very much anyway, because I feel like tests should rather only have the actually relevant bits in their reference outputs…) I agree, extra useless information in test outputs is always a pain.. We should pay more attention to it when add new tests. > >> iotests.log("=== Converted image check ===") >> qemu_img_log("check", nbd_uri) >> diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out >> index e2f6077e83..3e7c281b91 100644 >> --- a/tests/qemu-iotests/302.out >> +++ b/tests/qemu-iotests/302.out >> @@ -6,14 +6,13 @@ virtual size: 448 KiB (458752 bytes) >> disk size: unavailable >> === Converted image info === >> -image: nbd+unix:///exp?socket=SOCK_DIR/PID-nbd-sock >> -file format: qcow2 >> +image: TEST_IMG >> +file format: IMGFMT >> virtual size: 1 GiB (1073741824 bytes) >> -disk size: unavailable >> cluster_size: 65536 >> Format specific information: >> compat: 1.1 >> - compression type: zlib >> + compression type: COMPRESSION_TYPE >> lazy refcounts: false >> refcount bits: 16 >> corrupt: false >
diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302 index 5695af4914..2180dbc896 100755 --- a/tests/qemu-iotests/302 +++ b/tests/qemu-iotests/302 @@ -34,6 +34,7 @@ from iotests import ( qemu_img_measure, qemu_io, qemu_nbd_popen, + img_info_log, ) iotests.script_initialize(supported_fmts=["qcow2"]) @@ -99,7 +100,7 @@ with tarfile.open(tar_file, "w") as tar: nbd_uri) iotests.log("=== Converted image info ===") - qemu_img_log("info", nbd_uri) + img_info_log(nbd_uri) iotests.log("=== Converted image check ===") qemu_img_log("check", nbd_uri) diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out index e2f6077e83..3e7c281b91 100644 --- a/tests/qemu-iotests/302.out +++ b/tests/qemu-iotests/302.out @@ -6,14 +6,13 @@ virtual size: 448 KiB (458752 bytes) disk size: unavailable === Converted image info === -image: nbd+unix:///exp?socket=SOCK_DIR/PID-nbd-sock -file format: qcow2 +image: TEST_IMG +file format: IMGFMT virtual size: 1 GiB (1073741824 bytes) -disk size: unavailable cluster_size: 65536 Format specific information: compat: 1.1 - compression type: zlib + compression type: COMPRESSION_TYPE lazy refcounts: false refcount bits: 16 corrupt: false
Instead of qemu_img_log("info", ..) use generic helper img_info_log(). img_info_log() has smarter logic. For example it use filter_img_info() to filter output, which in turns filter a compression type. So it will help us in future when we implement a possibility to use zstd compression by default (with help of some runtime config file or maybe build option). For now to test you should recompile qemu with a small patch: --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) } } + if (!qcow2_opts->has_compression_type && version >= 3) { + qcow2_opts->has_compression_type = true; + qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD; + } + if (qcow2_opts->has_compression_type && qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) { Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/302 | 3 ++- tests/qemu-iotests/302.out | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-)