Re: [PATCH v2 19/31] qapi/qom: QAPIfy object-add

[Eric Blake]

On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This converts object-add from 'gen': false to the ObjectOptions QAPI
> type. As an immediate benefit, clients can now use QAPI schema
> introspection for user creatable QOM objects.
> 
> It is also the first step towards making the QAPI schema the only
> external interface for the creation of user creatable objects. Once all
> other places (HMP and command lines of the system emulator and all
> tools) go through QAPI, too, some object implementations can be
> simplified because some checks (e.g. that mandatory options are set) are
> already performed by QAPI, and in another step, QOM boilerplate code
> could be generated from the schema.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/qom.json                        | 11 +----------
>  include/qom/object_interfaces.h      |  7 -------
>  hw/block/xen-block.c                 | 16 ++++++++--------
>  monitor/misc.c                       |  2 --
>  qom/qom-qmp-cmds.c                   | 25 +++++++++++++++++++++++--
>  storage-daemon/qemu-storage-daemon.c |  2 --
>  6 files changed, 32 insertions(+), 31 deletions(-)
> 
> +++ b/qapi/qom.json
> @@ -839,13 +839,6 @@
>  #
>  # Create a QOM object.
>  #
> -# @qom-type: the class name for the object to be created
> -#
> -# @id: the name of the new object
> -#
> -# Additional arguments depend on qom-type and are passed to the backend
> -# unchanged.
> -#
>  # Returns: Nothing on success
>  #          Error if @qom-type is not a valid class name
>  #
> @@ -859,9 +852,7 @@
>  # <- { "return": {} }
>  #
>  ##
> -{ 'command': 'object-add',
> -  'data': {'qom-type': 'str', 'id': 'str'},
> -  'gen': false } # so we can get the additional arguments
> +{ 'command': 'object-add', 'data': 'ObjectOptions', 'boxed': true }

So much more concise ;)  A grep for TYPE_USER_CREATABLE doesn't seem to
turn up any *_class_init() functions that your earlier patches in the
series missed, so I think you captured an accurate 1:1 mapping.  There
is include/chardev/char.h with the comment about "TODO: eventually use
TYPE_USER_CREATABLE" which may point to the next item to be added to
ObjectOptions, but that's not for this series.

> +++ b/qom/qom-qmp-cmds.c

>  
> -void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp)
> +void qmp_object_add(ObjectOptions *options, Error **errp)
>  {
> -    user_creatable_add_dict(qdict, false, errp);
> +    Visitor *v;
> +    QObject *qobj;
> +    QDict *props;
> +    Object *obj;
> +
> +    v = qobject_output_visitor_new(&qobj);
> +    visit_type_ObjectOptions(v, NULL, &options, &error_abort);
> +    visit_complete(v, &qobj);
> +    visit_free(v);

This part is nice...

> +
> +    props = qobject_to(QDict, qobj);
> +    qdict_del(props, "qom-type");
> +    qdict_del(props, "id");

...while this part makes it seem like we still have more cleanup to come
later.  But hey, progress!

> +
> +    v = qobject_input_visitor_new(QOBJECT(props));
> +    obj = user_creatable_add_type(ObjectType_str(options->qom_type),
> +                                  options->id, props, v, errp);
> +    object_unref(obj);
> +    visit_free(v);
>  }
>  

Once you address Paolo's comment, you can also add

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org