Re: [Qemu-devel] [PATCH v2 06/11] qcow2: Fix .bdrv_has_zero_init()

[Maxim Levitsky <mlevitsk@redhat.com>]

On Wed, 2019-07-24 at 19:12 +0200, Max Reitz wrote:
> If a qcow2 file is preallocated, it can no longer guarantee that it
> initially appears as filled with zeroes.
> 
> So implement .bdrv_has_zero_init() by checking whether the file is
> preallocated; if so, forward the call to the underlying storage node,
> except for when it is encrypted: Encrypted preallocated images always
> return effectively random data, so .bdrv_has_zero_init() must always
> return 0 for them.
> 
> .bdrv_has_zero_init_truncate() can remain bdrv_has_zero_init_1(),
> because it presupposes PREALLOC_MODE_OFF.
> 
> Reported-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 5c40f54d64..b4e73aa443 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -4631,6 +4631,33 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs,
>      return spec_info;
>  }
>  
> +static int qcow2_has_zero_init(BlockDriverState *bs)
> +{
> +    BDRVQcow2State *s = bs->opaque;
> +    bool preallocated;
> +
> +    if (qemu_in_coroutine()) {
> +        qemu_co_mutex_lock(&s->lock);
> +    }
> +    /*
> +     * Check preallocation status: Preallocated images have all L2
> +     * tables allocated, nonpreallocated images have none.  It is
> +     * therefore enough to check the first one.
> +     */
> +    preallocated = s->l1_size > 0 && s->l1_table[0] != 0;
> +    if (qemu_in_coroutine()) {
> +        qemu_co_mutex_unlock(&s->lock);
> +    }
> +
> +    if (!preallocated) {
> +        return 1;
> +    } else if (bs->encrypted) {
> +        return 0;
> +    } else {
> +        return bdrv_has_zero_init(s->data_file->bs);
> +    }
> +}
> +
>  static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
>                                int64_t pos)
>  {
> @@ -5186,7 +5213,7 @@ BlockDriver bdrv_qcow2 = {
>      .bdrv_child_perm      = bdrv_format_default_perms,
>      .bdrv_co_create_opts  = qcow2_co_create_opts,
>      .bdrv_co_create       = qcow2_co_create,
> -    .bdrv_has_zero_init = bdrv_has_zero_init_1,
> +    .bdrv_has_zero_init   = qcow2_has_zero_init,
>      .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
>      .bdrv_co_block_status = qcow2_co_block_status,
>  


Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
	Maxim Levitsky