22.04.2019 16:59, Eric Blake wrote: > On 4/19/19 9:05 AM, Vladimir Sementsov-Ogievskiy wrote: >> This test checks bug in qcow2_process_discards, fixed by previous >> commit. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- > >> +# This test checks that qcow2_process_discards does not truncate a discard >> +# request > 2G. >> +# To reproduce bug we need to overflow int by one sequential discard, so we >> +# need size > 2G, bigger cluster size (as with default 64k we may have >> maximum >> +# of 512M sequential data, corresponding to one L1 entry), and we need some >> +# data of the beginning of the disk mapped to the end of file to prevent >> +# bdrv_co_truncate(bs->file) call in qcow2_co_truncate(), which may success > > s/may success/might succeed/ > >> +# anyway. >> + >> +size=2100M >> +IMGOPTS="cluster_size=1M,preallocation=metadata" >> + >> +_make_test_img $size >> +$QEMU_IO -c 'discard 0 10M' -c 'discard 2090M 10M' \ >> + -c 'write 2090M 10M' -c 'write 0 10M' "$TEST_IMG" | _filter_qemu_io >> + >> +# Check that our trick with swapping first and last 10M chunks succeeded. >> +# Otherwise test will may pass even if bdrv_pdiscard() fails in >> +# qcow2_process_discards() >> +$QEMU_IMG map "$TEST_IMG" | _filter_testdir >> +$QEMU_IMG info "$TEST_IMG" | grep size | _filter_testdir > > Nice - that's a lot faster than v1! And makes the test fit in the quick > group after all. > >> + >> +$QEMU_IMG -T 'qcow2_process_discards_failed*' resize --shrink "$TEST_IMG" 5M > > However, I'm quite certain that trace output is not reliable in iotests. > Depending on configure options, traces might not be enabled at all, or > might not go to stderr. Drop the -T '...'. Even without the trace line, > >> + >> +$QEMU_IMG info "$TEST_IMG" | grep size | _filter_testdir > > this second image info is sufficient to prove whether the resize had an > effect (post-patch) or exposes the bug (without patch 2/3). That is, > applying this patch but not 2/3, I see this (with my cleanups to > qemu-img info in place, from Kevin's block-next branch): > > +++ /home/eblake/qemu/tests/qemu-iotests/250.out.bad 2019-04-22 > 08:52:26.072968731 -0500 > @@ -14,8 +14,9 @@ > virtual size: 2.05 GiB (2202009600 bytes) > disk size: 24 MiB > cluster_size: 1048576 > +18274@1555941145.999195:qcow2_process_discards_failed_region offset > 0x500000 bytes 0x82a00000 ret -5 > Image resized. > virtual size: 5 MiB (5242880 bytes) > -disk size: 9.01 MiB > +disk size: 19 MiB > > where the trace did indeed show show that we had a bug, but where even > without the trace, the difference in size between 19M with incomplete > discards vs. 9M with patch 2/3 is enough to show that patch 2/3 fixes a bug. > >> +virtual size: 2.1G (2202009600 bytes) >> +disk size: 24M >> +cluster_size: 1048576 >> +Image resized. >> +virtual size: 5.0M (5242880 bytes) >> +disk size: 9.0M >> +cluster_size: 1048576 > > When Kevin's block-next branch is applied, you'll need this squashed in: > > diff --git a/tests/qemu-iotests/250.out b/tests/qemu-iotests/250.out > index 37e37f0c9e7..d1c1c7180b5 100644 > --- a/tests/qemu-iotests/250.out > +++ b/tests/qemu-iotests/250.out > @@ -11,11 +11,11 @@ wrote 10485760/10485760 bytes at offset 0 > Offset Length Mapped to File > 0 0xa00000 0x82f00000 TEST_DIR/t.qcow2 > 0x82a00000 0xa00000 0x500000 TEST_DIR/t.qcow2 > -virtual size: 2.1G (2202009600 bytes) > -disk size: 24M > +virtual size: 2.05 GiB (2202009600 bytes) > +disk size: 24 MiB > cluster_size: 1048576 > Image resized. > -virtual size: 5.0M (5242880 bytes) > -disk size: 9.0M > +virtual size: 5 MiB (5242880 bytes) > +disk size: 9.01 MiB > cluster_size: 1048576 > *** done > > With the updated output to match changes to qemu-img output, the grammar > fixes, and with the -T option removed, > > Reviewed-by: Eric Blake <ebl...@redhat.com> > Tested-by: Eric Blake <ebl...@redhat.com> >
Thank you! I'll resend soon. -- Best regards, Vladimir