Re: [Qemu-devel] [PATCH v14 08/21] qapi: allow QObjectInputVisitor to be created with QemuOpts

[Markus Armbruster <armbru@redhat.com>]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> Instead of requiring all callers to go through the mutli-step

multi-step

> process of turning QemuOpts into a suitable QObject for visiting,
> add a new constructor that encapsulates this logic. This will
> allow QObjectInputVisitor to be a drop-in replacement for the
> existing OptsVisitor with minimal code changes for callers.
>
> NB, at this point it is only supporting opts syntax which
> explicitly matches the QAPI schema structure, so is not yet
> a true drop-in replacement for OptsVisitor. The patches that
> follow will add the special cases requird for full backwards
> compatibility with OptsVisitor.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  include/qapi/qobject-input-visitor.h | 15 +++++++++++++++
>  include/qemu/option.h                |  2 +-
>  qapi/qobject-input-visitor.c         | 25 +++++++++++++++++++++++++
>  util/qemu-option.c                   |  2 +-
>  4 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h
> index 5022297..f134d90 100644
> --- a/include/qapi/qobject-input-visitor.h
> +++ b/include/qapi/qobject-input-visitor.h
> @@ -51,4 +51,19 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
>   */
>  Visitor *qobject_input_visitor_new_autocast(QObject *obj);
>  
> +
> +/**
> + * Create a new input visitor that converts @opts to a QAPI object.
> + *
> + * The QemuOpts will be converted into a QObject using the
> + * qdict_crumple() method to automatically create structs
> + * and lists. The resulting QDict will then be passed to the
> + * qobject_input_visitor_new_autocast() method. See the docs
> + * of that method for further details on processing behaviour.
> + *
> + * The returned input visitor should be released by calling
> + * visit_free() when no longer required.
> + */
> +Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts, Error **errp);
> +
>  #endif
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index 2a5266f..29f3f18 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -125,7 +125,7 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
>                              int permit_abbrev);
>  QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
>                                 Error **errp);
> -QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
> +QDict *qemu_opts_to_qdict(const QemuOpts *opts, QDict *qdict);
>  void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp);
>  
>  typedef int (*qemu_opts_loopfunc)(void *opaque, QemuOpts *opts, Error **errp);
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index cf41df6..d9269c9 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -545,3 +545,28 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj)
>  
>      return &v->visitor;
>  }
> +
> +
> +Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts,
> +                                        Error **errp)
> +{
> +    QDict *pdict;
> +    QObject *pobj = NULL;

@pdict and @pobj are unusual names.  Let's stick to the more common
@dict and @obj.

> +    Visitor *v = NULL;
> +
> +    pdict = qemu_opts_to_qdict(opts, NULL);
> +    if (!pdict) {
> +        goto cleanup;
> +    }
> +
> +    pobj = qdict_crumple(pdict, true, errp);
> +    if (!pobj) {
> +        goto cleanup;
> +    }
> +
> +    v = qobject_input_visitor_new_autocast(pobj);
> + cleanup:
> +    qobject_decref(pobj);
> +    QDECREF(pdict);
> +    return v;
> +}
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 41b356c..418cbb6 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -1058,7 +1058,7 @@ void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp)
>   * TODO We'll want to use types appropriate for opt->desc->type, but
>   * this is enough for now.

Implementing this TODO could break users that pass it to
qobject_input_visitor_new_autocast(), because despite its name, the
_autocast visitor expects *only* string scalars.

I guess (but have not checked) that these users have null opt->desc
since their opts->list->desc[] is empty.

We should either drop the TODO (needs review of the other users), or
update it to reflect the new situation.  Perhaps users of
qobject_input_visitor_new_autocast() should additionally assert
opts->list->desc[] is empty.

>   */
> -QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
> +QDict *qemu_opts_to_qdict(const QemuOpts *opts, QDict *qdict)
>  {
>      QemuOpt *opt;
>      QObject *val;