Re: [Qemu-devel] [PATCH v20 18/26] sheepdog.c: replace QEMUOptionParameter with QemuOpts

[Eric Blake]

On 02/11/2014 11:33 PM, Chunyan Liu wrote:
> sheepdog.c: replace QEMUOptionParameter with QemuOpts
> 
> Signed-off-by: Dong Xu Wang <address@hidden>
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
>  block/sheepdog.c |  101 
> +++++++++++++++++++++++++-----------------------------
>  1 files changed, 47 insertions(+), 54 deletions(-)
> 

> +
> +    buf = NULL;
> +    buf = qemu_opt_get_del(opts, BLOCK_OPT_REDUNDANCY);

Dead NULL assignment.

> +    if (buf) {
> +        ret = parse_redundancy(s, buf);
> +        if (ret < 0) {
> +            goto out;
>          }

Do you need to store into ret here, or is it sufficient to use 'if
(parse_redundancy(s, buf) < 0) {'

> -    {
> -        .name = BLOCK_OPT_REDUNDANCY,
> -        .type = OPT_STRING,
> -        .help = "Redundancy of the image"
> -    },
> -    { NULL }
> +static QemuOptsList sd_create_opts = {
> +    .name = "sheepdog-create-opts",
> +    .head = QTAILQ_HEAD_INITIALIZER(sd_create_opts.head),
> +    .desc = {
> +        {

> +            .name = BLOCK_OPT_PREALLOC,
> +            .type = QEMU_OPT_STRING,
> +            .help = "Preallocation mode (allowed values: off, full)"
> +        },
> +        { /* end of list */ }

Missing redundancy in the new list.

> @@ -2538,7 +2533,7 @@ static BlockDriver bdrv_sheepdog = {
>      .bdrv_save_vmstate  = sd_save_vmstate,
>      .bdrv_load_vmstate  = sd_load_vmstate,
>  
> -    .create_options = sd_create_options,
> +    .create_opts   = &sd_create_opts,

Another example of the inconsistent use of &.

>  };
>  
>  static BlockDriver bdrv_sheepdog_tcp = {
> @@ -2548,7 +2543,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
>      .bdrv_needs_filename = true,
>      .bdrv_file_open = sd_open,
>      .bdrv_close     = sd_close,
> -    .bdrv_create    = sd_create,
> +    .bdrv_create2   = sd_create,
>      .bdrv_has_zero_init = bdrv_has_zero_init_1,
>      .bdrv_getlength = sd_getlength,
>      .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
> @@ -2568,7 +2563,6 @@ static BlockDriver bdrv_sheepdog_tcp = {
>      .bdrv_save_vmstate  = sd_save_vmstate,
>      .bdrv_load_vmstate  = sd_load_vmstate,
>  
> -    .create_options = sd_create_options,
>  };

Why no .create_opts replacement?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature