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 <be...@igalia.com> > Reviewed-by: Eric Blake <ebl...@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
