Re: [Qemu-block] [PATCH] iotests: Split 214 off of 122

2018-04-11 Thread Max Reitz
On 2018-04-06 18:41, Max Reitz wrote:
> Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122
> regarding how the qcow2 driver handles an incorrect compressed data
> length value.  This does not really fit into 122, as that file is
> supposed to contain qemu-img convert test cases, which this case is not.
> So this patch splits it off into its own file; maybe we will even get
> more qcow2-only compression tests in the future.
> 
> Also, that test case does not work with refcount_bits=1, so mark that
> option as unsupported.
> 
> Signed-off-by: Max Reitz 
> ---
> Kind of a v2 for "iotests: 122 needs at least two refcount bits now"
> (fulfills the same purpose, but also splits the case into its own file
>  so you can still run 122 with refcount_bits=1 [Eric]).
> 
> I was a bit lost what to do about the copyright text, since this test
> case was written by Berto.  I figured I'd drop the "owner" variable (it
> isn't used anyway), but I put "Red Hat" into the copyright line --
> currently every test has copyright information, so I decided it'd be
> difficult to leave that out, and I figured I simply cannot claim
> copyright for Igalia.  So, here we go.
> ---
>  tests/qemu-iotests/122 | 47 ---
>  tests/qemu-iotests/122.out | 33 
>  tests/qemu-iotests/214 | 96 
> ++
>  tests/qemu-iotests/214.out | 35 +
>  tests/qemu-iotests/group   |  1 +
>  5 files changed, 132 insertions(+), 80 deletions(-)
>  create mode 100755 tests/qemu-iotests/214
>  create mode 100644 tests/qemu-iotests/214.out

Changed the copyright information, added Berto's S-o-b (and Eric's R-b)
and applied to my block-next branch.

Max



Re: [Qemu-block] [PATCH] iotests: Split 214 off of 122

2018-04-11 Thread Max Reitz
On 2018-04-09 11:28, Alberto Garcia wrote:
> On Fri 06 Apr 2018 06:41:08 PM CEST, Max Reitz  wrote:
>> Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122
>> regarding how the qcow2 driver handles an incorrect compressed data
>> length value.  This does not really fit into 122, as that file is
>> supposed to contain qemu-img convert test cases, which this case is not.
>> So this patch splits it off into its own file; maybe we will even get
>> more qcow2-only compression tests in the future.
>>
>> Also, that test case does not work with refcount_bits=1, so mark that
>> option as unsupported.
>>
>> Signed-off-by: Max Reitz 
> 
> Looks good to me
> 
>> I was a bit lost what to do about the copyright text, since this test
>> case was written by Berto.  I figured I'd drop the "owner" variable
>> (it isn't used anyway), but I put "Red Hat" into the copyright line --
>> currently every test has copyright information, so I decided it'd be
>> difficult to leave that out, and I figured I simply cannot claim
>> copyright for Igalia.  So, here we go.
> 
> The new file contains only my test, right?

Yep.

>You can use
> 
> Copyright (C) 2018 Igalia, S.L.
> Author: Alberto Garcia 
> 
> and add
> 
> Signed-off-by: Alberto Garcia 

Thanks! :-)

Max



Re: [Qemu-block] [PATCH] iotests: Split 214 off of 122

2018-04-09 Thread Alberto Garcia
On Fri 06 Apr 2018 06:41:08 PM CEST, Max Reitz  wrote:
> Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122
> regarding how the qcow2 driver handles an incorrect compressed data
> length value.  This does not really fit into 122, as that file is
> supposed to contain qemu-img convert test cases, which this case is not.
> So this patch splits it off into its own file; maybe we will even get
> more qcow2-only compression tests in the future.
>
> Also, that test case does not work with refcount_bits=1, so mark that
> option as unsupported.
>
> Signed-off-by: Max Reitz 

Looks good to me

> I was a bit lost what to do about the copyright text, since this test
> case was written by Berto.  I figured I'd drop the "owner" variable
> (it isn't used anyway), but I put "Red Hat" into the copyright line --
> currently every test has copyright information, so I decided it'd be
> difficult to leave that out, and I figured I simply cannot claim
> copyright for Igalia.  So, here we go.

The new file contains only my test, right? You can use

Copyright (C) 2018 Igalia, S.L.
Author: Alberto Garcia 

and add

Signed-off-by: Alberto Garcia 

Berto



Re: [Qemu-block] [PATCH] iotests: Split 214 off of 122

2018-04-06 Thread Eric Blake
On 04/06/2018 11:41 AM, Max Reitz wrote:
> Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122
> regarding how the qcow2 driver handles an incorrect compressed data
> length value.  This does not really fit into 122, as that file is
> supposed to contain qemu-img convert test cases, which this case is not.
> So this patch splits it off into its own file; maybe we will even get
> more qcow2-only compression tests in the future.
> 
> Also, that test case does not work with refcount_bits=1, so mark that
> option as unsupported.
> 
> Signed-off-by: Max Reitz 
> ---
> Kind of a v2 for "iotests: 122 needs at least two refcount bits now"
> (fulfills the same purpose, but also splits the case into its own file
>  so you can still run 122 with refcount_bits=1 [Eric]).
> 
> I was a bit lost what to do about the copyright text, since this test
> case was written by Berto.  I figured I'd drop the "owner" variable (it
> isn't used anyway), but I put "Red Hat" into the copyright line --
> currently every test has copyright information, so I decided it'd be
> difficult to leave that out, and I figured I simply cannot claim
> copyright for Igalia.  So, here we go.

Yeah, you'll want Berto's Signed-off-by if you tweak that line, or at
least his Reviewed-by otherwise :)

