From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35509) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vjlmx-00031h-3P for qemu-devel@nongnu.org; Fri, 22 Nov 2013 03:08:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vjlmr-0000Cn-1d for qemu-devel@nongnu.org; Fri, 22 Nov 2013 03:08:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:1853) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vjlmq-0000Cf-PK for qemu-devel@nongnu.org; Fri, 22 Nov 2013 03:08:24 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rAM88NF3022922 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 22 Nov 2013 03:08:24 -0500 Message-ID: <528F10F1.5090300@redhat.com> Date: Fri, 22 Nov 2013 16:08:17 +0800 From: Fam Zheng MIME-Version: 1.0 References: <1385060754-18821-1-git-send-email-mreitz@redhat.com> <1385060754-18821-4-git-send-email-mreitz@redhat.com> In-Reply-To: <1385060754-18821-4-git-send-email-mreitz@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/6] qemu-option: Add qemu_config_parse_qdict() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi On 2013=E5=B9=B411=E6=9C=8822=E6=97=A5 03:05, Max Reitz wrote: > This function basically parses command-line options given as a QDict > replacing a config file. > > For instance, the QDict {"section.opt1": 42, "section.opt2": 23} > corresponds to the config file: > > [section] > opt1 =3D 42 > opt2 =3D 23 > > It is possible to specify multiple sections and also multiple sections > of the same type. On the command line, this looks like the following: > > inject-error.0.event=3Dreftable_load,\ > inject-error.1.event=3Dl2_load,\ > set-state.event=3Dl1_update > > This would correspond to the following config file: > > [inject-error "inject-error.0"] > event =3D reftable_load > > [inject-error "inject-error.1"] > event =3D l2_load > > [set-state] > event =3D l1_update > > Signed-off-by: Max Reitz > --- > include/qemu/config-file.h | 6 +++ > util/qemu-config.c | 111 ++++++++++++++++++++++++++++++++++++= +++++++++ > 2 files changed, 117 insertions(+) > > diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h > index 508428f..dbd97c4 100644 > --- a/include/qemu/config-file.h > +++ b/include/qemu/config-file.h > @@ -4,6 +4,7 @@ > #include > #include "qemu/option.h" > #include "qapi/error.h" > +#include "qapi/qmp/qdict.h" > > QemuOptsList *qemu_find_opts(const char *group); > QemuOptsList *qemu_find_opts_err(const char *group, Error **errp); > @@ -18,6 +19,11 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists= , const char *fname); > > int qemu_read_config_file(const char *filename); > > +/* Parse QDict options as a replacement for a config file (allowing mu= ltiple > + enumerated (0..(n-1)) configuration "sections") */ > +void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists, > + Error **errp); > + > /* Read default QEMU config files > */ > int qemu_read_default_config_files(bool userconfig); > diff --git a/util/qemu-config.c b/util/qemu-config.c > index 04da942..80d82d4 100644 > --- a/util/qemu-config.c > +++ b/util/qemu-config.c > @@ -356,3 +356,114 @@ int qemu_read_config_file(const char *filename) > return -EINVAL; > } > } > + > +static void config_parse_qdict_section(QDict *options, QemuOptsList *o= pts, > + Error **errp) > +{ > + QemuOpts *subopts, *subopts_i; > + QDict *subqdict, *subqdict_i =3D NULL; > + char *prefix =3D g_strdup_printf("%s.", opts->name); > + Error *local_err =3D NULL; > + size_t orig_size, enum_size; > + > + qdict_extract_subqdict(options, &subqdict, prefix); > + orig_size =3D qdict_size(subqdict); > + if (!orig_size) { > + goto out; > + } > + > + subopts =3D qemu_opts_create_nofail(opts); > + qemu_opts_absorb_qdict(subopts, subqdict, &local_err); > + if (error_is_set(&local_err)) { > + error_propagate(errp, local_err); > + goto out; > + } > + > + enum_size =3D qdict_size(subqdict); > + if (enum_size < orig_size && enum_size) { > + error_setg(errp, "Unknown option '%s' for '%s'", > + qdict_first(subqdict)->key, opts->name); > + goto out; > + } > + > + if (enum_size) { > + /* Multiple, enumerated rules */ > + int i; > + > + /* Not required anymore */ > + qemu_opts_del(subopts); > + > + for (i =3D 0; i < INT_MAX; i++) { > + char i_prefix[32], opt_name[48]; > + size_t snprintf_ret; > + > + snprintf_ret =3D snprintf(i_prefix, 32, "%i.", i); > + assert(snprintf_ret < 32); > + > + snprintf_ret =3D snprintf(opt_name, 48, "%s.%i", opts->nam= e, i); > + assert(snprintf_ret < 48); Is there a length limit of opts->name? I.e. is this an error case rather=20 than an assertion case? > + > + qdict_extract_subqdict(subqdict, &subqdict_i, i_prefix); > + if (!qdict_size(subqdict_i)) { > + break; > + } > + > + subopts_i =3D qemu_opts_create(opts, opt_name, 1, NULL); Could pass in errp and skip error_setg below. > + if (!subopts_i) { > + error_setg(errp, "Option ID '%s' already in use", opt_= name); > + goto out; > + } > + > + qemu_opts_absorb_qdict(subopts_i, subqdict_i, &local_err); > + if (error_is_set(&local_err)) { > + error_propagate(errp, local_err); > + qemu_opts_del(subopts_i); > + goto out; > + } > + > + if (qdict_size(subqdict_i)) { > + error_setg(errp, "[%s] rules don't support the option = '%s'", s/rules don't/section doesn't/ > + opts->name, qdict_first(subqdict_i)->key); > + qemu_opts_del(subopts_i); > + goto out; > + } > + > + /* > + if (add_rule(subopts_i, (void *)ard) < 0) { > + error_setg(errp, "Could not add [%s] rule %i", opts->n= ame, i); s/rule/section/ Or just remove this hunk? > + goto out; > + } > + */ > + > + QDECREF(subqdict_i); > + subqdict_i =3D NULL; > + } > + > + if (qdict_size(subqdict)) { > + error_setg(errp, "Unused option '%s' for [%s]", > + qdict_first(subqdict)->key, opts->name); > + goto out; > + } > + } > + > +out: > + QDECREF(subqdict); > + QDECREF(subqdict_i); > + > + free(prefix); > +} > + > +void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists, > + Error **errp) > +{ > + int i; > + Error *local_err =3D NULL; > + > + for (i =3D 0; lists[i]; i++) { > + config_parse_qdict_section(options, lists[i], &local_err); > + if (error_is_set(&local_err)) { > + error_propagate(errp, local_err); > + return; > + } > + } > +} > This does the work, but also seems ad-hoc. Providing an array of dicts=20 in the command line should be a general logic, which would also be very=20 useful for Quorum driver. Fam