| Message ID | 1385060754-18821-4-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2013年11月22日 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 = 42 > opt2 = 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=reftable_load,\ > inject-error.1.event=l2_load,\ > set-state.event=l1_update > > This would correspond to the following config file: > > [inject-error "inject-error.0"] > event = reftable_load > > [inject-error "inject-error.1"] > event = l2_load > > [set-state] > event = l1_update > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > 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 <stdio.h> > #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 multiple > + 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 *opts, > + Error **errp) > +{ > + QemuOpts *subopts, *subopts_i; > + QDict *subqdict, *subqdict_i = NULL; > + char *prefix = g_strdup_printf("%s.", opts->name); > + Error *local_err = NULL; > + size_t orig_size, enum_size; > + > + qdict_extract_subqdict(options, &subqdict, prefix); > + orig_size = qdict_size(subqdict); > + if (!orig_size) { > + goto out; > + } > + > + subopts = 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 = 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 = 0; i < INT_MAX; i++) { > + char i_prefix[32], opt_name[48]; > + size_t snprintf_ret; > + > + snprintf_ret = snprintf(i_prefix, 32, "%i.", i); > + assert(snprintf_ret < 32); > + > + snprintf_ret = snprintf(opt_name, 48, "%s.%i", opts->name, i); > + assert(snprintf_ret < 48); Is there a length limit of opts->name? I.e. is this an error case rather than an assertion case? > + > + qdict_extract_subqdict(subqdict, &subqdict_i, i_prefix); > + if (!qdict_size(subqdict_i)) { > + break; > + } > + > + subopts_i = 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->name, i); s/rule/section/ Or just remove this hunk? > + goto out; > + } > + */ > + > + QDECREF(subqdict_i); > + subqdict_i = 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 = NULL; > + > + for (i = 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 in the command line should be a general logic, which would also be very useful for Quorum driver. Fam
On 22.11.2013 09:08, Fam Zheng wrote: > On 2013年11月22日 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 = 42 >> opt2 = 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=reftable_load,\ >> inject-error.1.event=l2_load,\ >> set-state.event=l1_update >> >> This would correspond to the following config file: >> >> [inject-error "inject-error.0"] >> event = reftable_load >> >> [inject-error "inject-error.1"] >> event = l2_load >> >> [set-state] >> event = l1_update >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> 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 <stdio.h> >> #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 >> multiple >> + 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 >> *opts, >> + Error **errp) >> +{ >> + QemuOpts *subopts, *subopts_i; >> + QDict *subqdict, *subqdict_i = NULL; >> + char *prefix = g_strdup_printf("%s.", opts->name); >> + Error *local_err = NULL; >> + size_t orig_size, enum_size; >> + >> + qdict_extract_subqdict(options, &subqdict, prefix); >> + orig_size = qdict_size(subqdict); >> + if (!orig_size) { >> + goto out; >> + } >> + >> + subopts = 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 = 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 = 0; i < INT_MAX; i++) { >> + char i_prefix[32], opt_name[48]; >> + size_t snprintf_ret; >> + >> + snprintf_ret = snprintf(i_prefix, 32, "%i.", i); >> + assert(snprintf_ret < 32); >> + >> + snprintf_ret = snprintf(opt_name, 48, "%s.%i", >> opts->name, i); >> + assert(snprintf_ret < 48); > > Is there a length limit of opts->name? I.e. is this an error case > rather than an assertion case? Hm, yes, I had most of the code directly in block/blkdebug.c at first, that's why there are some issues here which work fine just for blkdebug, but are not that good in the general case. I'll use g_strdup_sprintf instead. > >> + >> + qdict_extract_subqdict(subqdict, &subqdict_i, i_prefix); >> + if (!qdict_size(subqdict_i)) { >> + break; >> + } >> + >> + subopts_i = qemu_opts_create(opts, opt_name, 1, NULL); > > Could pass in errp and skip error_setg below. Oh, right, missed that, thanks. > >> + 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/ Yes, this one also is fine for blkdebug, but not so much for the general case. > >> + 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->name, i); > > s/rule/section/ > Or just remove this hunk? Hm, I probably should have completely reviewed my own code before sending it. Will do that in v2, promise. ;-) > >> + goto out; >> + } >> + */ >> + >> + QDECREF(subqdict_i); >> + subqdict_i = 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 = NULL; >> + >> + for (i = 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 > in the command line should be a general logic, which would also be > very useful for Quorum driver. Yes, I thought about quorum, too. I don't think just using QemuOpts there would be too bad, since the above function could do all the work there as well. I could probably introduce a QDict function which splits a QDict with keys enumerated in this way into a QList of QDicts. I'll have a look into how much more complex that will be and, depending on that, include it in v2 or not. Thanks for reviewing, Max
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 <stdio.h> #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 multiple + 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 *opts, + Error **errp) +{ + QemuOpts *subopts, *subopts_i; + QDict *subqdict, *subqdict_i = NULL; + char *prefix = g_strdup_printf("%s.", opts->name); + Error *local_err = NULL; + size_t orig_size, enum_size; + + qdict_extract_subqdict(options, &subqdict, prefix); + orig_size = qdict_size(subqdict); + if (!orig_size) { + goto out; + } + + subopts = 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 = 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 = 0; i < INT_MAX; i++) { + char i_prefix[32], opt_name[48]; + size_t snprintf_ret; + + snprintf_ret = snprintf(i_prefix, 32, "%i.", i); + assert(snprintf_ret < 32); + + snprintf_ret = snprintf(opt_name, 48, "%s.%i", opts->name, i); + assert(snprintf_ret < 48); + + qdict_extract_subqdict(subqdict, &subqdict_i, i_prefix); + if (!qdict_size(subqdict_i)) { + break; + } + + subopts_i = qemu_opts_create(opts, opt_name, 1, NULL); + 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'", + 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->name, i); + goto out; + } + */ + + QDECREF(subqdict_i); + subqdict_i = 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 = NULL; + + for (i = 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 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 = 42 opt2 = 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=reftable_load,\ inject-error.1.event=l2_load,\ set-state.event=l1_update This would correspond to the following config file: [inject-error "inject-error.0"] event = reftable_load [inject-error "inject-error.1"] event = l2_load [set-state] event = l1_update Signed-off-by: Max Reitz <mreitz@redhat.com> --- include/qemu/config-file.h | 6 +++ util/qemu-config.c | 111 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+)