Re: [Qemu-devel] [v19 22/25] vhdx.c: replace QEMUOptionParameter with QemuOpts

[Kevin Wolf]

Am 20.01.2014 um 15:20 hat Chunyan Liu geschrieben:
> vhdx.c: replace QEMUOptionParameter with QemuOpts
> 
> Signed-off-by: Dong Xu Wang <address@hidden>
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
>  block/vhdx.c |   93 +++++++++++++++++++++++++++------------------------------
>  block/vhdx.h |    1 +
>  2 files changed, 45 insertions(+), 49 deletions(-)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index c78675e..8021e8e 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1711,7 +1711,7 @@ exit:
>   *    .---- ~ ----------- ~ ------------ ~ ---------------- ~ -----------.
>   *   1MB
>   */
> -static int vhdx_create(const char *filename, QEMUOptionParameter *options,
> +static int vhdx_create(const char *filename, QemuOpts *opts,
>                         Error **errp)
>  {
>      int ret = 0;
> @@ -1729,20 +1729,11 @@ static int vhdx_create(const char *filename, 
> QEMUOptionParameter *options,
>      VHDXImageType image_type;
>      Error *local_err = NULL;
>  
> -    while (options && options->name) {
> -        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> -            image_size = options->value.n;
> -        } else if (!strcmp(options->name, VHDX_BLOCK_OPT_LOG_SIZE)) {
> -            log_size = options->value.n;
> -        } else if (!strcmp(options->name, VHDX_BLOCK_OPT_BLOCK_SIZE)) {
> -            block_size = options->value.n;
> -        } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) {
> -            type = options->value.s;
> -        } else if (!strcmp(options->name, VHDX_BLOCK_OPT_ZERO)) {
> -            use_zero_blocks = options->value.n != 0;
> -        }
> -        options++;
> -    }
> +    image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> +    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, 0);
>  
>      if (image_size > VHDX_MAX_IMAGE_SIZE) {
>          error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB");
> @@ -1791,7 +1782,7 @@ static int vhdx_create(const char *filename, 
> QEMUOptionParameter *options,
>      block_size = block_size > VHDX_BLOCK_SIZE_MAX ? VHDX_BLOCK_SIZE_MAX :
>                                                      block_size;
>  
> -    ret = bdrv_create_file(filename, options, NULL, &local_err);
> +    ret = bdrv_create_file(filename, NULL, opts, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          goto exit;
> @@ -1871,37 +1862,41 @@ static int vhdx_check(BlockDriverState *bs, 
> BdrvCheckResult *result,
>      return 0;
>  }
>  
> -static QEMUOptionParameter vhdx_create_options[] = {
> -    {
> -        .name = BLOCK_OPT_SIZE,
> -        .type = OPT_SIZE,
> -        .help = "Virtual disk size; max of 64TB."
> -    },
> -    {
> -        .name = VHDX_BLOCK_OPT_LOG_SIZE,
> -        .type = OPT_SIZE,
> -        .value.n = 1 * MiB,
> -        .help = "Log size; min 1MB."
> -    },
> -    {
> -        .name = VHDX_BLOCK_OPT_BLOCK_SIZE,
> -        .type = OPT_SIZE,
> -        .value.n = 0,
> -        .help = "Block Size; min 1MB, max 256MB. " \
> -                "0 means auto-calculate based on image size."
> -    },
> -    {
> -        .name = BLOCK_OPT_SUBFMT,
> -        .type = OPT_STRING,
> -        .help = "VHDX format type, can be either 'dynamic' or 'fixed'. "\
> -                "Default is 'dynamic'."
> -    },
> -    {
> -        .name = VHDX_BLOCK_OPT_ZERO,
> -        .type = OPT_FLAG,
> -        .help = "Force use of payload blocks of type 'ZERO'.  Non-standard."
> -    },
> -    { NULL }
> +static QemuOptsList vhdx_create_opts = {
> +    .name = "vhdx-create-opts",
> +    .head = QTAILQ_HEAD_INITIALIZER(vhdx_create_opts.head),
> +    .desc = {
> +        {
> +         .name = BLOCK_OPT_SIZE,
> +         .type = QEMU_OPT_SIZE,
> +         .help = "Virtual disk size; max of 64TB."
> +     },
> +     {
> +         .name = VHDX_BLOCK_OPT_LOG_SIZE,
> +         .type = QEMU_OPT_SIZE,
> +         .def_value_str = stringify(DEFAULT_LOG_SIZE),
> +         .help = "Log size; min 1MB."
> +     },
> +     {
> +         .name = VHDX_BLOCK_OPT_BLOCK_SIZE,
> +         .type = QEMU_OPT_SIZE,
> +         .def_value_str = stringify(0),
> +         .help = "Block Size; min 1MB, max 256MB. " \
> +                 "0 means auto-calculate based on image size."
> +     },
> +     {
> +         .name = BLOCK_OPT_SUBFMT,
> +         .type = QEMU_OPT_STRING,
> +         .help = "VHDX format type, can be either 'dynamic' or 'fixed'. "\
> +                 "Default is 'dynamic'."
> +     },
> +     {
> +         .name = VHDX_BLOCK_OPT_ZERO,
> +         .type = QEMU_OPT_BOOL,
> +         .help = "Force use of payload blocks of type 'ZERO'.  Non-standard."
> +     },
> +     { NULL }
> +    }
>  };
>  
>  static BlockDriver bdrv_vhdx = {
> @@ -1913,11 +1908,11 @@ static BlockDriver bdrv_vhdx = {
>      .bdrv_reopen_prepare    = vhdx_reopen_prepare,
>      .bdrv_co_readv          = vhdx_co_readv,
>      .bdrv_co_writev         = vhdx_co_writev,
> -    .bdrv_create            = vhdx_create,
> +    .bdrv_create2           = vhdx_create,
>      .bdrv_get_info          = vhdx_get_info,
>      .bdrv_check             = vhdx_check,
>  
> -    .create_options         = vhdx_create_options,
> +    .create_opts            = &vhdx_create_opts,
>  };
>  
>  static void bdrv_vhdx_init(void)
> diff --git a/block/vhdx.h b/block/vhdx.h
> index 2acd7c2..a5a36fc 100644
> --- a/block/vhdx.h
> +++ b/block/vhdx.h
> @@ -23,6 +23,7 @@
>  #define GiB            (MiB * 1024)
>  #define TiB ((uint64_t) GiB * 1024)
>  
> +#define DEFAULT_LOG_SIZE 1048576 /* 1MiB */

Why not write it as (1 * MiB) like before instead of putting a comment
there?

Kevin