Re: [Qemu-devel] [PATCH] qcow2: Support BDRV_REQ_MAY_UNMAP

[Kevin Wolf <kwolf@redhat.com>]

Am 28.09.2016 um 09:04 hat Fam Zheng geschrieben:
> Handling this is similar to what is done to the L2 entry in the case of
> compressed clusters.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/qcow2-cluster.c | 9 +++++----
>  block/qcow2.c         | 3 ++-
>  block/qcow2.h         | 3 ++-
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 61d1ffd..928c1e2 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1558,7 +1558,7 @@ fail:
>   * clusters.
>   */
>  static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
> -                          uint64_t nb_clusters)
> +                          uint64_t nb_clusters, int flags)
>  {
>      BDRVQcow2State *s = bs->opaque;
>      uint64_t *l2_table;
> @@ -1582,7 +1582,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
>  
>          /* Update L2 entries */
>          qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
> -        if (old_offset & QCOW_OFLAG_COMPRESSED) {
> +        if (old_offset & QCOW_OFLAG_COMPRESSED || flags & BDRV_REQ_MAY_UNMAP) {
>              l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
>              qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
>          } else {

I don't think we should always do this for BDRV_REQ_MAY_UNMAP. The user
may want to keep the image fully allocated even if the guest (or block
job etc.) thinks this can be discarded.

When we discussed this in another email thread (?), I think I suggested
checking whether the image was opened with pass-discard-request=on. If
so, go ahead and deallocate the cluster, otherwise keep it allocated.

In those cases where pass-discard-request=off, it doesn't make a lot of
sense to deallocate the cluster anyway because it won't shrink the file
size.

Kevin