δΊ 2013-1-25 2:26, Markus Armbruster ει: > Dong Xu Wang <wdon...@vnet.linux.ibm.com> writes: > >> 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 <wdon...@vnet.linux.ibm.com> >> --- >> v10->v11: >> 1) print all values that have actually been assigned while accept-any >> cases. >> >> 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 + >> util/qemu-option.c | 30 +++++++++++++++++++++++++----- >> 2 files changed, 26 insertions(+), 5 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/util/qemu-option.c b/util/qemu-option.c >> index f532b76..1aed418 100644 >> --- a/util/qemu-option.c >> +++ b/util/qemu-option.c >> @@ -863,13 +863,33 @@ 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) { >> + QTAILQ_FOREACH(opt, &opts->head, next) { >> + printf("%s=\"%s\" ", opt->name, opt->str); >> + } >> + 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); >> + } >> } >> - fprintf(stderr, "\n"); >> return 0; >> } > > I dislike def_value_str intensely, because it's a made up string that > needn't be related to the actual default value. Which is supplied by > the code getting the option. > > If I remember correctly, you want qemu_opts_print() print the default > value, because you don't want to change output of qemu-img. Fair > enough. > > Two sane ways to do that: > > 1. Make the caller to supply the default value. Just like the caller > supplies it when getting option values. > > Easiest way is to make it set the defaults in the opts whenever it > supplies a default. Like this: > > mumble = qemu_opt_get(opts, "mumble"); > if (!mumble) { > mumble = "mutter"; > qemu_opt_set(opts, "mumble", mumble); > } > > Problem: this doesn't set the value in the existing QemuOpt, it > appends a new one, which isn't what we want. We'd need a new > function to change the value. > > 2. Actually use def_value_str as default value! Replacing > > if (value) { > opt->str = g_strdup(value); > } > > in opt_set() by > > opt->str = g_strdup(value ?: def_value_str); > > could do. Looks easier than 1. > >
Okay, will use 2 in new version patches.