Re: [Qemu-devel] [PATCH v5 02/12] qcow2: Implement bdrv_make_empty()

[Kevin Wolf]

Am 17.04.2014 um 23:59 hat Max Reitz geschrieben:
> Implement bdrv_make_empty() by making all clusters in the image fall
> through to the backing file (via the now modified discard).
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block/qcow2.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 1e7b7d5..4d70665 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2006,6 +2006,27 @@ fail:
>      return ret;
>  }
>  
> +static int qcow2_make_empty(BlockDriverState *bs)
> +{
> +     uint64_t start_sector;
> +     int ret = 0;
> +
> +     /* The step taken here may not exceed INT_MAX when multiplied by
> +      * BDRV_SECTOR_SIZE. 64k is arbitrary, but works well. */

So you really mean something like this?

int max_sectors_per_iteration = (INT_MAX / BDRV_SECTOR_SIZE);

> +     for (start_sector = 0; start_sector < bs->total_sectors;
> +          start_sector += 65536)
> +     {
> +         ret = qcow2_discard_clusters(bs, start_sector * BDRV_SECTOR_SIZE,
> +                 MIN(65536, bs->total_sectors - start_sector),
> +                 QCOW2_DISCARD_REQUEST, true);

QCOW2_DISCARD_REQUEST is for requests that come from the guest. SNAPSHOT
would be a nice fit if we reinterpreted it so that it doesn't only refer
to internal snapshots but also to external ones. I would be okay with
OTHER as well if you prefer.

Perhaps a look at the defaults can help us: SNAPSHOT defaults to on,
OTHER to off. I think it totally makes sense to physically discard
clusters in qcow2_make_empty() in almost every case, so that might be a
good reason to use SNAPSHOT here.

> +         if (ret < 0) {
> +             break;
> +         }
> +     }
> +
> +     return ret;
> +}
> +

Kevin