From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57888) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSrdZ-0003la-KO for qemu-devel@nongnu.org; Sat, 13 Sep 2014 14:01:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSrdU-0006PK-Fj for qemu-devel@nongnu.org; Sat, 13 Sep 2014 14:01:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:28159) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSrdU-0006PC-8P for qemu-devel@nongnu.org; Sat, 13 Sep 2014 14:01:24 -0400 Message-ID: <54148668.1020102@redhat.com> Date: Sat, 13 Sep 2014 20:01:12 +0200 From: Max Reitz MIME-Version: 1.0 References: <1410515461-12740-1-git-send-email-famz@redhat.com> In-Reply-To: <1410515461-12740-1-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] vmdk: Fix integer overflow in offset calculation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Paolo Bonzini , Jeff Cody , Peter Lieven , Stefan Hajnoczi On 12.09.2014 11:51, Fam Zheng wrote: > This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster > allocation). > > $ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k' > write failed: Invalid argument > > Reported-by: Mark Cave-Ayland > Signed-off-by: Fam Zheng > --- > block/vmdk.c | 2 +- > tests/qemu-iotests/059 | 6 ++++++ > tests/qemu-iotests/059.out | 7 +++++++ > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/block/vmdk.c b/block/vmdk.c > index a1cb911..3fd7738 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs, > uint32_t min_count, *l2_table; > bool zeroed = false; > int64_t ret; > - int32_t cluster_sector; > + int64_t cluster_sector; > > if (m_data) { > m_data->valid = 0; The fix is okay. > diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 > index 3c053c2..6ed2a25 100755 > --- a/tests/qemu-iotests/059 > +++ b/tests/qemu-iotests/059 > @@ -126,6 +126,12 @@ _img_info > $QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io > $QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io > > +echo > +echo "=== Testing reading and writing at big offset ===" > +_make_test_img 4T > +$QEMU_IO -c "write -P 0xa 1T 1024" "$TEST_IMG" | _filter_qemu_io > +$QEMU_IO -c "read -P 0xa 1T 1024" "$TEST_IMG" | _filter_qemu_io > + > # success, all done > echo "*** done" > rm -f $seq.full > diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out > index 0dadba6..51c72a6 100644 > --- a/tests/qemu-iotests/059.out > +++ b/tests/qemu-iotests/059.out > @@ -2332,4 +2332,11 @@ e1000003e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > e1000003f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > read 1024/1024 bytes at offset 966367641600 > 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > + > +=== Testing reading and writing at big offset === > +Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=4398046511104 > +wrote 1024/1024 bytes at offset 1099511627776 > +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > +read 1024/1024 bytes at offset 1099511627776 > +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > *** done This test is not. It works even without the fix for me. The problem is that this does not depend on the guest disk size or the guest offset, but on the host offset. So the problem only appears if the image has been filled enough so that the host offsets grow large enough (apparently, at least). However, the first host offset written to does depend on the image size. For 2 GB images, it's 0x90000; for 4 TB images, it's 0x20110000; and for 16 TB images, it finally overflows, *independently* on the guest offset written to. So I suggest you create a 16 TB image here (or maybe an image which is as large as possible, for testing purposes) and then you can write anywhere (maybe once at the very beginning and once at the very end, if that works). Max