[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v10 03/17] qcow2: Reuse preallocated zero cluste
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v10 03/17] qcow2: Reuse preallocated zero clusters |
Date: |
Fri, 28 Apr 2017 19:51:10 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
On 27.04.2017 03:46, Eric Blake wrote:
> From: Max Reitz <address@hidden>
>
> Instead of just freeing preallocated zero clusters and completely
> allocating them from scratch, reuse them.
>
> We cannot do this in handle_copied(), however, since this is a COW
> operation. Therefore, we have to add the new logic to handle_alloc() and
> simply return the existing offset if it exists. The only catch is that
> we have to convince qcow2_alloc_cluster_link_l2() not to free the old
> clusters (because we have reused them).
>
> Reported-by: Eric Blake <address@hidden>
> Signed-off-by: Max Reitz <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v10: new patch. Max hasn't posted the patch directly on list, but
> did mention it here:
> https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg03936.html
> and that he would post it once he has tests. Well, my later patches
> add a test that requires this one :) The other two patches that
> he mentioned there are also good, but not essential for my series
> (and I didn't want to write tests to expose the behavior difference,
> because it would deprive Max of that fun).
Well, the main reason I didn't send the patches yet is because I was
tired while writing them ("Date: Sat Apr 22 01:17:52 2017 +0200") and I
wanted to take another look before sending them. I guess now's as good a
time as any.
> ---
> block/qcow2.h | 3 ++
> block/qcow2-cluster.c | 83
> +++++++++++++++++++++++++++++++++++----------------
> 2 files changed, 60 insertions(+), 26 deletions(-)
[...]
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index d1063df..db3d937 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
[...]
> @@ -1192,31 +1199,53 @@ static int handle_alloc(BlockDriverState *bs,
> uint64_t guest_offset,
> * wrong with our code. */
> assert(nb_clusters > 0);
>
> + if (!*host_offset && qcow2_get_cluster_type(entry) == QCOW2_CLUSTER_ZERO
> &&
> + (entry & L2E_OFFSET_MASK) != 0 && (entry & QCOW_OFLAG_COPIED))
*host_offset works with this, too, if
start_of_cluster(s, *host_offset) == (entry & L2E_OFFSET_MASK).
If !(entry & QCOW_OFLAG_COPIED), we should check whether the refcount
maybe is 1 and then set OFLAG_COPIED. But that is something we don't
even do for normal clusters yet, so it's something to fix another day.
> + {
> + /* Try to reuse preallocated zero clusters; contiguous normal
> clusters
> + * would be fine, too, but count_cow_clusters() above has limited
> + * nb_clusters already to a range of COW clusters */
> + int preallocated_nb_clusters =
> + count_contiguous_clusters(nb_clusters, s->cluster_size, l2_table,
s/l2_table/&l2_table[l2_index]/
> + QCOW_OFLAG_COPIED);
> +
> + if (preallocated_nb_clusters) {
preallocated_nb_clusters must be at least 1, so an assertion would be
better.
Max
> + nb_clusters = preallocated_nb_clusters;
> + alloc_cluster_offset = entry & L2E_OFFSET_MASK;
> +
> + /* We want to reuse these clusters, so
> qcow2_alloc_cluster_link_l2()
> + * should not free them. */
> + keep_old_clusters = true;
> + }
> + }
> +
> qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
>
> - /* Allocate, if necessary at a given offset in the image file */
> - alloc_cluster_offset = start_of_cluster(s, *host_offset);
> - ret = do_alloc_cluster_offset(bs, guest_offset, &alloc_cluster_offset,
> - &nb_clusters);
> - if (ret < 0) {
> - goto fail;
> - }
> -
> - /* Can't extend contiguous allocation */
> - if (nb_clusters == 0) {
> - *bytes = 0;
> - return 0;
> - }
> -
> - /* !*host_offset would overwrite the image header and is reserved for "no
> - * host offset preferred". If 0 was a valid host offset, it'd trigger the
> - * following overlap check; do that now to avoid having an invalid value
> in
> - * *host_offset. */
> if (!alloc_cluster_offset) {
> - ret = qcow2_pre_write_overlap_check(bs, 0, alloc_cluster_offset,
> - nb_clusters * s->cluster_size);
> - assert(ret < 0);
> - goto fail;
> + /* Allocate, if necessary at a given offset in the image file */
> + alloc_cluster_offset = start_of_cluster(s, *host_offset);
> + ret = do_alloc_cluster_offset(bs, guest_offset,
> &alloc_cluster_offset,
> + &nb_clusters);
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + /* Can't extend contiguous allocation */
> + if (nb_clusters == 0) {
> + *bytes = 0;
> + return 0;
> + }
> +
> + /* !*host_offset would overwrite the image header and is reserved for
> + * "no host offset preferred". If 0 was a valid host offset, it'd
> + * trigger the following overlap check; do that now to avoid having
> an
> + * invalid value in *host_offset. */
> + if (!alloc_cluster_offset) {
> + ret = qcow2_pre_write_overlap_check(bs, 0, alloc_cluster_offset,
> + nb_clusters *
> s->cluster_size);
> + assert(ret < 0);
> + goto fail;
> + }
> }
>
> /*
> @@ -1247,6 +1276,8 @@ static int handle_alloc(BlockDriverState *bs, uint64_t
> guest_offset,
> .offset = start_of_cluster(s, guest_offset),
> .nb_clusters = nb_clusters,
>
> + .keep_old_clusters = keep_old_clusters,
> +
> .cow_start = {
> .offset = 0,
> .nb_bytes = offset_into_cluster(s, guest_offset),
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v10 00/17] add blkdebug tests, Eric Blake, 2017/04/26
- [Qemu-block] [PATCH v10 02/17] qcow2: Correctly report status of preallocated zero clusters, Eric Blake, 2017/04/26
- [Qemu-block] [PATCH v10 01/17] block: Update comments on BDRV_BLOCK_* meanings, Eric Blake, 2017/04/26
- [Qemu-block] [PATCH v10 03/17] qcow2: Reuse preallocated zero clusters, Eric Blake, 2017/04/26
- Re: [Qemu-block] [PATCH v10 03/17] qcow2: Reuse preallocated zero clusters,
Max Reitz <=
- [Qemu-block] [PATCH v10 04/17] qcow2: Optimize zero_single_l2() to minimize L2 churn, Eric Blake, 2017/04/26
- [Qemu-block] [PATCH v10 06/17] qemu-io: Don't open-code QEMU_IS_ALIGNED, Eric Blake, 2017/04/26
- [Qemu-block] [PATCH v10 05/17] iotests: Add test 179 to cover write zeroes with unmap, Eric Blake, 2017/04/26
- [Qemu-block] [PATCH v10 07/17] qemu-io: Switch 'alloc' command to byte-based length, Eric Blake, 2017/04/26