From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52200) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXKtC-0000yu-3t for qemu-devel@nongnu.org; Mon, 07 Apr 2014 21:31:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WXKt6-0000l4-EN for qemu-devel@nongnu.org; Mon, 07 Apr 2014 21:31:50 -0400 Received: from mail-yh0-f50.google.com ([209.85.213.50]:48326) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXKt6-0000l0-8x for qemu-devel@nongnu.org; Mon, 07 Apr 2014 21:31:44 -0400 Received: by mail-yh0-f50.google.com with SMTP id c41so246324yho.23 for ; Mon, 07 Apr 2014 18:31:43 -0700 (PDT) Date: Mon, 7 Apr 2014 22:31:49 -0300 From: Leandro Dorileo Message-ID: <20140408013149.GA11200@dorilex-lnv.MultilaserAP> References: <1396518889-21681-1-git-send-email-cyliu@suse.com> <1396518889-21681-3-git-send-email-cyliu@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1396518889-21681-3-git-send-email-cyliu@suse.com> Subject: Re: [Qemu-devel] [PATCH v24 02/31] QemuOpts: add def_value_str to QemuOptDesc List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chunyan Liu Cc: qemu-devel@nongnu.org, stefanha@redhat.com 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 > Signed-off-by: Dong Xu Wang > Signed-off-by: Chunyan Liu > --- > 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 : ""); > - 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 >