[Qemu-devel] [PATCH for-2.11] iotests: Fix 176 on 32-bit host

Eric Blake posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171117190422.23626-1-eblake@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
tests/qemu-iotests/176     | 3 ++-
tests/qemu-iotests/176.out | 8 ++++----
2 files changed, 6 insertions(+), 5 deletions(-)
[Qemu-devel] [PATCH for-2.11] iotests: Fix 176 on 32-bit host
Posted by Eric Blake 6 years, 5 months ago
The contents of a qcow2 bitmap are rounded up to a size that
matches the number of bits available for the granularity, but
that granularity differs for 32-bit hosts (our default 64k
cluster allows for 2M bitmap coverage per 'long') and 64-bit
hosts (4M bitmap per 'long').  If the image is a multiple of
2M but not 4M, then the number of bytes occupied by the array
of longs in memory differs between architecture, thus
resulting in different SHA256 hashes.

Furthermore (but untested by me), if our computation of the
SHA256 hash is at all endian-dependent because of how we store
data in memory, that's another variable we'd have to account
for (ideally, we specified the bitmap stored in qcow2 as
fixed-endian on disk, because the same qcow2 file must be
usable across any architecture; but that says nothing about
how we represent things in memory).  But we already have test
165 to validate that bitmaps are stored correctly on disk,
while this test is merely testing that the bitmap exists.

So for this test, the easiest solution is to filter out the
actual hash value.  Broken in commit 4096974e.

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/176     | 3 ++-
 tests/qemu-iotests/176.out | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
