Re: [RFC PATCH 02/12] qom: Create object_configure()

[Markus Armbruster <armbru@redhat.com>]

Kevin Wolf <kwolf@redhat.com> writes:

> This renames object_set_properties_from_qdict() to object_configure()
> and removes the QDict parameter from it: With visit_next_struct_member()
> it can set all properties without looking at the keys of the QDict.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qom/object_interfaces.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 3b61c195c5..f9f5608194 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -42,16 +42,15 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
>      }
>  }
>  
> -static void object_set_properties_from_qdict(Object *obj, const QDict *qdict,
> -                                             Visitor *v, Error **errp)
> +static void object_configure(Object *obj, Visitor *v, Error **errp)
>  {
> -    const QDictEntry *e;
> +    const char *key;
>  
>      if (!visit_start_struct(v, NULL, NULL, 0, errp)) {
>          return;
>      }
> -    for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
> -        if (!object_property_set(obj, e->key, v, errp)) {
> +    while ((key = visit_next_struct_member(v))) {
> +        if (!object_property_set(obj, key, v, errp)) {
>              goto out;
>          }
>      }
> @@ -69,7 +68,7 @@ void object_set_properties_from_keyval(Object *obj, const QDict *qdict,
>      } else {
>          v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
>      }
> -    object_set_properties_from_qdict(obj, qdict, v, errp);
> +    object_configure(obj, v, errp);
>      visit_free(v);
>  }
>  
> @@ -108,7 +107,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
>  
>      assert(qdict);

This is the only remaining use of parameter @qdict.  Let's drop it.

>      obj = object_new(type);
> -    object_set_properties_from_qdict(obj, qdict, v, &local_err);
> +    object_configure(obj, v, &local_err);
>      if (local_err) {
>          goto out;
>      }

Brief recap how configuration data flows trough QMP object-add to QOM:

* QMP object-add is fully typed: union ObjectOptions.  QMP core parses
  input via QDict to ObjectOptions (JSON parser + QObject input
  visitor), and passes that to qmp_object_add().

* qmp_object_add() passes it on to user_creatable_add_qapi().

  Aside: getting rid of the wrapper would be as easy as renaming
  user_creatable_add_qapi() to qmp_object_add().

* user_creatable_add_qapi() converts right back to QDict (QObject output
  visitor).  It extracts the non-properties "qom-type" and "id", and
  passes the properties (wrapped in a QObject input visitor) to
  user_creatable_add_type().

* user_creatable_add_type() feeds the properties to object_configure(),
  formerly object_set_properties_from_qdict().

Your patch simplifies one detail of the last step.  Small
simplifications are welcome, too.

The visitor ping-pong remains: input -> output -> input.

We play visitor ping-pong because we reduce the problem "for each member
of ObjectOptions: set property" to the solved problem "set properties
for an input visitor".

Straight solution of the problem: a QOM property output visitor.

Observation, not demand.