From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57816) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VglnO-00029d-PY for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:32:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VglnH-0008Ky-04 for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:32:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40652) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VglnG-0008Ku-Oy for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:32:26 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rAE1WPfL008509 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 13 Nov 2013 20:32:26 -0500 Message-ID: <52842827.6030909@redhat.com> Date: Thu, 14 Nov 2013 09:32:23 +0800 From: Fam Zheng MIME-Version: 1.0 References: <1384346626-27479-1-git-send-email-kwolf@redhat.com> In-Reply-To: <1384346626-27479-1-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3] qemu-img: Fix overwriting 'ret' before using List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-devel@nongnu.org Cc: mrezanin@redhat.com, akong@redhat.com, stefanha@redhat.com On 2013=E5=B9=B411=E6=9C=8813=E6=97=A5 20:43, Kevin Wolf wrote: > From: Fam Zheng > > 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=3D/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 cod= e > in the next line: pnum is never added to mismatch offset even if ret wa= s > 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 > Signed-off-by: Amos Kong > Signed-off-by: Kevin Wolf > --- > > 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 =3D compare_sectors(buf1, buf2, nb_sectors, &pnum= ); > if (ret || pnum !=3D nb_sectors) { > - ret =3D 1; > qprintf(quiet, "Content mismatch at offset %" PRI= d64 "!\n", > sectors_to_bytes( > ret ? sector_num : sector_num + pnum)= ); > + ret =3D 1; > goto out; > } > } > @@ -1045,9 +1045,9 @@ static int img_compare(int argc, char **argv) > } > if (ret) { > if (ret < 0) { > - ret =3D 4; > error_report("Error while reading offset %" PRId6= 4 ": %s", > sectors_to_bytes(sector_num), strerr= or(-ret)); > + ret =3D 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 =3D 4; > error_report("Error while reading offset %" P= RId64 > " of %s: %s", sectors_to_bytes(s= ector_num), > filename_over, strerror(-ret)); > + ret =3D 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"< +[inject-error] > +event =3D "read_aio" > +errno =3D "5" > +once =3D"off" > +EOF > +_make_test_img $size > +cp "$TEST_IMG" "$TEST_IMG2" > +io_pattern write 512 512 0 1 102 > +TEST_IMG=3D"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"< +[inject-error] > +event =3D "read_aio" > +errno =3D "5" > +once =3D"off" > +EOF > +_make_test_img $size > +TEST_IMG=3D"$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=20 of the culprit, if this test fails. But it's up to you. Thanks for=20 adding this. Reviewed-by: Fam Zheng > +TEST_IMG=3D"blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG" _compare 2>&1 = |\ > + _filter_testdir | _filter_imgfmt > + > # Cleanup > status=3D0 > 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=3DIMGFMT size=3D1073741824 > +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D1073741824 > =3D=3D=3D 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=3DIMGFMT size=3D1073741824 > +=3D=3D=3D 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> =3D=3D=3D 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=3DIMGFMT size=3D1073741824 > +=3D=3D=3D 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/b= lkdebug.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=3DIMGFMT size=3D1073741824 > +Formatting 'TEST_DIR/t.IMGFMT.2', fmt=3DIMGFMT size=3D0 > +=3D=3D=3D 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/b= lkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error > +qemu-img: Error while reading offset 0 of blkdebug:TEST_DIR/blkdebug.c= onf:TEST_DIR/t.IMGFMT: Input/output error > +Warning: Image size mismatch! > +4 > Cleanup >