Re: [Qemu-devel] [PATCH v22 12/25] qcow2.c: replace QEMUOptionParameter with QemuOpts

[Stefan Hajnoczi]

On Mon, Mar 10, 2014 at 03:31:48PM +0800, Chunyan Liu wrote:
> @@ -1636,64 +1636,64 @@ static int qcow2_create(const char *filename, 
> QEMUOptionParameter *options,
>      int ret;
>  
>      /* Read out options */
> -    while (options && options->name) {
> -        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> -            sectors = options->value.n / 512;
> -        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
> -            backing_file = options->value.s;
> -        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
> -            backing_fmt = options->value.s;
> -        } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) {
> -            flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0;
> -        } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) {
> -            if (options->value.n) {
> -                cluster_size = options->value.n;
> -            }
> -        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> -            if (!options->value.s || !strcmp(options->value.s, "off")) {
> -                prealloc = 0;
> -            } else if (!strcmp(options->value.s, "metadata")) {
> -                prealloc = 1;
> -            } else {
> -                error_setg(errp, "Invalid preallocation mode: '%s'",
> -                           options->value.s);
> -                return -EINVAL;
> -            }
> -        } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) {
> -            if (!options->value.s) {
> -                /* keep the default */
> -            } else if (!strcmp(options->value.s, "0.10")) {
> -                version = 2;
> -            } else if (!strcmp(options->value.s, "1.1")) {
> -                version = 3;
> -            } else {
> -                error_setg(errp, "Invalid compatibility level: '%s'",
> -                           options->value.s);
> -                return -EINVAL;
> -            }
> -        } else if (!strcmp(options->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
> -            flags |= options->value.n ? BLOCK_FLAG_LAZY_REFCOUNTS : 0;
> -        }
> -        options++;
> +    sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> +    backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> +    backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
> +    if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> +        flags |= BLOCK_FLAG_ENCRYPT;
> +    }
> +    cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
> +                                         DEFAULT_CLUSTER_SIZE);
> +    buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> +    if (!buf || !strcmp(buf, "off")) {
> +        prealloc = 0;
> +    } else if (!strcmp(buf, "metadata")) {
> +        prealloc = 1;
> +    } else {
> +        fprintf(stderr, "Invalid preallocation mode: '%s'\n", buf);

This should be error_setg(errp, "Invalid preallocation mode: '%s'",
options->value.s).

Please check the other patches too to ensure the error_setg() hasn't
been turned into fprintf().