All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>, qemu-block@nongnu.org
Cc: mreitz@redhat.com, den@openvz.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 3/3] qemu-iotests: Some more write_zeroes tests
Date: Tue, 17 May 2016 14:35:17 -0600	[thread overview]
Message-ID: <573B8085.4010706@redhat.com> (raw)
In-Reply-To: <1463503863-19009-4-git-send-email-kwolf@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4315 bytes --]

On 05/17/2016 10:51 AM, Kevin Wolf wrote:
> This covers some more write_zeroes cases which are relevant for the
> recent qcow2 optimisations that check the allocation status of the
> backing file for partial cluster write_zeroes requests.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---

> +# qcow2 specific bdrv_write_zeroes tests with backing files (complements 034)

> +_supported_fmt qcow2

I guess test 034 covered cases of larger-than-sector write_zero, and
that the reason you needed a new test id is because this one is qcow2
specific (the general test covers multiple formats, but the behavior on
sub-cluster zeroing is specific to qcow2). Makes sense so far.

> +
> +CLUSTER_SIZE=4k
> +size=128M
> +
> +echo
> +echo == backing file contains zeros ==
> +
> +# Make sure that the whole cluster is allocated even for partial write_zeroes
> +# when the backing file contains zeros
> +
> +CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
> +_make_test_img -b "$TEST_IMG.base"
> +
> +$QEMU_IO -c "write -z 0k 2k" "$TEST_IMG" | _filter_qemu_io

head aligned, tail is unaligned

> +$QEMU_IO -c "write -z 10k 2k" "$TEST_IMG" | _filter_qemu_io

head is unaligned, tail is unaligned

> +$QEMU_IO -c "write -z 17k 2k" "$TEST_IMG" | _filter_qemu_io

both unaligned, and completely contained within a single cluster

No test of crossing a cluster boundary at this point?  I would probably do:

$QEMU_IO -c "write -z 27k 2k" "$TEST_IMG" | _filter_qemu_io

then check that the map shows an allocation starting at 24k for 8192 bytes

> +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
> +
> +echo
> +echo == backing file contains non-zero data before write_zeros ==
> +
> +CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
> +_make_test_img -b "$TEST_IMG.base"
> +
> +$QEMU_IO -c "write -P 0x11 32k 1k" "$TEST_IMG.base" | _filter_qemu_io
> +$QEMU_IO -c "write -z 34k 1k" "$TEST_IMG" | _filter_qemu_io

cluster contains partial non-zero data, which does not overlap with the
write zero request. Good - you're testing that we don't nuke the rest of
the cluster (that is, we cannot convert an entire cluster to 0 unless
all sectors read as 0 before the conversion), but instead do proper
copy-on-write.

> +$QEMU_IO -c "read -P 0x11 32k 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 33k 3k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
> +
> +echo
> +echo == backing file contains non-zero data after write_zeros ==
> +
> +CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
> +_make_test_img -b "$TEST_IMG.base"
> +
> +$QEMU_IO -c "write -P 0x11 43k 1k" "$TEST_IMG.base" | _filter_qemu_io
> +$QEMU_IO -c "write -z 41k 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0x11 43k 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 40k 3k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
> +
> +echo
> +echo == spanning multiple clusters ==
> +
> +CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
> +_make_test_img -b "$TEST_IMG.base"
> +
> +$QEMU_IO -c "write -P 0x11 74k 2k" "$TEST_IMG.base" | _filter_qemu_io
> +$QEMU_IO -c "write -z 66k 7k" "$TEST_IMG" | _filter_qemu_io

the tail from 72k to 73k overlaps the sector that has to be
copy-on-write to pick up the data at 74k; while the earlier portion
picks up the clusters at 64k and 68k.  Looks good.

What you have looks fine, but again, I'd consider also testing a
sub-cluster size 'write -z' that spans a cluster boundary, including the
case where it is overwriting data from the backing file.  Something like:

$QEMU_IO -c "write -P 0x11 2k 4k" "$TEST_IMG.base" | _filter_qemu_io
$QEMU_IO -c "write -z 3k 2k" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -P 0 0k 2k" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -P 0x11 2k 1k" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -P 0 3k 2k" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -P 0x11 5k 1k" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -P 0 6k 2k" "$TEST_IMG" | _filter_qemu_io

-- 
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: 604 bytes --]

  reply	other threads:[~2016-05-18  1:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-17 16:51 [Qemu-devel] [PATCH 0/3] qcow2: write_zeroes corruption fixes and tests Kevin Wolf
2016-05-17 16:51 ` [Qemu-devel] [PATCH 1/3] qcow2: fix condition in is_zero_cluster Kevin Wolf
2016-05-17 17:25   ` Eric Blake
2016-05-17 16:51 ` [Qemu-devel] [PATCH 2/3] qcow2: Fix write_zeroes with partially allocated backing file cluster Kevin Wolf
2016-05-17 17:22   ` Eric Blake
2016-05-17 19:20   ` Denis V. Lunev
2016-05-17 16:51 ` [Qemu-devel] [PATCH 3/3] qemu-iotests: Some more write_zeroes tests Kevin Wolf
2016-05-17 20:35   ` Eric Blake [this message]
2016-05-18 10:09     ` Kevin Wolf
2016-05-18 12:27     ` [Qemu-devel] [PATCH v2 " Kevin Wolf
2016-05-18 14:12       ` Eric Blake
2016-05-18 14:22         ` Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=573B8085.4010706@redhat.com \
    --to=eblake@redhat.com \
    --cc=den@openvz.org \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.