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

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt; -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org