[Qemu-devel] [PATCH] block: Tweak error message related to qemu-img convert

[Qemu-devel] [PATCH] block: Tweak error message related to qemu-img convert

[Eric Blake]

When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
a contrived failure where allocating a cluster used to replace a
zero cluster reads unaligned data.  Since it is a zero cluster
rather than a data cluster being converted, changing the error
message to match our earlier change in 'qcow2: Make distinction
bewteen zero cluster types obvious' is worthwhile.

Suggested-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---

There's one more instance of "Data cluster offset" in qcow2-cluster.c,
but that one in handle_copied() is contained inside a
cluster_type == QCOW2_CLUSTER_NORMAL conditional.

 block/qcow2-cluster.c      | 3 ++-
 tests/qemu-iotests/060.out | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 347d94b..d779ea1 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1797,7 +1797,8 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
             }

             if (offset_into_cluster(s, offset)) {
-                qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset "
+                qcow2_signal_corruption(bs, true, -1, -1,
+                                        "Cluster allocation offset "
                                         "%#" PRIx64 " unaligned (L2 offset: %#"
                                         PRIx64 ", L2 index: %#x)", offset,
                                         l2_offset, j);
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 9e8f5b9..3bc1461 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -143,7 +143,7 @@ read failed: Input/output error
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-qcow2: Marking image as corrupt: Data cluster offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
+qcow2: Marking image as corrupt: Cluster allocation offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
 qemu-img: Error while amending options: Input/output error

 === Testing unaligned reftable entry ===
-- 
2.9.3

Re: [Qemu-devel] [PATCH] block: Tweak error message related to qemu-img convert

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]

On 08.05.2017 19:13, Eric Blake wrote:
> When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
> a contrived failure where allocating a cluster used to replace a
> zero cluster reads unaligned data.  Since it is a zero cluster
> rather than a data cluster being converted, changing the error
> message to match our earlier change in 'qcow2: Make distinction
> bewteen zero cluster types obvious' is worthwhile.
> 
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> There's one more instance of "Data cluster offset" in qcow2-cluster.c,
> but that one in handle_copied() is contained inside a
> cluster_type == QCOW2_CLUSTER_NORMAL conditional.
> 
>  block/qcow2-cluster.c      | 3 ++-
>  tests/qemu-iotests/060.out | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Looks good, except for one thing: This is for qemu-img amend, not
qemu-img convert.

I can handle that, if you'd like, though. :-)

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [Qemu-devel] [PATCH] block: Tweak error message related to qemu-img convert

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

On 05/08/2017 01:41 PM, Max Reitz wrote:
> On 08.05.2017 19:13, Eric Blake wrote:
>> When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
>> a contrived failure where allocating a cluster used to replace a
>> zero cluster reads unaligned data.  Since it is a zero cluster
>> rather than a data cluster being converted, changing the error
>> message to match our earlier change in 'qcow2: Make distinction
>> bewteen zero cluster types obvious' is worthwhile.
>>
>> Suggested-by: Max Reitz <mreitz@redhat.com>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> There's one more instance of "Data cluster offset" in qcow2-cluster.c,
>> but that one in handle_copied() is contained inside a
>> cluster_type == QCOW2_CLUSTER_NORMAL conditional.
>>
>>  block/qcow2-cluster.c      | 3 ++-
>>  tests/qemu-iotests/060.out | 2 +-
>>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Looks good, except for one thing: This is for qemu-img amend, not
> qemu-img convert.
> 
> I can handle that, if you'd like, though. :-)

So is fixing my commit message considered a conversion, or an amendment?

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] block: Tweak error message related to qemu-img convert

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

On 08.05.2017 21:06, Eric Blake wrote:
> On 05/08/2017 01:41 PM, Max Reitz wrote:
>> On 08.05.2017 19:13, Eric Blake wrote:
>>> When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
>>> a contrived failure where allocating a cluster used to replace a
>>> zero cluster reads unaligned data.  Since it is a zero cluster
>>> rather than a data cluster being converted, changing the error
>>> message to match our earlier change in 'qcow2: Make distinction
>>> bewteen zero cluster types obvious' is worthwhile.
>>>
>>> Suggested-by: Max Reitz <mreitz@redhat.com>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>
>>> There's one more instance of "Data cluster offset" in qcow2-cluster.c,
>>> but that one in handle_copied() is contained inside a
>>> cluster_type == QCOW2_CLUSTER_NORMAL conditional.
>>>
>>>  block/qcow2-cluster.c      | 3 ++-
>>>  tests/qemu-iotests/060.out | 2 +-
>>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> Looks good, except for one thing: This is for qemu-img amend, not
>> qemu-img convert.
>>
>> I can handle that, if you'd like, though. :-)
> 
> So is fixing my commit message considered a conversion, or an amendment?

The git commit parameter is called "--amend", so take your guess. ;-)

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [Qemu-devel] [PATCH] block: Tweak error message related to qemu-img convert

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

On 2017-05-08 19:13, Eric Blake wrote:
> When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
> a contrived failure where allocating a cluster used to replace a
> zero cluster reads unaligned data.  Since it is a zero cluster
> rather than a data cluster being converted, changing the error
> message to match our earlier change in 'qcow2: Make distinction
> bewteen zero cluster types obvious' is worthwhile.
> 
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> There's one more instance of "Data cluster offset" in qcow2-cluster.c,
> but that one in handle_copied() is contained inside a
> cluster_type == QCOW2_CLUSTER_NORMAL conditional.
> 
>  block/qcow2-cluster.c      | 3 ++-
>  tests/qemu-iotests/060.out | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Assuming no objection means consent:

Thanks, fixed the commit title and applied to my block branch:

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

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH] block: Tweak error message related to qemu-img convert

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

On 05/19/2017 08:30 AM, Max Reitz wrote:
> On 2017-05-08 19:13, Eric Blake wrote:
>> When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
>> a contrived failure where allocating a cluster used to replace a
>> zero cluster reads unaligned data.  Since it is a zero cluster
>> rather than a data cluster being converted, changing the error
>> message to match our earlier change in 'qcow2: Make distinction
>> bewteen zero cluster types obvious' is worthwhile.

I really had problems with that commit message. If you want to re-stage,
you could s/bewteen/between/

>>
>> Suggested-by: Max Reitz <mreitz@redhat.com>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> There's one more instance of "Data cluster offset" in qcow2-cluster.c,
>> but that one in handle_copied() is contained inside a
>> cluster_type == QCOW2_CLUSTER_NORMAL conditional.
>>
>>  block/qcow2-cluster.c      | 3 ++-
>>  tests/qemu-iotests/060.out | 2 +-
>>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Assuming no objection means consent:

Okay by me.

> 
> Thanks, fixed the commit title and applied to my block branch:
> 
> https://github.com/XanClic/qemu/commits/block
> 
> Max
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] block: Tweak error message related to qemu-img convert

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On 2017-05-20 00:07, Eric Blake wrote:
> On 05/19/2017 08:30 AM, Max Reitz wrote:
>> On 2017-05-08 19:13, Eric Blake wrote:
>>> When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
>>> a contrived failure where allocating a cluster used to replace a
>>> zero cluster reads unaligned data.  Since it is a zero cluster
>>> rather than a data cluster being converted, changing the error
>>> message to match our earlier change in 'qcow2: Make distinction
>>> bewteen zero cluster types obvious' is worthwhile.
> 
> I really had problems with that commit message. If you want to re-stage,
> you could s/bewteen/between/

Sure, can do (and done :-)).

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]