index 0f31a20294..b8dc17c592 100755
--- a/tests/qemu-iotests/176
+++ b/tests/qemu-iotests/176
@@ -52,7 +52,8 @@ _supported_os Linux
 function run_qemu()
 {
     $QEMU -nographic -qmp stdio -serial none "$@" 2>&1 \
-	| _filter_testdir | _filter_qmp | _filter_qemu
+	| _filter_testdir | _filter_qmp | _filter_qemu \
+        | sed 's/"sha256": ".\{64\}"/"sha256": HASH/'
 }

 for reason in snapshot bitmap; do
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
index e62085cd0a..f03a2e776c 100644
--- a/tests/qemu-iotests/176.out
+++ b/tests/qemu-iotests/176.out
@@ -205,7 +205,7 @@ Offset          Length          File
 QMP_VERSION
 {"return": {}}
 {"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}

@@ -255,7 +255,7 @@ Offset          Length          File
 QMP_VERSION
 {"return": {}}
 {"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}

@@ -305,7 +305,7 @@ Offset          Length          File
 QMP_VERSION
 {"return": {}}
 {"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}

@@ -352,7 +352,7 @@ Offset          Length          File
 QMP_VERSION
 {"return": {}}
 {"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
 *** done
-- 
2.13.6


Re: [Qemu-devel] [PATCH for-2.11] iotests: Fix 176 on 32-bit host
Posted by Eric Blake 6 years, 5 months ago
On 11/17/2017 01:04 PM, Eric Blake wrote:
> The contents of a qcow2 bitmap are rounded up to a size that
> matches the number of bits available for the granularity, but
> that granularity differs for 32-bit hosts (our default 64k
> cluster allows for 2M bitmap coverage per 'long') and 64-bit
> hosts (4M bitmap per 'long').  If the image is a multiple of
> 2M but not 4M, then the number of bytes occupied by the array
> of longs in memory differs between architecture, thus
> resulting in different SHA256 hashes.
> 
> Furthermore (but untested by me), if our computation of the
> SHA256 hash is at all endian-dependent because of how we store
> data in memory, that's another variable we'd have to account
> for (ideally, we specified the bitmap stored in qcow2 as
> fixed-endian on disk, because the same qcow2 file must be
> usable across any architecture; but that says nothing about
> how we represent things in memory).  But we already have test
> 165 to validate that bitmaps are stored correctly on disk,
> while this test is merely testing that the bitmap exists.
> 
> So for this test, the easiest solution is to filter out the
> actual hash value.  Broken in commit 4096974e.

Of course, if Kevin sends a v2 pull, it's probably better to just squash
this in to my original testsuite change (since a v2 would probably
invalidate that commit id).

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

Re: [Qemu-devel] [Qemu-block] [PATCH for-2.11] iotests: Fix 176 on 32-bit host
Posted by John Snow 6 years, 5 months ago

On 11/17/2017 02:10 PM, Eric Blake wrote:
> On 11/17/2017 01:04 PM, Eric Blake wrote:
>> The contents of a qcow2 bitmap are rounded up to a size that
>> matches the number of bits available for the granularity, but
>> that granularity differs for 32-bit hosts (our default 64k
>> cluster allows for 2M bitmap coverage per 'long') and 64-bit
>> hosts (4M bitmap per 'long').  If the image is a multiple of
>> 2M but not 4M, then the number of bytes occupied by the array
>> of longs in memory differs between architecture, thus
>> resulting in different SHA256 hashes.
>>
>> Furthermore (but untested by me), if our computation of the
>> SHA256 hash is at all endian-dependent because of how we store
>> data in memory, that's another variable we'd have to account
>> for (ideally, we specified the bitmap stored in qcow2 as
>> fixed-endian on disk, because the same qcow2 file must be
>> usable across any architecture; but that says nothing about
>> how we represent things in memory).  But we already have test
>> 165 to validate that bitmaps are stored correctly on disk,
>> while this test is merely testing that the bitmap exists.
>>
>> So for this test, the easiest solution is to filter out the
>> actual hash value.  Broken in commit 4096974e.
> 
> Of course, if Kevin sends a v2 pull, it's probably better to just squash
> this in to my original testsuite change (since a v2 would probably
> invalidate that commit id).
> 

Whichever way you go,

Reviewed-by: John Snow <jsnow@redhat.com>

I don't know how important it is to nail this down, but the purpose of
this command is primarily for sanity testing and not necessarily between
architectures, so this might just be a limitation to document.

Also, don't use this for anything except testing.

:(

Re: [Qemu-devel] [Qemu-block] [PATCH for-2.11] iotests: Fix 176 on 32-bit host
Posted by Eric Blake 6 years, 5 months ago
On 11/17/2017 04:59 PM, John Snow wrote:
>>> So for this test, the easiest solution is to filter out the
>>> actual hash value.  Broken in commit 4096974e.
>>
>> Of course, if Kevin sends a v2 pull, it's probably better to just squash
>> this in to my original testsuite change (since a v2 would probably
>> invalidate that commit id).
>>
> 
> Whichever way you go,

Commit 4096974e has already landed in master, so this commit message is
correct as-is and belongs in the next pull request (whether for -rc2 or
-rc3 depends on timing).

> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> I don't know how important it is to nail this down, but the purpose of
> this command is primarily for sanity testing and not necessarily between
> architectures, so this might just be a limitation to document.
> 
> Also, don't use this for anything except testing.

Indeed, the name of x-debug-block-dirty-bitmap-sha256 already implies
its limited portability; but it also gives us free reign to change it's
behavior if we find something we like better for the same ease of bitmap
testing/debugging.

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

Re: [Qemu-devel] [PATCH for-2.11] iotests: Fix 176 on 32-bit host
Posted by Max Reitz 6 years, 5 months ago
On 2017-11-17 20:04, Eric Blake wrote:
> The contents of a qcow2 bitmap are rounded up to a size that
> matches the number of bits available for the granularity, but
> that granularity differs for 32-bit hosts (our default 64k
> cluster allows for 2M bitmap coverage per 'long') and 64-bit
> hosts (4M bitmap per 'long').  If the image is a multiple of
> 2M but not 4M, then the number of bytes occupied by the array
> of longs in memory differs between architecture, thus
> resulting in different SHA256 hashes.
> 
> Furthermore (but untested by me), if our computation of the
> SHA256 hash is at all endian-dependent because of how we store
> data in memory, that's another variable we'd have to account
> for (ideally, we specified the bitmap stored in qcow2 as
> fixed-endian on disk, because the same qcow2 file must be
> usable across any architecture; but that says nothing about
> how we represent things in memory).  But we already have test
> 165 to validate that bitmaps are stored correctly on disk,
> while this test is merely testing that the bitmap exists.
> 
> So for this test, the easiest solution is to filter out the
> actual hash value.  Broken in commit 4096974e.
> 
> Reported-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/qemu-iotests/176     | 3 ++-
>  tests/qemu-iotests/176.out | 8 ++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH for-2.11] iotests: Fix 176 on 32-bit host
Posted by Max Reitz 6 years, 5 months ago
On 2017-11-17 20:04, Eric Blake wrote:
> The contents of a qcow2 bitmap are rounded up to a size that
> matches the number of bits available for the granularity, but
> that granularity differs for 32-bit hosts (our default 64k
> cluster allows for 2M bitmap coverage per 'long') and 64-bit
> hosts (4M bitmap per 'long').  If the image is a multiple of
> 2M but not 4M, then the number of bytes occupied by the array
> of longs in memory differs between architecture, thus
> resulting in different SHA256 hashes.
> 
> Furthermore (but untested by me), if our computation of the
> SHA256 hash is at all endian-dependent because of how we store
> data in memory, that's another variable we'd have to account
> for (ideally, we specified the bitmap stored in qcow2 as
> fixed-endian on disk, because the same qcow2 file must be
> usable across any architecture; but that says nothing about
> how we represent things in memory).  But we already have test
> 165 to validate that bitmaps are stored correctly on disk,
> while this test is merely testing that the bitmap exists.
> 
> So for this test, the easiest solution is to filter out the
> actual hash value.  Broken in commit 4096974e.
> 
> Reported-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/qemu-iotests/176     | 3 ++-
>  tests/qemu-iotests/176.out | 8 ++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)

Now that I've failed to keep my tree empty anyway:

Thanks, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

(No offense taken if **someone** *cough* *cough* were to take it from me)

Max