[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9 28/34] qcow2: Add subcluster support to qcow2_co_pwrite_ze
From: |
Max Reitz |
Subject: |
Re: [PATCH v9 28/34] qcow2: Add subcluster support to qcow2_co_pwrite_zeroes() |
Date: |
Thu, 2 Jul 2020 16:28:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 |
On 28.06.20 13:02, Alberto Garcia wrote:
> This works now at the subcluster level and pwrite_zeroes_alignment is
> updated accordingly.
>
> qcow2_cluster_zeroize() is turned into qcow2_subcluster_zeroize() with
> the following changes:
>
> - The request can now be subcluster-aligned.
>
> - The cluster-aligned body of the request is still zeroized using
> zero_in_l2_slice() as before.
>
> - The subcluster-aligned head and tail of the request are zeroized
> with the new zero_l2_subclusters() function.
>
> There is just one thing to take into account for a possible future
> improvement: compressed clusters cannot be partially zeroized so
> zero_l2_subclusters() on the head or the tail can return -ENOTSUP.
> This makes the caller repeat the *complete* request and write actual
> zeroes to disk. This is sub-optimal because
>
> 1) if the head area was compressed we would still be able to use
> the fast path for the body and possibly the tail.
>
> 2) if the tail area was compressed we are writing zeroes to the
> head and the body areas, which are already zeroized.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2.h | 4 +--
> block/qcow2-cluster.c | 80 +++++++++++++++++++++++++++++++++++++++----
> block/qcow2.c | 27 ++++++++-------
> 3 files changed, 90 insertions(+), 21 deletions(-)
[...]
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index deff838fe8..1641976028 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -2015,12 +2015,58 @@ static int zero_in_l2_slice(BlockDriverState *bs,
> uint64_t offset,
> return nb_clusters;
> }
>
> -int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t offset,
> - uint64_t bytes, int flags)
> +static int zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> + unsigned nb_subclusters)
> +{
> + BDRVQcow2State *s = bs->opaque;
> + uint64_t *l2_slice;
> + uint64_t old_l2_bitmap, l2_bitmap;
> + int l2_index, ret, sc = offset_to_sc_index(s, offset);
> +
> + /* For full clusters use zero_in_l2_slice() instead */
> + assert(nb_subclusters > 0 && nb_subclusters <
> s->subclusters_per_cluster);
> + assert(sc + nb_subclusters <= s->subclusters_per_cluster);
Maybe we should also assert that @offset is aligned to the subcluster size.
[...]
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 86258fbc22..4edc3c72b9 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
[...]
> @@ -4367,12 +4367,13 @@ static int coroutine_fn
> qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> uint64_t zero_start = QEMU_ALIGN_UP(old_length, s->cluster_size);
Can we instead align this to just subclusters?
>
> /*
> - * Use zero clusters as much as we can. qcow2_cluster_zeroize()
> + * Use zero clusters as much as we can. qcow2_subcluster_zeroize()
> * requires a cluster-aligned start. The end may be unaligned if it
> is
s/cluster/subcluster/?
Max
> * at the end of the image (which it is here).
> */
> if (offset > zero_start) {
> - ret = qcow2_cluster_zeroize(bs, zero_start, offset - zero_start,
> 0);
> + ret = qcow2_subcluster_zeroize(bs, zero_start, offset -
> zero_start,
> + 0);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Failed to zero out new
> clusters");
> goto fail;
>
signature.asc
Description: OpenPGP digital signature
- Re: [PATCH v9 28/34] qcow2: Add subcluster support to qcow2_co_pwrite_zeroes(),
Max Reitz <=