But you can have mine,
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH] iotests: Split 214 off of 122

2018-04-06 Thread Max Reitz
Sorry, Berto, forgot to CC you again...

On 2018-04-06 18:41, Max Reitz wrote:
> Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122
> regarding how the qcow2 driver handles an incorrect compressed data
> length value.  This does not really fit into 122, as that file is
> supposed to contain qemu-img convert test cases, which this case is not.
> So this patch splits it off into its own file; maybe we will even get
> more qcow2-only compression tests in the future.
> 
> Also, that test case does not work with refcount_bits=1, so mark that
> option as unsupported.
> 
> Signed-off-by: Max Reitz 
> ---
> Kind of a v2 for "iotests: 122 needs at least two refcount bits now"
> (fulfills the same purpose, but also splits the case into its own file
>  so you can still run 122 with refcount_bits=1 [Eric]).
> 
> I was a bit lost what to do about the copyright text, since this test
> case was written by Berto.  I figured I'd drop the "owner" variable (it
> isn't used anyway), but I put "Red Hat" into the copyright line --
> currently every test has copyright information, so I decided it'd be
> difficult to leave that out, and I figured I simply cannot claim
> copyright for Igalia.  So, here we go.
> ---
>  tests/qemu-iotests/122 | 47 ---
>  tests/qemu-iotests/122.out | 33 
>  tests/qemu-iotests/214 | 96 
> ++
>  tests/qemu-iotests/214.out | 35 +
>  tests/qemu-iotests/group   |  1 +
>  5 files changed, 132 insertions(+), 80 deletions(-)
>  create mode 100755 tests/qemu-iotests/214
>  create mode 100644 tests/qemu-iotests/214.out
> 
> diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122
> index 6cf4fcb866..45b359c2ba 100755
> --- a/tests/qemu-iotests/122
> +++ b/tests/qemu-iotests/122
> @@ -129,53 +129,6 @@ $QEMU_IO -c "read -P 0x44 1023k1k" "$TEST_IMG" 2>&1 
> | _filter_qemu_io | _fil
>  $QEMU_IO -c "read -P 01024k 1022k" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
> _filter_testdir
>  
>  
> -echo
> -echo "=== Corrupted size field in compressed cluster descriptor ==="
> -echo
> -# Create an empty image and fill half of it with compressed data.
> -# The L2 entries of the two compressed clusters are located at
> -# 0x80 and 0x88, their original values are 0x400800a0
> -# and 0x400800a00802 (5 sectors for compressed data each).
> -_make_test_img 8M -o cluster_size=2M
> -$QEMU_IO -c "write -c -P 0x11 0 2M" -c "write -c -P 0x11 2M 2M" "$TEST_IMG" \
> - 2>&1 | _filter_qemu_io | _filter_testdir
> -
> -# Reduce size of compressed data to 4 sectors: this corrupts the image.
> -poke_file "$TEST_IMG" $((0x80)) "\x40\x06"
> -$QEMU_IO -c "read  -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
> _filter_testdir
> -
> -# 'qemu-img check' however doesn't see anything wrong because it
> -# doesn't try to decompress the data and the refcounts are consistent.
> -# TODO: update qemu-img so this can be detected.
> -_check_test_img
> -
> -# Increase size of compressed data to the maximum (8192 sectors).
> -# This makes QEMU read more data (8192 sectors instead of 5, host
> -# addresses [0xa0, 0xdf]), but the decompression algorithm
> -# stops once we have enough to restore the uncompressed cluster, so
> -# the rest of the data is ignored.
> -poke_file "$TEST_IMG" $((0x80)) "\x7f\xfe"
> -# Do it also for the second compressed cluster (L2 entry at 0x88).
> -# In this case the compressed data would span 3 host clusters
> -# (host addresses: [0xa00802, 0xe00801])
> -poke_file "$TEST_IMG" $((0x88)) "\x7f\xfe"
> -
> -# Here the image is too small so we're asking QEMU to read beyond the
> -# end of the image.
> -$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
> _filter_testdir
> -# But if we grow the image we won't be reading beyond its end anymore.
> -$QEMU_IO -c "write -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
> _filter_testdir
> -$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
> _filter_testdir
> -
> -# The refcount data is however wrong because due to the increased size
> -# of the compressed data it now reaches the following host clusters.
> -# This can be repaired by qemu-img check by increasing the refcount of
> -# those clusters.
> -# TODO: update qemu-img to correct the compressed cluster size instead.
> -_check_test_img -r all
> -$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
> _filter_testdir
> -$QEMU_IO -c "read  -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
> _filter_testdir
> -
>  echo
>  echo "=== Full allocation with -S 0 ==="
>  echo
> diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
> index a6b7fe007e..47d8656db8 100644
> --- a/tests/qemu-iotests/122.out
> +++ b/tests/qemu-iotests/122.out
> @@ -99,39 +99,6 @@ read 1024/1024 bytes at offset 1047552
>  read 1046528/1046528 bytes at offset 1048576
>  1022 KiB, X ops; XX:XX:XX.X (XXX YYY/sec 

[Qemu-block] [PATCH] iotests: Split 214 off of 122

2018-04-06 Thread Max Reitz
Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122
regarding how the qcow2 driver handles an incorrect compressed data
length value.  This does not really fit into 122, as that file is
supposed to contain qemu-img convert test cases, which this case is not.
So this patch splits it off into its own file; maybe we will even get
more qcow2-only compression tests in the future.

Also, that test case does not work with refcount_bits=1, so mark that
option as unsupported.

Signed-off-by: Max Reitz 
---
Kind of a v2 for "iotests: 122 needs at least two refcount bits now"
(fulfills the same purpose, but also splits the case into its own file
 so you can still run 122 with refcount_bits=1 [Eric]).

I was a bit lost what to do about the copyright text, since this test
case was written by Berto.  I figured I'd drop the "owner" variable (it
isn't used anyway), but I put "Red Hat" into the copyright line --
currently every test has copyright information, so I decided it'd be
difficult to leave that out, and I figured I simply cannot claim
copyright for Igalia.  So, here we go.
---
 tests/qemu-iotests/122 | 47 ---
 tests/qemu-iotests/122.out | 33 
 tests/qemu-iotests/214 | 96 ++
 tests/qemu-iotests/214.out | 35 +
 tests/qemu-iotests/group   |  1 +
 5 files changed, 132 insertions(+), 80 deletions(-)
 create mode 100755 tests/qemu-iotests/214
 create mode 100644 tests/qemu-iotests/214.out

diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122
index 6cf4fcb866..45b359c2ba 100755
--- a/tests/qemu-iotests/122
+++ b/tests/qemu-iotests/122
@@ -129,53 +129,6 @@ $QEMU_IO -c "read -P 0x44 1023k1k" "$TEST_IMG" 2>&1 | 
_filter_qemu_io | _fil
 $QEMU_IO -c "read -P 01024k 1022k" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
 
 
-echo
-echo "=== Corrupted size field in compressed cluster descriptor ==="
-echo
-# Create an empty image and fill half of it with compressed data.
-# The L2 entries of the two compressed clusters are located at
-# 0x80 and 0x88, their original values are 0x400800a0
-# and 0x400800a00802 (5 sectors for compressed data each).
-_make_test_img 8M -o cluster_size=2M
-$QEMU_IO -c "write -c -P 0x11 0 2M" -c "write -c -P 0x11 2M 2M" "$TEST_IMG" \
- 2>&1 | _filter_qemu_io | _filter_testdir
-
-# Reduce size of compressed data to 4 sectors: this corrupts the image.
-poke_file "$TEST_IMG" $((0x80)) "\x40\x06"
-$QEMU_IO -c "read  -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
-
-# 'qemu-img check' however doesn't see anything wrong because it
-# doesn't try to decompress the data and the refcounts are consistent.
-# TODO: update qemu-img so this can be detected.
-_check_test_img
-
-# Increase size of compressed data to the maximum (8192 sectors).
-# This makes QEMU read more data (8192 sectors instead of 5, host
-# addresses [0xa0, 0xdf]), but the decompression algorithm
-# stops once we have enough to restore the uncompressed cluster, so
-# the rest of the data is ignored.
-poke_file "$TEST_IMG" $((0x80)) "\x7f\xfe"
-# Do it also for the second compressed cluster (L2 entry at 0x88).
-# In this case the compressed data would span 3 host clusters
-# (host addresses: [0xa00802, 0xe00801])
-poke_file "$TEST_IMG" $((0x88)) "\x7f\xfe"
-
-# Here the image is too small so we're asking QEMU to read beyond the
-# end of the image.
-$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
-# But if we grow the image we won't be reading beyond its end anymore.
-$QEMU_IO -c "write -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
-$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
-
-# The refcount data is however wrong because due to the increased size
-# of the compressed data it now reaches the following host clusters.
-# This can be repaired by qemu-img check by increasing the refcount of
-# those clusters.
-# TODO: update qemu-img to correct the compressed cluster size instead.
-_check_test_img -r all
-$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
-$QEMU_IO -c "read  -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
-
 echo
 echo "=== Full allocation with -S 0 ==="
 echo
diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
index a6b7fe007e..47d8656db8 100644
--- a/tests/qemu-iotests/122.out
+++ b/tests/qemu-iotests/122.out
@@ -99,39 +99,6 @@ read 1024/1024 bytes at offset 1047552
 read 1046528/1046528 bytes at offset 1048576
 1022 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
-=== Corrupted size field in compressed cluster descriptor ===
-
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8388608
-wrote 2097152/2097152 bytes at offset 0
-2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 2097152/2097152 bytes at offset 2097152
-2 MiB,