Am 26.02.2021 um 22:18 hat Eric Blake geschrieben:
> 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@xxxxxxxxxx>
> > ---
> > 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...
It's not really, though.
We're going from ObjectOptions to QDict just to feed the QDict back into
a visitor. The QDict step feels unnecessary, but we don't have a visitor
that visits existing QAPI objects. I think it would be somewhat similar
to the clone visitor, but not exactly the same thing.
> > +
> > + 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!
Ideally, I would like the whole function to look more or less like this:
void qmp_object_add(ObjectOptions *options, Error **errp)
{
Visitor *v = qapi_object_visitor_new(options);
Object *obj = user_creatable_add_type(v, errp);
object_unref(obj);
visit_free(v);
}
Can be done later (or never).
Kevin
> > +
> > + 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@xxxxxxxxxx>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
