[V10,1/4] add def_print_str and use it in qemu_opts_print.

Submitted by Robert Wang on Jan. 7, 2013, 5:26 a.m.

Details

Message ID 1357536383-7299-2-git-send-email-wdongxu@linux.vnet.ibm.com
State New
Headers show

Commit Message

Robert Wang Jan. 7, 2013, 5:26 a.m.
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>
---
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.

 include/qemu/option.h |    1 +
 qemu-option.c         |   31 ++++++++++++++++++++++++-------
 2 files changed, 25 insertions(+), 7 deletions(-)

Comments

Kevin Wolf Jan. 15, 2013, 11:08 a.m.
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

Patch hide | download patch | download mbox

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;
     }
-    fprintf(stderr, "\n");
-    return 0;
+    for (; desc && desc->name; desc++) {
+        const char *value = desc->def_value_str;
+        QemuOpt *opt;
+
+        opt = qemu_opt_find(opts, desc->name);
+        if (opt) {
+            value = opt->str;
+        }
+
+        if (!value) {
+            continue;
+        }
+
+        if (desc->type == QEMU_OPT_STRING) {
+            printf("%s='%s' ", desc->name, value);
+        } else {
+            printf("%s=%s ", desc->name, value);
+        }
+    }
+     return 0;
 }
 
 static int opts_do_parse(QemuOpts *opts, const char *params,