On Thu, Apr 03, 2014 at 05:54:20PM +0800, Chunyan Liu wrote: > Add def_value_str (default value) to QemuOptDesc, to replace function of the > default value in QEMUOptionParameter. > > Improve qemu_opts_get_* functions: if find opt, return opt->str; otherwise, > if desc->def_value_str is set, return desc->def_value_str; otherwise, return > input defval. > > Improve qemu_opts_print: if option is set, print opt->str; otherwise, if > desc->def_value_str is set, also print it. > > Reviewed-by: Eric Blake <ebl...@redhat.com> > Signed-off-by: Dong Xu Wang <wdon...@linux.vnet.ibm.com> > Signed-off-by: Chunyan Liu <cy...@suse.com> > --- > include/qemu/option.h | 3 ++- > util/qemu-option.c | 56 > ++++++++++++++++++++++++++++++++++++++++++--------- > 2 files changed, 49 insertions(+), 10 deletions(-) > > diff --git a/include/qemu/option.h b/include/qemu/option.h > index 8c0ac34..c3b0a91 100644 > --- a/include/qemu/option.h > +++ b/include/qemu/option.h > @@ -99,6 +99,7 @@ typedef struct QemuOptDesc { > const char *name; > enum QemuOptType type; > const char *help; > + const char *def_value_str; > } QemuOptDesc;
I still think you don't need to define the default value as a string to later parse it, see what I mean https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg04273.html But I think we can live with it. See bellow, if you fix the issue you can add my Reviewed-by. > > struct QemuOptsList { > @@ -156,7 +157,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict); > void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp); > > typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque); > -int qemu_opts_print(QemuOpts *opts, void *dummy); > +void qemu_opts_print(QemuOpts *opts); > int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void > *opaque, > int abort_on_failure); > > diff --git a/util/qemu-option.c b/util/qemu-option.c > index e6d10bc..d5e80da 100644 > --- a/util/qemu-option.c > +++ b/util/qemu-option.c > @@ -570,6 +570,13 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char > *name) > const char *qemu_opt_get(QemuOpts *opts, const char *name) > { > QemuOpt *opt = qemu_opt_find(opts, name); > + > + if (!opt) { > + const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); > + if (desc && desc->def_value_str) { > + return desc->def_value_str; > + } > + } > return opt ? opt->str : NULL; > } > > @@ -589,8 +596,13 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, > bool defval) > { > QemuOpt *opt = qemu_opt_find(opts, name); > > - if (opt == NULL) > + if (opt == NULL) { > + const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); > + if (desc && desc->def_value_str) { > + parse_option_bool(name, desc->def_value_str, &defval, > &error_abort); > + } > return defval; > + } > assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL); > return opt->value.boolean; > } > @@ -599,8 +611,14 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char > *name, uint64_t defval) > { > QemuOpt *opt = qemu_opt_find(opts, name); > > - if (opt == NULL) > + if (opt == NULL) { > + const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); > + if (desc && desc->def_value_str) { > + parse_option_number(name, desc->def_value_str, &defval, > + &error_abort); > + } > return defval; > + } > assert(opt->desc && opt->desc->type == QEMU_OPT_NUMBER); > return opt->value.uint; > } > @@ -609,8 +627,13 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char > *name, uint64_t defval) > { > QemuOpt *opt = qemu_opt_find(opts, name); > > - if (opt == NULL) > + if (opt == NULL) { > + const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); > + if (desc && desc->def_value_str) { > + parse_option_size(name, desc->def_value_str, &defval, > &error_abort); > + } > return defval; > + } > assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE); > return opt->value.uint; > } > @@ -895,17 +918,32 @@ void qemu_opts_del(QemuOpts *opts) > g_free(opts); > } > > -int qemu_opts_print(QemuOpts *opts, void *dummy) > +void qemu_opts_print(QemuOpts *opts) > { > 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) { > + fprintf(stderr, "%s=\"%s\" ", opt->name, opt->str); > + } > + return; > + } > + for (; desc && desc->name; desc++) { > + const char *value; > + QemuOpt *opt = qemu_opt_find(opts, desc->name); > + > + value = opt ? opt->str : desc->def_value_str; > + if (!value) { > + continue; > + } > + if (desc->type == QEMU_OPT_STRING) { > + fprintf(stderr, "%s='%s' ", desc->name, value); > + } else { > + fprintf(stderr, "%s=%s ", desc->name, value); > + } > } > fprintf(stderr, "\n"); I see you removed this fprintf() in the commit afe65f5c92694d551aaebc128233b623e8665d5e Please do this change in this commit and avoid breaking the block tests in further commits. -- Leandro Dorileo > - return 0; > } > > static int opts_do_parse(QemuOpts *opts, const char *params, > -- > 1.7.12.4 >