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

[Maxim Levitsky <mlevitsk@redhat.com>]

On Wed, 2019-07-24 at 19:12 +0200, Max Reitz wrote:
> Fixed VHDX images cannot guarantee to be zero-initialized.  If the image
> has the "fixed" subformat, forward the call to the underlying storage
> node.
> 
> Reported-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/vhdx.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index a02d1c99a7..6a09d0a55c 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -2075,6 +2075,30 @@ static int coroutine_fn vhdx_co_check(BlockDriverState *bs,
>      return 0;
>  }
>  
> +static int vhdx_has_zero_init(BlockDriverState *bs)
> +{
> +    BDRVVHDXState *s = bs->opaque;
> +    int state;
> +
> +    /*
> +     * Check the subformat: Fixed images have all BAT entries present,
> +     * dynamic images have none (right after creation).  It is
> +     * therefore enough to check the first BAT entry.
> +     */
> +    if (!s->bat_entries) {
> +        return 1;
> +    }
> +
> +    state = s->bat[0] & VHDX_BAT_STATE_BIT_MASK;
> +    if (state == PAYLOAD_BLOCK_FULLY_PRESENT) {
> +        /* Fixed subformat */
> +        return bdrv_has_zero_init(bs->file->bs);
> +    }
> +
> +    /* Dynamic subformat */
> +    return 1;
> +}
> +
>  static QemuOptsList vhdx_create_opts = {
>      .name = "vhdx-create-opts",
>      .head = QTAILQ_HEAD_INITIALIZER(vhdx_create_opts.head),
> @@ -2128,7 +2152,7 @@ static BlockDriver bdrv_vhdx = {
>      .bdrv_co_create_opts    = vhdx_co_create_opts,
>      .bdrv_get_info          = vhdx_get_info,
>      .bdrv_co_check          = vhdx_co_check,
> -    .bdrv_has_zero_init     = bdrv_has_zero_init_1,
> +    .bdrv_has_zero_init     = vhdx_has_zero_init,
>  
>      .create_opts            = &vhdx_create_opts,
>  };

I am not familiar with VHDX format to be honest too, but knowing that dynamic format allows for growing
and static are preallocated this makes sense.

Its a bit amusing and not surprising that the the spec for this format is in .docx. 
I took a quick look to get a rough impression of the file format.


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