[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V10 1/4] add def_print_str and use it in qemu_op
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH V10 1/4] add def_print_str and use it in qemu_opts_print. |
Date: |
Tue, 15 Jan 2013 12:08:42 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
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 <address@hidden>
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