Re: [PATCH 06/14] iotest 302: use img_info_log() helper

2021-07-16 Thread Vladimir Sementsov-Ogievskiy 

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 
---
  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 

(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





--
Best regards,
Vladimir

Re: [PATCH 06/14] iotest 302: use img_info_log() helper

2021-07-16 Thread Max Reitz 

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 
---
  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 

(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

Re: [PATCH 06/14] iotest 302: use img_info_log() helper

2021-07-05 Thread Vladimir Sementsov-Ogievskiy 

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


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;
}


--
Best regards,
Vladimir

[PATCH 06/14] iotest 302: use img_info_log() helper

2021-07-05 Thread Vladimir Sementsov-Ogievskiy 
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 
---
 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)
 
 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
-- 
2.29.2