* [Qemu-devel] [PATCH v3] qemu-img: Fix overwriting 'ret' before using
@ 2013-11-13 12:43 Kevin Wolf
2013-11-13 14:44 ` Eric Blake
2013-11-14 1:32 ` Fam Zheng
0 siblings, 2 replies; 3+ messages in thread
From: Kevin Wolf @ 2013-11-13 12:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, mrezanin, akong, famz, stefanha
From: Fam Zheng <famz@redhat.com>
This patch moves ret assignment after reporting original error.
We were lucky to pass qemu-iotests 048 (qemu-img compare case) but when
I tried to run with TEST_DIR=/tmp (tmpfs), it fails with a "wrong"
mismatch offset. This fixes two bugs.
In the first if branch, setting ret to 1 before using it makes dead code
in the next line: pnum is never added to mismatch offset even if ret was
0.
In the other if branch, currently the output error is always -4:
strerror(-4) -> Unknown error -4
Added regression test in case 048.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
There was another case of overwritten ret that I added in v3.
qemu-img.c | 6 +++---
tests/qemu-iotests/048 | 34 ++++++++++++++++++++++++++++++++++
tests/qemu-iotests/048.out | 27 ++++++++++++++++++++++++++-
3 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index bf3fb4f..b6b5644 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1020,10 +1020,10 @@ static int img_compare(int argc, char **argv)
}
ret = compare_sectors(buf1, buf2, nb_sectors, &pnum);
if (ret || pnum != nb_sectors) {
- ret = 1;
qprintf(quiet, "Content mismatch at offset %" PRId64 "!\n",
sectors_to_bytes(
ret ? sector_num : sector_num + pnum));
+ ret = 1;
goto out;
}
}
@@ -1045,9 +1045,9 @@ static int img_compare(int argc, char **argv)
}
if (ret) {
if (ret < 0) {
- ret = 4;
error_report("Error while reading offset %" PRId64 ": %s",
sectors_to_bytes(sector_num), strerror(-ret));
+ ret = 4;
}
goto out;
}
@@ -1092,10 +1092,10 @@ static int img_compare(int argc, char **argv)
filename_over, buf1, quiet);
if (ret) {
if (ret < 0) {
- ret = 4;
error_report("Error while reading offset %" PRId64
" of %s: %s", sectors_to_bytes(sector_num),
filename_over, strerror(-ret));
+ ret = 4;
}
goto out;
}
diff --git a/tests/qemu-iotests/048 b/tests/qemu-iotests/048
index 9b9d118..9def7fc 100755
--- a/tests/qemu-iotests/048
+++ b/tests/qemu-iotests/048
@@ -74,5 +74,39 @@ _compare
io_pattern write 0 $CLUSTER_SIZE 0 1 123
_compare
+# Test unaligned case of mismatch offsets in allocated clusters
+_make_test_img $size
+io_pattern write 0 512 0 1 100
+cp "$TEST_IMG" "$TEST_IMG2"
+io_pattern write 512 512 0 1 101
+_compare
+
+# Test cluster allocated in one, with IO error
+cat > "$TEST_DIR/blkdebug.conf"<<EOF
+[inject-error]
+event = "read_aio"
+errno = "5"
+once ="off"
+EOF
+_make_test_img $size
+cp "$TEST_IMG" "$TEST_IMG2"
+io_pattern write 512 512 0 1 102
+TEST_IMG="blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG" _compare 2>&1 |\
+ _filter_testdir | _filter_imgfmt
+
+# Test cluster allocated in one, with different sizes and IO error in the part
+# that exists only in one image
+cat > "$TEST_DIR/blkdebug.conf"<<EOF
+[inject-error]
+event = "read_aio"
+errno = "5"
+once ="off"
+EOF
+_make_test_img $size
+TEST_IMG="$TEST_IMG2" _make_test_img 0
+io_pattern write 512 512 0 1 102
+TEST_IMG="blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG" _compare 2>&1 |\
+ _filter_testdir | _filter_imgfmt
+
# Cleanup
status=0
diff --git a/tests/qemu-iotests/048.out b/tests/qemu-iotests/048.out
index 68f65d5..d141e05 100644
--- a/tests/qemu-iotests/048.out
+++ b/tests/qemu-iotests/048.out
@@ -1,5 +1,5 @@
QA output created by 048
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
=== IO: pattern 45
qemu-io> wrote 4096/4096 bytes at offset 524288
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -28,4 +28,29 @@ qemu-io> wrote 4096/4096 bytes at offset 0
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qemu-io> Content mismatch at offset 0!
1
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+=== IO: pattern 100
+qemu-io> wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> === IO: pattern 101
+qemu-io> wrote 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> Content mismatch at offset 512!
+1
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+=== IO: pattern 102
+qemu-io> wrote 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> qemu-img: Error while reading offset 0 of blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
+qemu-img: Error while reading offset 0: Input/output error
+4
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=0
+=== IO: pattern 102
+qemu-io> wrote 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> qemu-img: Error while reading offset 0 of blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
+qemu-img: Error while reading offset 0 of blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
+Warning: Image size mismatch!
+4
Cleanup
--
1.8.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH v3] qemu-img: Fix overwriting 'ret' before using
2013-11-13 12:43 [Qemu-devel] [PATCH v3] qemu-img: Fix overwriting 'ret' before using Kevin Wolf
@ 2013-11-13 14:44 ` Eric Blake
2013-11-14 1:32 ` Fam Zheng
1 sibling, 0 replies; 3+ messages in thread
From: Eric Blake @ 2013-11-13 14:44 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: mrezanin, akong, famz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]
On 11/13/2013 05:43 AM, Kevin Wolf wrote:
> From: Fam Zheng <famz@redhat.com>
>
> This patch moves ret assignment after reporting original error.
>
> We were lucky to pass qemu-iotests 048 (qemu-img compare case) but when
> I tried to run with TEST_DIR=/tmp (tmpfs), it fails with a "wrong"
> mismatch offset. This fixes two bugs.
>
> In the first if branch, setting ret to 1 before using it makes dead code
> in the next line: pnum is never added to mismatch offset even if ret was
> 0.
>
> In the other if branch, currently the output error is always -4:
> strerror(-4) -> Unknown error -4
>
> Added regression test in case 048.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Amos Kong <akong@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>
> There was another case of overwritten ret that I added in v3.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] qemu-img: Fix overwriting 'ret' before using
2013-11-13 12:43 [Qemu-devel] [PATCH v3] qemu-img: Fix overwriting 'ret' before using Kevin Wolf
2013-11-13 14:44 ` Eric Blake
@ 2013-11-14 1:32 ` Fam Zheng
1 sibling, 0 replies; 3+ messages in thread
From: Fam Zheng @ 2013-11-14 1:32 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: mrezanin, akong, stefanha
On 2013年11月13日 20:43, Kevin Wolf wrote:
> From: Fam Zheng <famz@redhat.com>
>
> This patch moves ret assignment after reporting original error.
>
> We were lucky to pass qemu-iotests 048 (qemu-img compare case) but when
> I tried to run with TEST_DIR=/tmp (tmpfs), it fails with a "wrong"
> mismatch offset. This fixes two bugs.
>
> In the first if branch, setting ret to 1 before using it makes dead code
> in the next line: pnum is never added to mismatch offset even if ret was
> 0.
>
> In the other if branch, currently the output error is always -4:
> strerror(-4) -> Unknown error -4
>
> Added regression test in case 048.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Amos Kong <akong@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>
> There was another case of overwritten ret that I added in v3.
>
> qemu-img.c | 6 +++---
> tests/qemu-iotests/048 | 34 ++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/048.out | 27 ++++++++++++++++++++++++++-
> 3 files changed, 63 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index bf3fb4f..b6b5644 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1020,10 +1020,10 @@ static int img_compare(int argc, char **argv)
> }
> ret = compare_sectors(buf1, buf2, nb_sectors, &pnum);
> if (ret || pnum != nb_sectors) {
> - ret = 1;
> qprintf(quiet, "Content mismatch at offset %" PRId64 "!\n",
> sectors_to_bytes(
> ret ? sector_num : sector_num + pnum));
> + ret = 1;
> goto out;
> }
> }
> @@ -1045,9 +1045,9 @@ static int img_compare(int argc, char **argv)
> }
> if (ret) {
> if (ret < 0) {
> - ret = 4;
> error_report("Error while reading offset %" PRId64 ": %s",
> sectors_to_bytes(sector_num), strerror(-ret));
> + ret = 4;
> }
> goto out;
> }
> @@ -1092,10 +1092,10 @@ static int img_compare(int argc, char **argv)
> filename_over, buf1, quiet);
> if (ret) {
> if (ret < 0) {
> - ret = 4;
> error_report("Error while reading offset %" PRId64
> " of %s: %s", sectors_to_bytes(sector_num),
> filename_over, strerror(-ret));
> + ret = 4;
> }
> goto out;
> }
> diff --git a/tests/qemu-iotests/048 b/tests/qemu-iotests/048
> index 9b9d118..9def7fc 100755
> --- a/tests/qemu-iotests/048
> +++ b/tests/qemu-iotests/048
> @@ -74,5 +74,39 @@ _compare
> io_pattern write 0 $CLUSTER_SIZE 0 1 123
> _compare
>
> +# Test unaligned case of mismatch offsets in allocated clusters
> +_make_test_img $size
> +io_pattern write 0 512 0 1 100
> +cp "$TEST_IMG" "$TEST_IMG2"
> +io_pattern write 512 512 0 1 101
> +_compare
> +
> +# Test cluster allocated in one, with IO error
> +cat > "$TEST_DIR/blkdebug.conf"<<EOF
> +[inject-error]
> +event = "read_aio"
> +errno = "5"
> +once ="off"
> +EOF
> +_make_test_img $size
> +cp "$TEST_IMG" "$TEST_IMG2"
> +io_pattern write 512 512 0 1 102
> +TEST_IMG="blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG" _compare 2>&1 |\
> + _filter_testdir | _filter_imgfmt
> +
> +# Test cluster allocated in one, with different sizes and IO error in the part
> +# that exists only in one image
> +cat > "$TEST_DIR/blkdebug.conf"<<EOF
> +[inject-error]
> +event = "read_aio"
> +errno = "5"
> +once ="off"
> +EOF
> +_make_test_img $size
> +TEST_IMG="$TEST_IMG2" _make_test_img 0
> +io_pattern write 512 512 0 1 102
I think the patterns are incremented for each step, so there's some clue
of the culprit, if this test fails. But it's up to you. Thanks for
adding this.
Reviewed-by: Fam Zheng <famz@redhat.com>
> +TEST_IMG="blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG" _compare 2>&1 |\
> + _filter_testdir | _filter_imgfmt
> +
> # Cleanup
> status=0
> diff --git a/tests/qemu-iotests/048.out b/tests/qemu-iotests/048.out
> index 68f65d5..d141e05 100644
> --- a/tests/qemu-iotests/048.out
> +++ b/tests/qemu-iotests/048.out
> @@ -1,5 +1,5 @@
> QA output created by 048
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> === IO: pattern 45
> qemu-io> wrote 4096/4096 bytes at offset 524288
> 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> @@ -28,4 +28,29 @@ qemu-io> wrote 4096/4096 bytes at offset 0
> 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> qemu-io> Content mismatch at offset 0!
> 1
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> +=== IO: pattern 100
> +qemu-io> wrote 512/512 bytes at offset 0
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +qemu-io> === IO: pattern 101
> +qemu-io> wrote 512/512 bytes at offset 512
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +qemu-io> Content mismatch at offset 512!
> +1
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> +=== IO: pattern 102
> +qemu-io> wrote 512/512 bytes at offset 512
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +qemu-io> qemu-img: Error while reading offset 0 of blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
> +qemu-img: Error while reading offset 0: Input/output error
> +4
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> +Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=0
> +=== IO: pattern 102
> +qemu-io> wrote 512/512 bytes at offset 512
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +qemu-io> qemu-img: Error while reading offset 0 of blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
> +qemu-img: Error while reading offset 0 of blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
> +Warning: Image size mismatch!
> +4
> Cleanup
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-11-14 1:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-13 12:43 [Qemu-devel] [PATCH v3] qemu-img: Fix overwriting 'ret' before using Kevin Wolf
2013-11-13 14:44 ` Eric Blake
2013-11-14 1:32 ` Fam Zheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).