diff mbox

[3/4] block: vhdx - change .vhdx_create default block state to ZERO

Message ID 85164899eacc86e150c3ceba793cf93b398dedd7.1418018421.git.jcody@redhat.com
State New
Headers show

Commit Message

Jeff Cody Dec. 8, 2014, 6:07 a.m. UTC
The VHDX spec specifies that the default new block state is
PAYLOAD_BLOCK_NOT_PRESENT for a dynamic VHDX image, and
PAYLOAD_BLOCK_FULLY_PRESENT for a fixed VHDX image.

However, in order to create space-efficient VHDX images with qemu-img
convert, it is desirable to be able to set has_zero_init to true for
VHDX.

There is currently an option when creating VHDX images, to use block
state ZERO for new blocks.  However, this currently defaults to 'off'.
In order to be able to eventually set has_zero_init to true for VHDX,
this needs to default to 'on'.

This patch changes the default to 'on', and provides some help
information to warn against setting it to 'off' when using qemu-img
convert.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vhdx.c  | 6 ++++--
 qemu-doc.texi | 6 +++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

Comments

Max Reitz Dec. 8, 2014, 8:48 a.m. UTC | #1
On 2014-12-08 at 07:07, Jeff Cody wrote:
> The VHDX spec specifies that the default new block state is
> PAYLOAD_BLOCK_NOT_PRESENT for a dynamic VHDX image, and
> PAYLOAD_BLOCK_FULLY_PRESENT for a fixed VHDX image.
>
> However, in order to create space-efficient VHDX images with qemu-img
> convert, it is desirable to be able to set has_zero_init to true for
> VHDX.
>
> There is currently an option when creating VHDX images, to use block
> state ZERO for new blocks.  However, this currently defaults to 'off'.
> In order to be able to eventually set has_zero_init to true for VHDX,
> this needs to default to 'on'.
>
> This patch changes the default to 'on', and provides some help
> information to warn against setting it to 'off' when using qemu-img
> convert.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/vhdx.c  | 6 ++++--
>   qemu-doc.texi | 6 +++++-
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/block/vhdx.c b/block/vhdx.c
> index bec10bd..ddefc2a 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1774,7 +1774,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
>       log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
>       block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
>       type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
> -    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, false);
> +    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, true);
>   
>       if (image_size > VHDX_MAX_IMAGE_SIZE) {
>           error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB");
> @@ -1936,7 +1936,9 @@ static QemuOptsList vhdx_create_opts = {
>          {
>              .name = VHDX_BLOCK_OPT_ZERO,
>              .type = QEMU_OPT_BOOL,
> -           .help = "Force use of payload blocks of type 'ZERO'.  Non-standard."
> +           .help = "Force use of payload blocks of type 'ZERO'. "\
> +                   "Non-standard, but default.  Do not set to 'off' when "\
> +                   "using 'qemu-img convert' with subformat=dynamic"

Full stop missing?

>          },
>          { NULL }
>       }
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index ad418f8..aabe8df 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -698,7 +698,11 @@ Supported options:
>   Specifies which VHDX subformat to use. Valid options are
>   @code{dynamic} (default) and @code{fixed}.
>   @item block_state_zero
> -Force use of payload blocks of type 'ZERO'.
> +Force use of payload blocks of type 'ZERO'.  Can be set to @code{on} (default)
> +or @code{off}.  When set to @code{off}, new blocks will be created as
> +@code{PAYLOAD_BLOCK_NOT_PRESENT}, which means parsers are free to return
> +arbitrary data for those blocks.  Do not set to @code{off} when using
> +@code{qemu-img convert} with @code{subformat=dynamic}.

Well, it's fine to use block_state_zero=off with -S 0, but then you 
won't get much usage out of having set it to off because all blocks will 
be allocated anyway.

>   @item block_size
>   Block size; min 1 MB, max 256 MB.  0 means auto-calculate based on image size.
>   @item log_size

With the full stop added:

Reviewed-by: Max Reitz <mreitz@redhat.com>
diff mbox

Patch

diff --git a/block/vhdx.c b/block/vhdx.c
index bec10bd..ddefc2a 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1774,7 +1774,7 @@  static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
     block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
     type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
-    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, false);
+    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, true);
 
     if (image_size > VHDX_MAX_IMAGE_SIZE) {
         error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB");
@@ -1936,7 +1936,9 @@  static QemuOptsList vhdx_create_opts = {
        {
            .name = VHDX_BLOCK_OPT_ZERO,
            .type = QEMU_OPT_BOOL,
-           .help = "Force use of payload blocks of type 'ZERO'.  Non-standard."
+           .help = "Force use of payload blocks of type 'ZERO'. "\
+                   "Non-standard, but default.  Do not set to 'off' when "\
+                   "using 'qemu-img convert' with subformat=dynamic"
        },
        { NULL }
     }
diff --git a/qemu-doc.texi b/qemu-doc.texi
index ad418f8..aabe8df 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -698,7 +698,11 @@  Supported options:
 Specifies which VHDX subformat to use. Valid options are
 @code{dynamic} (default) and @code{fixed}.
 @item block_state_zero
-Force use of payload blocks of type 'ZERO'.
+Force use of payload blocks of type 'ZERO'.  Can be set to @code{on} (default)
+or @code{off}.  When set to @code{off}, new blocks will be created as
+@code{PAYLOAD_BLOCK_NOT_PRESENT}, which means parsers are free to return
+arbitrary data for those blocks.  Do not set to @code{off} when using
+@code{qemu-img convert} with @code{subformat=dynamic}.
 @item block_size
 Block size; min 1 MB, max 256 MB.  0 means auto-calculate based on image size.
 @item log_size