On 24/10/24 17:53, Peter Xu wrote:
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..

Maybe a simpler alternative is to add a field in ObjectClass, maintain
a single GHashTable to store TypeName -> Instance and retrieve as:

Object *singleton_get_instance(ObjectClass *class)
{
    return g_hash_table_lookup(&singletons,
                               object_class_get_name(class));
}

TBH the TYPE_SINGLETON interface seems a bit over-engineered and its
implementation error prone.

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,



Reply via email to