"Daniel P. Berrange" <berra...@redhat.com> writes:
> When converting QemuOpts to a QObject, there is no information
> about compound types available,
Yes, that's a drawback of splitting the conversion into a QemuOpts ->
QObject part that is oblivious of types, and a QObject -> QAPI object
part that knows the types.
> so when visiting a list, the
> corresponding QObject is not guaranteed to be a QList. We
> therefore need to be able to auto-create a single element QList
> from whatever type we find.
>
> This mode should only be enabled if you have compatibility
> requirements for
>
> -arg foo=hello,foo=world
>
> to be treated as equivalent to the preferred syntax:
>
> -arg foo.0=hello,foo.1=world
Not sure this is "preferred". "More powerfully warty" is probably
closer to the truth ;)
How is "-arg foo=hello,foo=world" treated if this mode isn't enabled?
What would be the drawbacks of doing this always instead of only when we
"have compatibility requirements"?
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> ---
> include/qapi/qobject-input-visitor.h | 20 +++++++-
> qapi/qobject-input-visitor.c | 27 +++++++++--
> tests/test-qobject-input-visitor.c | 88
> +++++++++++++++++++++++++++++++-----
> 3 files changed, 117 insertions(+), 18 deletions(-)
>
> diff --git a/include/qapi/qobject-input-visitor.h
> b/include/qapi/qobject-input-visitor.h
> index f134d90..1809f48 100644
> --- a/include/qapi/qobject-input-visitor.h
> +++ b/include/qapi/qobject-input-visitor.h
> @@ -42,6 +42,19 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool
> strict);
> * represented as strings. i.e. if visiting a boolean, the value should
> * be a QString whose contents represent a valid boolean.
> *
> + * If @autocreate_list is true, then as an alternative to a normal QList,
> + * list values can be stored as a QString or QDict instead, which will
> + * be interpreted as representing single element lists. This should only
> + * by used if compatibility is required with the OptsVisitor which allowed
> + * repeated keys, without list indexes, to represent lists. e.g. set this
> + * to true if you have compatibility requirements for
> + *
> + * -arg foo=hello,foo=world
> + *
> + * to be treated as equivalent to the preferred syntax:
> + *
> + * -arg foo.0=hello,foo.1=world
> + *
> * The visitor always operates in strict mode, requiring all dict keys
> * to be consumed during visitation. An error will be reported if this
> * does not happen.
> @@ -49,7 +62,8 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool
> strict);
> * The returned input visitor should be released by calling
> * visit_free() when no longer required.
> */
> -Visitor *qobject_input_visitor_new_autocast(QObject *obj);
> +Visitor *qobject_input_visitor_new_autocast(QObject *obj,
> + bool autocreate_list);
>
>
> /**
> @@ -64,6 +78,8 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj);
> * 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);
> +Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts,
> + bool autocreate_list,
> + Error **errp);
>
> #endif
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index d9269c9..d88e9f9 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -48,6 +48,10 @@ struct QObjectInputVisitor
>
> /* True to reject parse in visit_end_struct() if unvisited keys remain.
> */
> bool strict;
> +
> + /* Whether we can auto-create single element lists when
> + * encountering a non-QList type */
> + bool autocreate_list;
> };
>
> static QObjectInputVisitor *to_qiv(Visitor *v)
> @@ -108,6 +112,7 @@ static const QListEntry
> *qobject_input_push(QObjectInputVisitor *qiv,
> assert(obj);
> tos->obj = obj;
> tos->qapi = qapi;
> + qobject_incref(obj);
>
> if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) {
> h = g_hash_table_new(g_str_hash, g_str_equal);
> @@ -147,6 +152,7 @@ static void qobject_input_stack_object_free(StackObject
> *tos)
> if (tos->h) {
> g_hash_table_unref(tos->h);
> }
> + qobject_decref(tos->obj);
>
> g_free(tos);
> }
Can you explain the reference counting change?
> @@ -197,7 +203,7 @@ static void qobject_input_start_list(Visitor *v, const
> char *name,
> QObject *qobj = qobject_input_get_object(qiv, name, true);
> const QListEntry *entry;
>
> - if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
> + if (!qobj || (!qiv->autocreate_list && qobject_type(qobj) !=
> QTYPE_QLIST)) {
Long line, but I believe it'll go away when you rebase for commit
1382d4a.
> if (list) {
> *list = NULL;
> }
> @@ -206,7 +212,16 @@ static void qobject_input_start_list(Visitor *v, const
> char *name,
> return;
> }
>
> - entry = qobject_input_push(qiv, qobj, list, errp);
> + if (qobject_type(qobj) != QTYPE_QLIST) {
> + QList *tmplist = qlist_new();
> + qlist_append_obj(tmplist, qobj);
> + qobject_incref(qobj);
> + entry = qobject_input_push(qiv, QOBJECT(tmplist), list, errp);
> + QDECREF(tmplist);
> + } else {
> + entry = qobject_input_push(qiv, qobj, list, errp);
> + }
> +
> if (list) {
> if (entry) {
> *list = g_malloc0(size);
Buries autolist behavior in the middle of things. What about doing it
first, so it's more separate?
QObjectInputVisitor *qiv = to_qiv(v);
QObject *qobj = qobject_input_get_object_(qiv, name, true, errp);
const QListEntry *entry;
if (!qobj) {
return;
}
+ if (qiv->autocreate_list && qobject_type(qobj) != QTYPE_QLIST) {
+ QList *auto_list = qlist_new();
+ qlist_append_obj(auto_list, qobj);
+ qobj = auto_list;
+ }
+
if (qobject_type(qobj) != QTYPE_QLIST) {
I ignored reference counting here, because I don't yet understand how
and why you're changing it.
> @@ -514,7 +529,8 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool
> strict)
> return &v->visitor;
> }
>
> -Visitor *qobject_input_visitor_new_autocast(QObject *obj)
> +Visitor *qobject_input_visitor_new_autocast(QObject *obj,
> + bool autocreate_list)
> {
> QObjectInputVisitor *v;
>
> @@ -539,6 +555,7 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj)
> v->visitor.optional = qobject_input_optional;
> v->visitor.free = qobject_input_free;
> v->strict = true;
> + v->autocreate_list = autocreate_list;
>
> v->root = obj;
> qobject_incref(obj);
> @@ -548,6 +565,7 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj)
>
>
> Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts,
> + bool autocreate_list,
> Error **errp)
> {
> QDict *pdict;
> @@ -564,7 +582,8 @@ Visitor *qobject_input_visitor_new_opts(const QemuOpts
> *opts,
> goto cleanup;
> }
>
> - v = qobject_input_visitor_new_autocast(pobj);
> + v = qobject_input_visitor_new_autocast(pobj,
> + autocreate_list);
> cleanup:
> qobject_decref(pobj);
> QDECREF(pdict);
[Skipping test updates for now...]
