On Thu, Oct 24, 2024 at 05:02:19PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Peter,
Hi, Phil,
Thanks for the quick reviews!
>
> (Cc'ing Mark)
>
> On 24/10/24 13:56, Peter Xu wrote:
> > Signed-off-by: Peter Xu <pet...@redhat.com>
> > ---
> > include/qom/object_interfaces.h | 47 +++++++++++++++++++++++++++++++++
> > qom/object.c | 3 +++
> > qom/object_interfaces.c | 24 +++++++++++++++++
> > qom/qom-qmp-cmds.c | 22 ++++++++++++---
> > system/qdev-monitor.c | 7 +++++
> > 5 files changed, 100 insertions(+), 3 deletions(-)
>
>
> > +/**
> > + * SingletonClass:
> > + *
> > + * @parent_class: the base class
> > + * @get_instance: fetch the singleton instance if it is created,
> > + * NULL otherwise.
> > + *
> > + * Singleton class describes the type of object classes that can only
> > + * provide one instance for the whole lifecycle of QEMU. It will fail the
> > + * operation if one attemps to create more than one instance.
> > + *
> > + * One can fetch the single object using class's get_instance() callback if
> > + * it was created before. This can be useful for operations like QMP
> > + * qom-list-properties, where dynamically creating an object might not be
> > + * feasible.
> > + */
> > +struct SingletonClass {
> > + /* <private> */
> > + InterfaceClass parent_class;
> > + /* <public> */
> > + Object *(*get_instance)(Error **errp);
>
> IMHO asking get_instance() is overkill ...
>
> > +};
> > +
> > +/**
> > + * object_class_is_singleton:
> > + *
> > + * @class: the class to detect singleton
> > + *
> > + * Returns: true if it's a singleton class, false otherwise.
> > + */
> > +bool object_class_is_singleton(ObjectClass *class);
> > +
> > +/**
> > + * singleton_get_instance:
> > + *
> > + * @class: the class to fetch singleton instance
> > + *
> > + * Returns: the object* if the class is a singleton class and the singleton
> > + * object is created, NULL otherwise.
> > + */
> > +Object *singleton_get_instance(ObjectClass *class);
> > +
> > #endif
>
> > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> > index e0833c8bfe..6766060d0a 100644
> > --- a/qom/object_interfaces.c
> > +++ b/qom/object_interfaces.c
> > @@ -354,6 +354,23 @@ void user_creatable_cleanup(void)
> > object_unparent(object_get_objects_root());
> > }
> > +bool object_class_is_singleton(ObjectClass *class)
> > +{
> > + return !!object_class_dynamic_cast(class, TYPE_SINGLETON);
> > +}
> > +
> > +Object *singleton_get_instance(ObjectClass *class)
> > +{
>
> ... when we can use object_resolve_type_unambiguous:
>
> return object_resolve_type_unambiguous(object_class_get_name(class,
> NULL));
I think an issue is migration object is nowhere to be find under
object_get_root(), so it won't work there. A side benefit is, it's also
faster..
How about I use object_resolve_type_unambiguous() as a fallback? Then it's
used only if get_instance() is not provided.
>
> BTW should we pass Error** argument to singleton_get_instance()?
I didn't expect it to fail, hence I didn't even add it to make it more like
"this will always success or it asserts" kind of things. I left Error** in
the hook just to be slightly flexible, but I always pass in error_abort()
in this patch.
I can either add Error** if anyone thinks it useful, or even drop Error**
in ->get_instance() since it's mostly not used at least for now.
Let me know!
>
> > + SingletonClass *singleton =
> > + (SingletonClass *)object_class_dynamic_cast(class, TYPE_SINGLETON);
> > +
> > + if (!singleton) {
> > + return NULL;
> > + }
> > +
> > + return singleton->get_instance(&error_abort);
> > +}
>
> Alternatively call object_resolve_type_unambiguous() in instance_init()?
Thanks,
--
Peter Xu
