On 5/13/24 08:32, Andrey Drobyshev wrote:
> When zeroizing subclusters within single cluster, detect usage of the
> BDRV_REQ_MAY_UNMAP flag and fall through to the subcluster-based discard
> operation, much like it's done with the cluster-based discards. That
> way subcluster-aligned operations "qemu-io -c 'write -z -u ...'" will
> lead to actual unmap.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/qcow2-cluster.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 3c134a7e80..53e04eff93 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -2107,15 +2107,16 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
>
> static int coroutine_fn GRAPH_RDLOCK
> discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> - uint64_t nb_subclusters,
> - enum qcow2_discard_type type,
> - bool full_discard)
> + uint64_t nb_subclusters, enum qcow2_discard_type type,
> + bool full_discard, bool uncond_zeroize)
> {
> BDRVQcow2State *s = bs->opaque;
> uint64_t new_l2_bitmap, bitmap_alloc_mask, bitmap_zero_mask;
> int ret, sc = offset_to_sc_index(s, offset);
> g_auto(SubClusterRangeInfo) scri = { 0 };
>
> + assert(!(full_discard && uncond_zeroize));
> +
> ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
> if (ret < 0) {
> return ret;
> @@ -2140,7 +2141,8 @@ discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> */
> if (full_discard) {
> new_l2_bitmap &= ~bitmap_zero_mask;
> - } else if (bs->backing || scri.l2_bitmap & bitmap_alloc_mask) {
> + } else if (uncond_zeroize || bs->backing ||
> + scri.l2_bitmap & bitmap_alloc_mask) {
> new_l2_bitmap |= bitmap_zero_mask;
> }
>
> @@ -2197,7 +2199,7 @@ int qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset,
> if (head) {
> ret = discard_l2_subclusters(bs, offset - head,
> size_to_subclusters(s, head), type,
> - full_discard);
> + full_discard, false);
> if (ret < 0) {
> goto fail;
> }
> @@ -2221,7 +2223,7 @@ int qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset,
> if (tail) {
> ret = discard_l2_subclusters(bs, end_offset,
> size_to_subclusters(s, tail), type,
> - full_discard);
> + full_discard, false);
> if (ret < 0) {
> goto fail;
> }
> @@ -2318,6 +2320,18 @@ zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> int ret, sc = offset_to_sc_index(s, offset);
> g_auto(SubClusterRangeInfo) scri = { 0 };
>
> + /*
> + * If the request allows discarding subclusters we go down the discard
> + * path regardless of their allocation status. After the discard
> + * operation with full_discard=false subclusters are going to be read as
> + * zeroes anyway. But we make sure that subclusters are explicitly
> + * zeroed anyway with uncond_zeroize=true.
> + */
> + if (flags & BDRV_REQ_MAY_UNMAP) {
> + return discard_l2_subclusters(bs, offset, nb_subclusters,
> + QCOW2_DISCARD_REQUEST, false, true);
> + }
> +
> ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
> if (ret < 0) {
> return ret;
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov