Re: [Qemu-devel] [PATCH 3/6] qemu-option: Add qemu_config_parse_qdict()

[Fam Zheng <famz@redhat.com>]

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