Message ID | 20170508171302.17805-1-eblake@redhat.com |
---|---|
State | New |
Headers | show |
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
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?
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
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
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 >
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
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 ===
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(-)