[Qemu-devel] [PATCH v2 07/11] vdi: Fix .bdrv_has_zero_init()

Max Reitz posted 11 patches 6 years, 6 months ago
Maintainers: "Denis V. Lunev" <den@openvz.org>, Kevin Wolf <kwolf@redhat.com>, Peter Lieven <pl@kamp.de>, Jeff Cody <codyprime@gmail.com>, Stefan Weil <sw@weilnetz.de>, Max Reitz <mreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, John Snow <jsnow@redhat.com>, Markus Armbruster <armbru@redhat.com>, Jason Dillaman <dillaman@redhat.com>, "Richard W.M. Jones" <rjones@redhat.com>, Liu Yuan <namei.unix@gmail.com>
[Qemu-devel] [PATCH v2 07/11] vdi: Fix .bdrv_has_zero_init()
Posted by Max Reitz 6 years, 6 months ago
Static VDI images cannot guarantee to be zero-initialized.  If the image
has been statically allocated, forward the call to the underlying
storage node.

Reported-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Stefano Garzarella <sgarzare@redhat.com>
---
 block/vdi.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/block/vdi.c b/block/vdi.c
index b9845a4cbd..0caa3f281d 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -988,6 +988,17 @@ static void vdi_close(BlockDriverState *bs)
     error_free(s->migration_blocker);
 }
 
+static int vdi_has_zero_init(BlockDriverState *bs)
+{
+    BDRVVdiState *s = bs->opaque;
+
+    if (s->header.image_type == VDI_TYPE_STATIC) {
+        return bdrv_has_zero_init(bs->file->bs);
+    } else {
+        return 1;
+    }
+}
+
 static QemuOptsList vdi_create_opts = {
     .name = "vdi-create-opts",
     .head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head),
@@ -1028,7 +1039,7 @@ static BlockDriver bdrv_vdi = {
     .bdrv_child_perm          = bdrv_format_default_perms,
     .bdrv_co_create      = vdi_co_create,
     .bdrv_co_create_opts = vdi_co_create_opts,
-    .bdrv_has_zero_init = bdrv_has_zero_init_1,
+    .bdrv_has_zero_init  = vdi_has_zero_init,
     .bdrv_co_block_status = vdi_co_block_status,
     .bdrv_make_empty = vdi_make_empty,
 
-- 
2.21.0


Re: [Qemu-devel] [PATCH v2 07/11] vdi: Fix .bdrv_has_zero_init()
Posted by Maxim Levitsky 6 years, 6 months ago
On Wed, 2019-07-24 at 19:12 +0200, Max Reitz wrote:
> Static VDI images cannot guarantee to be zero-initialized.  If the image
> has been statically allocated, forward the call to the underlying
> storage node.
> 
> Reported-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Stefan Weil <sw@weilnetz.de>
> Acked-by: Stefano Garzarella <sgarzare@redhat.com>
> Tested-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  block/vdi.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/block/vdi.c b/block/vdi.c
> index b9845a4cbd..0caa3f281d 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -988,6 +988,17 @@ static void vdi_close(BlockDriverState *bs)
>      error_free(s->migration_blocker);
>  }
>  
> +static int vdi_has_zero_init(BlockDriverState *bs)
> +{
> +    BDRVVdiState *s = bs->opaque;
> +
> +    if (s->header.image_type == VDI_TYPE_STATIC) {
> +        return bdrv_has_zero_init(bs->file->bs);
> +    } else {
> +        return 1;
> +    }
> +}
> +
>  static QemuOptsList vdi_create_opts = {
>      .name = "vdi-create-opts",
>      .head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head),
> @@ -1028,7 +1039,7 @@ static BlockDriver bdrv_vdi = {
>      .bdrv_child_perm          = bdrv_format_default_perms,
>      .bdrv_co_create      = vdi_co_create,
>      .bdrv_co_create_opts = vdi_co_create_opts,
> -    .bdrv_has_zero_init = bdrv_has_zero_init_1,
> +    .bdrv_has_zero_init  = vdi_has_zero_init,
>      .bdrv_co_block_status = vdi_co_block_status,
>      .bdrv_make_empty = vdi_make_empty,
>  


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

I see that the code when it allocates a new block at the end of the file, actually zeroes it out, so most
likely this is right.


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