Re: [Qemu-devel] [PATCH V10 1/4] add def_print_str and use it in qemu_opts_print.

[Kevin Wolf <kwolf@redhat.com>]

Am 07.01.2013 06:26, schrieb Dong Xu Wang:
> qemu_opts_print has no user now, so can re-write the function safely.
> 
> qemu_opts_print will be used while using "qemu-img create", it will
> produce the same output as previous code.
> 
> The behavior of this function has changed:
> 
> 1. Print every possible option, whether a value has been set or not.
> 2. Option descriptors may provide a default value.
> 3. Print to stdout instead of stderr.
> 
> Previously the behavior was to print every option that has been set.
> Options that have not been set would be skipped.
> 
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>

The subject line hasn't been updated when you changed the naem of
"def_print_str".

> ---
> v7->v8:
> 1) print "elements => accept any params" while opts_accepts_any() ==
> true.
> 2) since def_print_str is the default value if an option isn't set,
> so rename it to def_value_str.

It's not really, it still only influences printing, not what values you
get. The idea was that you say, for example:

        {
            .name = BLOCK_OPT_CLUSTER_SIZE,
            .type = QEMU_OPT_SIZE,
            .help = "qcow2 cluster size",
            .def_value_str = stringify(DEFAULT_CLUSTER_SIZE)
        },

And then this would actually be enough for the default to be applied.
That is, you would only write

        cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE);

instead of passing the default a second time like you do now:

        cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
                                         DEFAULT_CLUSTER_SIZE);

We can however add patches to make it a real default value on top of
this series, so it doesn't necessarily mean that we can't commit it as
it is.

> 
>  include/qemu/option.h |    1 +
>  qemu-option.c         |   31 ++++++++++++++++++++++++-------
>  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index ba197cd..394170a 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -96,6 +96,7 @@ typedef struct QemuOptDesc {
>      const char *name;
>      enum QemuOptType type;
>      const char *help;
> +    const char *def_value_str;
>  } QemuOptDesc;
>  
>  struct QemuOptsList {
> diff --git a/qemu-option.c b/qemu-option.c
> index f532b76..6f19fd3 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -862,15 +862,32 @@ void qemu_opts_del(QemuOpts *opts)
>  
>  int qemu_opts_print(QemuOpts *opts, void *dummy)
>  {
> -    QemuOpt *opt;
> +    QemuOptDesc *desc = opts->list->desc;
>  
> -    fprintf(stderr, "%s: %s:", opts->list->name,
> -            opts->id ? opts->id : "<noid>");
> -    QTAILQ_FOREACH(opt, &opts->head, next) {
> -        fprintf(stderr, " %s=\"%s\"", opt->name, opt->str);
> +    if (desc[0].name == NULL) {
> +        printf("no elements => accept any params");
> +        return 0;
>      }

Wouldn't it make more sense to keep the old behaviour for accept-any
cases? That is, print all values that have actually been assigned?

Kevin