[PATCH 0/9] qom: misc cleanups / fixes

[PATCH 0/9] qom: misc cleanups / fixes

[Daniel P. Berrangé]

The overall theme of this patch is to move some logic out of
object_interfaces.c into object.c, since it is not tied to
the user creatable interface.

Daniel P. Berrangé (9):
  qom: add trace events for object/property lifecycle
  qom: validate ID format when creating objects
  qom: make errp last param in methods taking va_list
  qom: shorten name of object_set_properties_from_keyval
  qom: have object_set_props_keyval return bool
  qom: move object_set_prop_keyval into object.c
  qom: add object_new_with_props_from_qdict
  qom: fix ability to create objects without a parent
  qom: drop user_creatable_add_type method

 authz/listfile.c                |   4 +-
 include/qom/object.h            | 121 ++++++++++++++---
 include/qom/object_interfaces.h |  18 ---
 qom/object.c                    | 229 +++++++++++++++++++++++++++++---
 qom/object_interfaces.c         | 101 +-------------
 qom/trace-events                |  12 +-
 system/qdev-monitor.c           |   4 +-
 system/vl.c                     |   7 +-
 tests/unit/check-qom-proplist.c |  97 +++++++++++---
 9 files changed, 409 insertions(+), 184 deletions(-)

-- 
2.54.0

[PATCH 1/9] qom: add trace events for object/property lifecycle

[Daniel P. Berrangé]

This adds tracing around object allocation & finalization, the addition &
deletion of properties, and the addition & deletion of children.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qom/object.c     | 34 +++++++++++++++++++++++++++-------
 qom/trace-events | 12 ++++++++++--
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index f981e27044..b99da7309d 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -603,6 +603,8 @@ static void object_property_del_all(Object *obj)
         object_property_iter_init(&iter, obj);
         while ((prop = object_property_iter_next(&iter)) != NULL) {
             if (g_hash_table_add(done, prop)) {
+                trace_object_property_del(obj, obj->class->type->name,
+                                          prop->name, prop->opaque);
                 if (prop->release) {
                     prop->release(obj, prop->name, prop->opaque);
                     released = true;
@@ -621,10 +623,14 @@ static void object_property_del_child(Object *obj, Object *child)
     GHashTableIter iter;
     gpointer key, value;
 
+    trace_object_property_del_child(obj, obj->class->type->name,
+                                    child, child->class->type->name);
     g_hash_table_iter_init(&iter, obj->properties);
     while (g_hash_table_iter_next(&iter, &key, &value)) {
         prop = value;
         if (object_property_is_child(prop) && prop->opaque == child) {
+            trace_object_property_del(obj, obj->class->type->name,
+                                      prop->name, prop->opaque);
             if (prop->release) {
                 prop->release(obj, prop->name, prop->opaque);
                 prop->release = NULL;
@@ -664,7 +670,7 @@ static void object_finalize(void *data)
 {
     Object *obj = data;
     TypeImpl *ti = obj->class->type;
-
+    trace_object_finalize(obj, obj->class->type->name);
     object_property_del_all(obj);
     object_deinit(obj, ti);
 
@@ -714,6 +720,7 @@ static Object *object_new_with_type(Type type)
     object_initialize_with_type(obj, size, type);
     obj->free = obj_free;
 
+    trace_object_new(obj, obj ? obj->class->type->name : "(null)");
     return obj;
 }
 
@@ -844,8 +851,9 @@ Object *object_dynamic_cast(Object *obj, const char *typename)
 Object *object_dynamic_cast_assert(Object *obj, const char *typename,
                                    const char *file, int line, const char *func)
 {
-    trace_object_dynamic_cast_assert(obj ? obj->class->type->name : "(null)",
-                                     typename, file, line, func);
+    trace_object_dynamic_cast_assert(
+        obj, obj ? obj->class->type->name : "(null)",
+        typename, file, line, func);
 
 #ifdef CONFIG_QOM_CAST_DEBUG
     int i;
@@ -935,8 +943,9 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
 {
     ObjectClass *ret;
 
-    trace_object_class_dynamic_cast_assert(class ? class->type->name : "(null)",
-                                           typename, file, line, func);
+    trace_object_class_dynamic_cast_assert(
+        class ? class->type->name : "(null)",
+        typename, file, line, func);
 
 #ifdef CONFIG_QOM_CAST_DEBUG
     int i;
@@ -1220,6 +1229,8 @@ object_property_try_add(Object *obj, const char *name, const char *type,
     prop->release = release;
     prop->opaque = opaque;
 
+    trace_object_property_add(obj, obj->class->type->name,
+                              prop->name, prop->opaque);
     g_hash_table_insert(obj->properties, prop->name, prop);
     return prop;
 }
@@ -1258,6 +1269,8 @@ object_class_property_add(ObjectClass *klass,
     prop->release = release;
     prop->opaque = opaque;
 
+    trace_object_class_property_add(klass->type->name, prop->name,
+                                    prop->opaque);
     g_hash_table_insert(klass->properties, prop->name, prop);
 
     return prop;
@@ -1346,6 +1359,8 @@ void object_property_del(Object *obj, const char *name)
 {
     ObjectProperty *prop = g_hash_table_lookup(obj->properties, name);
 
+    trace_object_property_del(obj, obj->class->type->name, prop->name,
+                              prop->opaque);
     if (prop->release) {
         prop->release(obj, name, prop->opaque);
     }
@@ -1634,8 +1649,11 @@ int object_property_get_enum(Object *obj, const char *name,
 bool object_property_parse(Object *obj, const char *name,
                            const char *string, Error **errp)
 {
-    Visitor *v = string_input_visitor_new(string);
-    bool ok = object_property_set(obj, name, v, errp);
+    Visitor *v;
+    bool ok;
+    trace_object_property_parse(obj, obj->class->type->name, name, string);
+    v = string_input_visitor_new(string);
+    ok = object_property_set(obj, name, v, errp);
 
     visit_free(v);
     return ok;
@@ -1766,6 +1784,8 @@ object_property_try_add_child(Object *obj, const char *name,
     g_autofree char *type = NULL;
     ObjectProperty *op;
 
+    trace_object_property_add_child(obj, obj->class->type->name, name,
+                                    child, child->class->type->name);
     assert(!child->parent);
 
     type = g_strdup_printf("child<%s>", object_get_typename(child));
diff --git a/qom/trace-events b/qom/trace-events
index b2e9f4a712..9aabbae4f7 100644
--- a/qom/trace-events
+++ b/qom/trace-events
@@ -1,5 +1,13 @@
 # See docs/devel/tracing.rst for syntax documentation.
 
 # object.c
-object_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
-object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
+object_dynamic_cast_assert(void *obj, const char *type, const char *target, const char *file, int line, const char *func) "obj=%p type=%s->%s (%s:%d:%s)"
+object_finalize(void *obj, const char *type) "obj=%p type=%s"
+object_new(void *obj, const char *type) "obj=%p type=%s"
+object_property_add(void *obj, const char *type, const char *name, void *value) "obj=%p type=%s name=%s value=%p"
+object_property_add_child(void *obj, const char *type, const char *name, void *parent, const char *parenttype) "obj=%p type=%s name=%s parent=%p parent-type=%s"
+object_property_del(void *obj, const char *type, const char *name, void *value) "obj=%p type=%s name=%s value=%p"
+object_property_del_child(void *obj, const char *type, void *parent, const char *parenttype) "obj=%p type=%s parent=%p parent-type=%s"
+object_property_parse(void *obj, const char *type, const char *name, const char *value) "obj=%p type=%s prop=%s value=%s"
+object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "type=%s->%s (%s:%d:%s)"
+object_class_property_add(const char *type, const char *name, void *value) "type=%s name=%s value=%p"
-- 
2.54.0

[PATCH 2/9] qom: validate ID format when creating objects

[Daniel P. Berrangé]

The object_new_with_props/propv methods failed to validate the ID string
format, thus diverging from user_creatable_add_type.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qom/object.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/qom/object.c b/qom/object.c
index b99da7309d..9bbd02f4ab 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -24,6 +24,8 @@
 #include "qapi/forward-visitor.h"
 #include "qapi/qapi-builtin-visit.h"
 #include "qobject/qjson.h"
+#include "qemu/id.h"
+#include "qapi/qmp/qerror.h"
 #include "trace.h"
 
 /* TODO: replace QObject with a simpler visitor to avoid a dependency
@@ -764,6 +766,13 @@ Object *object_new_with_propv(const char *typename,
     ObjectClass *klass;
     UserCreatable *uc;
 
+    if (id != NULL && !id_wellformed(id)) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
+        error_append_hint(errp, "Identifiers consist of letters, digits, "
+                          "'-', '.', '_', starting with a letter.\n");
+        return NULL;
+    }
+
     klass = object_class_by_name(typename);
     if (!klass) {
         error_setg(errp, "invalid object type: %s", typename);
-- 
2.54.0

[PATCH 3/9] qom: make errp last param in methods taking va_list

[Daniel P. Berrangé]

object_new_with_props can't put 'errp' last due to the use of
variadic arguments. That constraint does not apply to the use
of va_list with object_new_with_propv, so follow normal practice
with 'errp' placement.

The same rationale applies to object_set_propv.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h            | 10 +++++-----
 qom/object.c                    | 20 ++++++++++----------
 tests/unit/check-qom-proplist.c |  4 ++--
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 510885218b..c7d80914fb 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -689,16 +689,16 @@ Object *object_new_with_props(const char *typename,
  * @typename:  The name of the type of the object to instantiate.
  * @parent: the parent object
  * @id: The unique ID of the object
- * @errp: pointer to error object
  * @vargs: list of property names and values
+ * @errp: pointer to error object
  *
  * See object_new_with_props() for documentation.
  */
 Object *object_new_with_propv(const char *typename,
                               Object *parent,
                               const char *id,
-                              Error **errp,
-                              va_list vargs);
+                              va_list vargs,
+                              Error **errp);
 
 /**
  * object_set_props:
@@ -739,14 +739,14 @@ bool object_set_props(Object *obj, Error **errp, ...) G_GNUC_NULL_TERMINATED;
 /**
  * object_set_propv:
  * @obj: the object instance to set properties on
- * @errp: pointer to error object
  * @vargs: list of property names and values
+ * @errp: pointer to error object
  *
  * See object_set_props() for documentation.
  *
  * Returns: %true on success, %false on error.
  */
-bool object_set_propv(Object *obj, Error **errp, va_list vargs);
+bool object_set_propv(Object *obj, va_list vargs, Error **errp);
 
 /**
  * object_initialize:
diff --git a/qom/object.c b/qom/object.c
index 9bbd02f4ab..2671c471ab 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -550,7 +550,7 @@ bool object_initialize_child_with_propsv(Object *parentobj,
     object_initialize(childobj, size, type);
     obj = OBJECT(childobj);
 
-    if (!object_set_propv(obj, errp, vargs)) {
+    if (!object_set_propv(obj, vargs, errp)) {
         goto out;
     }
 
@@ -749,7 +749,7 @@ Object *object_new_with_props(const char *typename,
     Object *obj;
 
     va_start(vargs, errp);
-    obj = object_new_with_propv(typename, parent, id, errp, vargs);
+    obj = object_new_with_propv(typename, parent, id, vargs, errp);
     va_end(vargs);
 
     return obj;
@@ -759,8 +759,8 @@ Object *object_new_with_props(const char *typename,
 Object *object_new_with_propv(const char *typename,
                               Object *parent,
                               const char *id,
-                              Error **errp,
-                              va_list vargs)
+                              va_list vargs,
+                              Error **errp)
 {
     Object *obj;
     ObjectClass *klass;
@@ -785,7 +785,7 @@ Object *object_new_with_propv(const char *typename,
     }
     obj = object_new_with_type(klass->type);
 
-    if (!object_set_propv(obj, errp, vargs)) {
+    if (!object_set_propv(obj, vargs, errp)) {
         goto error;
     }
 
@@ -813,14 +813,14 @@ Object *object_new_with_propv(const char *typename,
 
 
 bool object_set_props(Object *obj,
-                     Error **errp,
-                     ...)
+                      Error **errp,
+                      ...)
 {
     va_list vargs;
     bool ret;
 
     va_start(vargs, errp);
-    ret = object_set_propv(obj, errp, vargs);
+    ret = object_set_propv(obj, vargs, errp);
     va_end(vargs);
 
     return ret;
@@ -828,8 +828,8 @@ bool object_set_props(Object *obj,
 
 
 bool object_set_propv(Object *obj,
-                     Error **errp,
-                     va_list vargs)
+                      va_list vargs,
+                      Error **errp)
 {
     const char *propname;
 
diff --git a/tests/unit/check-qom-proplist.c b/tests/unit/check-qom-proplist.c
index ee3c6fb32b..7f31735459 100644
--- a/tests/unit/check-qom-proplist.c
+++ b/tests/unit/check-qom-proplist.c
@@ -373,8 +373,8 @@ static Object *new_helper(Error **errp,
     obj = object_new_with_propv(TYPE_DUMMY,
                                 parent,
                                 "dummy0",
-                                errp,
-                                vargs);
+                                vargs,
+                                errp);
     va_end(vargs);
     return obj;
 }
-- 
2.54.0

[PATCH 4/9] qom: shorten name of object_set_properties_from_keyval

[Daniel P. Berrangé]

This matches the convention established by the object_set_props and
object_set_propv methods.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h    |  6 +++---
 qom/object_interfaces.c | 12 ++++++------
 system/qdev-monitor.c   |  4 ++--
 system/vl.c             |  7 ++++---
 4 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index c7d80914fb..d841c338a2 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -915,7 +915,7 @@ type_init(do_qemu_init_ ## type_array)
 bool type_print_class_properties(const char *type);
 
 /**
- * object_set_properties_from_keyval:
+ * object_set_props_from_keyval:
  * @obj: a QOM object
  * @qdict: a dictionary with the properties to be set
  * @from_json: true if leaf values of @qdict are typed, false if they
@@ -925,8 +925,8 @@ bool type_print_class_properties(const char *type);
  * For each key in the dictionary, parse the value string if needed,
  * then set the corresponding property in @obj.
  */
-void object_set_properties_from_keyval(Object *obj, const QDict *qdict,
-                                       bool from_json, Error **errp);
+void object_set_props_from_keyval(Object *obj, const QDict *qdict,
+                                  bool from_json, Error **errp);
 
 /**
  * object_class_dynamic_cast_assert:
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 415cbee8c5..4377d65b76 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -44,8 +44,8 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
     }
 }
 
-static void object_set_properties_from_qdict(Object *obj, const QDict *qdict,
-                                             Visitor *v, Error **errp)
+static void object_set_props_from_qdict(Object *obj, const QDict *qdict,
+                                        Visitor *v, Error **errp)
 {
     const QDictEntry *e;
 
@@ -62,8 +62,8 @@ out:
     visit_end_struct(v, NULL);
 }
 
-void object_set_properties_from_keyval(Object *obj, const QDict *qdict,
-                                       bool from_json, Error **errp)
+void object_set_props_from_keyval(Object *obj, const QDict *qdict,
+                                  bool from_json, Error **errp)
 {
     Visitor *v;
     if (from_json) {
@@ -71,7 +71,7 @@ void object_set_properties_from_keyval(Object *obj, const QDict *qdict,
     } else {
         v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
     }
-    object_set_properties_from_qdict(obj, qdict, v, errp);
+    object_set_props_from_qdict(obj, qdict, v, errp);
     visit_free(v);
 }
 
@@ -110,7 +110,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
 
     assert(qdict);
     obj = object_new_with_class(klass);
-    object_set_properties_from_qdict(obj, qdict, v, &local_err);
+    object_set_props_from_qdict(obj, qdict, v, &local_err);
     if (local_err) {
         goto out;
     }
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 1ac6d9a857..bbbab5acae 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -721,8 +721,8 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
     qdict_del(properties, "bus");
     qdict_del(properties, "id");
 
-    object_set_properties_from_keyval(&dev->parent_obj, properties, from_json,
-                                      errp);
+    object_set_props_from_keyval(&dev->parent_obj, properties, from_json,
+                                 errp);
     qobject_unref(properties);
     if (*errp) {
         goto err_del_dev;
diff --git a/system/vl.c b/system/vl.c
index 0e1fc217b4..c599ee57b9 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2007,7 +2007,8 @@ static bool object_create_early(const char *type)
 
 static void qemu_apply_machine_options(QDict *qdict)
 {
-    object_set_properties_from_keyval(OBJECT(current_machine), qdict, false, &error_fatal);
+    object_set_props_from_keyval(OBJECT(current_machine), qdict,
+                                 false, &error_fatal);
 
     if (semihosting_enabled(false) && !semihosting_get_argc()) {
         /* fall back to the -kernel/-append */
@@ -2222,8 +2223,8 @@ static void qemu_create_machine(QDict *qdict)
             keyval_parse(machine_class->default_machine_opts, NULL, NULL,
                          &error_abort);
         qemu_apply_legacy_machine_options(default_opts);
-        object_set_properties_from_keyval(OBJECT(current_machine), default_opts,
-                                          false, &error_abort);
+        object_set_props_from_keyval(OBJECT(current_machine), default_opts,
+                                     false, &error_abort);
         qobject_unref(default_opts);
     }
 }
-- 
2.54.0

[PATCH 5/9] qom: have object_set_props_keyval return bool

[Daniel P. Berrangé]

This matches the convention established by the object_set_props and
object_set_propv methods.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h    |  4 +++-
 qom/object_interfaces.c | 13 +++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index d841c338a2..20a04cc4a9 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -924,8 +924,10 @@ bool type_print_class_properties(const char *type);
  *
  * For each key in the dictionary, parse the value string if needed,
  * then set the corresponding property in @obj.
+ *
+ * Returns: %true on success, %false on error.
  */
-void object_set_props_from_keyval(Object *obj, const QDict *qdict,
+bool object_set_props_from_keyval(Object *obj, const QDict *qdict,
                                   bool from_json, Error **errp);
 
 /**
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 4377d65b76..50736b80c8 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -44,13 +44,14 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
     }
 }
 
-static void object_set_props_from_qdict(Object *obj, const QDict *qdict,
+static bool object_set_props_from_qdict(Object *obj, const QDict *qdict,
                                         Visitor *v, Error **errp)
 {
+    ERRP_GUARD();
     const QDictEntry *e;
 
     if (!visit_start_struct(v, NULL, NULL, 0, errp)) {
-        return;
+        return false;
     }
     for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
         if (!object_property_set(obj, e->key, v, errp)) {
@@ -60,19 +61,23 @@ static void object_set_props_from_qdict(Object *obj, const QDict *qdict,
     visit_check_struct(v, errp);
 out:
     visit_end_struct(v, NULL);
+
+    return *errp == NULL;
 }
 
-void object_set_props_from_keyval(Object *obj, const QDict *qdict,
+bool object_set_props_from_keyval(Object *obj, const QDict *qdict,
                                   bool from_json, Error **errp)
 {
+    bool ret;
     Visitor *v;
     if (from_json) {
         v = qobject_input_visitor_new(QOBJECT(qdict));
     } else {
         v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
     }
-    object_set_props_from_qdict(obj, qdict, v, errp);
+    ret = object_set_props_from_qdict(obj, qdict, v, errp);
     visit_free(v);
+    return ret;
 }
 
 Object *user_creatable_add_type(const char *type, const char *id,
-- 
2.54.0

[PATCH 6/9] qom: move object_set_prop_keyval into object.c

[Daniel P. Berrangé]

This matches the location of the object_set_props and object_set_propv
methods, since this method is not inherently tied to the user creatable
interface. As part of this, object_set_props_from_qdict is also exposed
as a public API since it is still called from object_interfaces.c.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h    | 47 +++++++++++++++++++++++++++--------------
 qom/object.c            | 37 ++++++++++++++++++++++++++++++++
 qom/object_interfaces.c | 36 -------------------------------
 3 files changed, 68 insertions(+), 52 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 20a04cc4a9..1b2b2702fc 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -748,6 +748,37 @@ bool object_set_props(Object *obj, Error **errp, ...) G_GNUC_NULL_TERMINATED;
  */
 bool object_set_propv(Object *obj, va_list vargs, Error **errp);
 
+/**
+ * object_set_props_from_qdict:
+ * @obj: a QOM object
+ * @qdict: a dictionary with the properties to be set
+ * @v: a visitor to iterate over @dict
+ * @errp: pointer to error object
+ *
+ * For each key in the dictionary, set the corresponding
+ * property in @obj.
+ *
+ * Returns: %true on success, %false on error.
+ */
+bool object_set_props_from_qdict(Object *obj, const QDict *qdict,
+                                 Visitor *v, Error **errp);
+
+/**
+ * object_set_props_from_keyval:
+ * @obj: a QOM object
+ * @qdict: a dictionary with the properties to be set
+ * @from_json: true if leaf values of @qdict are typed, false if they
+ * are strings
+ * @errp: pointer to error object
+ *
+ * For each key in the dictionary, parse the value string if needed,
+ * then set the corresponding property in @obj.
+ *
+ * Returns: %true on success, %false on error.
+ */
+bool object_set_props_from_keyval(Object *obj, const QDict *qdict,
+                                  bool from_json, Error **errp);
+
 /**
  * object_initialize:
  * @obj: A pointer to the memory to be used for the object.
@@ -914,22 +945,6 @@ type_init(do_qemu_init_ ## type_array)
  */
 bool type_print_class_properties(const char *type);
 
-/**
- * object_set_props_from_keyval:
- * @obj: a QOM object
- * @qdict: a dictionary with the properties to be set
- * @from_json: true if leaf values of @qdict are typed, false if they
- * are strings
- * @errp: pointer to error object
- *
- * For each key in the dictionary, parse the value string if needed,
- * then set the corresponding property in @obj.
- *
- * Returns: %true on success, %false on error.
- */
-bool object_set_props_from_keyval(Object *obj, const QDict *qdict,
-                                  bool from_json, Error **errp);
-
 /**
  * object_class_dynamic_cast_assert:
  * @klass: The #ObjectClass to attempt to cast.
diff --git a/qom/object.c b/qom/object.c
index 2671c471ab..c44e2b2eff 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -23,7 +23,9 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/forward-visitor.h"
 #include "qapi/qapi-builtin-visit.h"
+#include "qobject/qdict.h"
 #include "qobject/qjson.h"
+#include "qobject/qdict.h"
 #include "qemu/id.h"
 #include "qapi/qmp/qerror.h"
 #include "trace.h"
@@ -847,6 +849,41 @@ bool object_set_propv(Object *obj,
     return true;
 }
 
+bool object_set_props_from_qdict(Object *obj, const QDict *qdict,
+                                 Visitor *v, Error **errp)
+{
+    ERRP_GUARD();
+    const QDictEntry *e;
+
+    if (!visit_start_struct(v, NULL, NULL, 0, errp)) {
+        return false;
+    }
+    for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+        if (!object_property_set(obj, e->key, v, errp)) {
+            goto out;
+        }
+    }
+    visit_check_struct(v, errp);
+out:
+    visit_end_struct(v, NULL);
+
+    return *errp == NULL;
+}
+
+bool object_set_props_from_keyval(Object *obj, const QDict *qdict,
+                                  bool from_json, Error **errp)
+{
+    bool ret;
+    Visitor *v;
+    if (from_json) {
+        v = qobject_input_visitor_new(QOBJECT(qdict));
+    } else {
+        v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+    }
+    ret = object_set_props_from_qdict(obj, qdict, v, errp);
+    visit_free(v);
+    return ret;
+}
 
 Object *object_dynamic_cast(Object *obj, const char *typename)
 {
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 50736b80c8..e0a3cd8d0f 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -44,42 +44,6 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
     }
 }
 
-static bool object_set_props_from_qdict(Object *obj, const QDict *qdict,
-                                        Visitor *v, Error **errp)
-{
-    ERRP_GUARD();
-    const QDictEntry *e;
-
-    if (!visit_start_struct(v, NULL, NULL, 0, errp)) {
-        return false;
-    }
-    for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
-        if (!object_property_set(obj, e->key, v, errp)) {
-            goto out;
-        }
-    }
-    visit_check_struct(v, errp);
-out:
-    visit_end_struct(v, NULL);
-
-    return *errp == NULL;
-}
-
-bool object_set_props_from_keyval(Object *obj, const QDict *qdict,
-                                  bool from_json, Error **errp)
-{
-    bool ret;
-    Visitor *v;
-    if (from_json) {
-        v = qobject_input_visitor_new(QOBJECT(qdict));
-    } else {
-        v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
-    }
-    ret = object_set_props_from_qdict(obj, qdict, v, errp);
-    visit_free(v);
-    return ret;
-}
-
 Object *user_creatable_add_type(const char *type, const char *id,
                                 const QDict *qdict,
                                 Visitor *v, Error **errp)
-- 
2.54.0

[PATCH 7/9] qom: add object_new_with_props_from_qdict

[Daniel P. Berrangé]

This will be used to replace user_creatable_add_type with an impl
that shares most code with object_new_with_props, and is not tied
to the user creatable interface.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h | 19 +++++++++++
 qom/object.c         | 75 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 88 insertions(+), 6 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 1b2b2702fc..cd59f1f171 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -700,6 +700,25 @@ Object *object_new_with_propv(const char *typename,
                               va_list vargs,
                               Error **errp);
 
+/**
+ * object_new_with_props_from_qdict:
+ * @typename:  The name of the type of the object to instantiate.
+ * @parent: the parent object
+ * @id: The unique ID of the object
+ * @props: dictionary of property names and values
+ * @v: visitor to iterate over @props
+ * @errp: pointer to error object
+ *
+ * A variant of object_new_with_props() which accepts the
+ * properties in a QDict.
+ */
+Object *object_new_with_props_from_qdict(const char *typename,
+                                         Object *parent,
+                                         const char *id,
+                                         QDict *props,
+                                         Visitor *v,
+                                         Error **errp);
+
 /**
  * object_set_props:
  * @obj: the object instance to set properties on
diff --git a/qom/object.c b/qom/object.c
index c44e2b2eff..b35a3dff06 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -758,11 +758,14 @@ Object *object_new_with_props(const char *typename,
 }
 
 
-Object *object_new_with_propv(const char *typename,
-                              Object *parent,
-                              const char *id,
-                              va_list vargs,
-                              Error **errp)
+static Object *
+object_new_with_props_helper(const char *typename,
+                             Object *parent,
+                             const char *id,
+                             void *props,
+                             bool (set_props)(Object *obj, void *props,
+                                              Error **errp),
+                             Error **errp)
 {
     Object *obj;
     ObjectClass *klass;
@@ -787,7 +790,7 @@ Object *object_new_with_propv(const char *typename,
     }
     obj = object_new_with_type(klass->type);
 
-    if (!object_set_propv(obj, vargs, errp)) {
+    if (!set_props(obj, props, errp)) {
         goto error;
     }
 
@@ -813,6 +816,66 @@ Object *object_new_with_propv(const char *typename,
     return NULL;
 }
 
+struct ObjectNewVargsData {
+    va_list vargs;
+};
+
+static bool object_new_with_propv_setter(Object *obj,
+                                         void *props,
+                                         Error **errp)
+{
+    struct ObjectNewVargsData *data = props;
+    return object_set_propv(obj, data->vargs, errp);
+}
+
+Object *object_new_with_propv(const char *typename,
+                              Object *parent,
+                              const char *id,
+                              va_list vargs,
+                              Error **errp)
+{
+    Object *obj;
+    struct ObjectNewVargsData data;
+
+    va_copy(data.vargs, vargs);
+    obj = object_new_with_props_helper(typename,
+                                       parent,
+                                       id,
+                                       &data,
+                                       object_new_with_propv_setter,
+                                       errp);
+    va_end(data.vargs);
+    return obj;
+}
+
+struct ObjectNewQDictData {
+    QDict *props;
+    Visitor *v;
+};
+
+static bool object_new_with_qdict_setter(Object *obj,
+                                         void *props,
+                                         Error **errp)
+{
+    struct ObjectNewQDictData *data = props;
+    return object_set_props_from_qdict(obj, data->props, data->v, errp);
+}
+
+Object *object_new_with_props_from_qdict(const char *typename,
+                                         Object *parent,
+                                         const char *id,
+                                         QDict *props,
+                                         Visitor *v,
+                                         Error **errp)
+{
+    struct ObjectNewQDictData data = { props, v };
+    return object_new_with_props_helper(typename,
+                                        parent,
+                                        id,
+                                        &data,
+                                        object_new_with_qdict_setter,
+                                        errp);
+}
 
 bool object_set_props(Object *obj,
                       Error **errp,
-- 
2.54.0

[PATCH 8/9] qom: fix ability to create objects without a parent

[Daniel P. Berrangé]

object_new_with_propv allowed id/parent to be optional, in which case
the caller was expected to own the returned object. Unfortunately a
trailing object_unref() meant that the returned object was already
freed.

It is confusing to have a single method with two different ownership
scenarios for the returned object.

Make id/parent mandatory in object_new_with_propv once more, and add
a new object_new_with_propv_parentless that does not accept id/parent
at all and lets the caller own the returned reference.

The helper method has abstracted the way properties are represented
and setk in order to facilitate the subsequent commit.

Unit tests are added to address the root cause that allowed the bug
to slip through in commit 6134d752.

Fixes: 6134d7522e570a30d7f0d1e092ee37351c5183ed
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qom/object.h            | 47 ++++++++++++++++++
 qom/object.c                    | 74 +++++++++++++++++++++++----
 tests/unit/check-qom-proplist.c | 88 ++++++++++++++++++++++++++++-----
 3 files changed, 187 insertions(+), 22 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index cd59f1f171..71530dac76 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -719,6 +719,53 @@ Object *object_new_with_props_from_qdict(const char *typename,
                                          Visitor *v,
                                          Error **errp);
 
+/**
+ * object_new_with_props_parentless:
+ * @typename:  The name of the type of the object to instantiate.
+ * @errp: pointer to error object
+ * @...: list of property names and values
+ *
+ * Behaviour as object_new_with_props(), except the object
+ * will not be added to any parent and thus the caller will
+ * own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_props_parentless(const char *typename,
+                                         Error **errp,
+                                         ...) G_GNUC_NULL_TERMINATED;
+
+/**
+ * object_new_with_propv_parentless:
+ * @typename:  The name of the type of the object to instantiate.
+ * @vargs: list of property names and values
+ * @errp: pointer to error object
+ *
+ * Behaviour as object_new_with_propv(), except the object
+ * will not be added to any parent and thus the caller will
+ * own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_propv_parentless(const char *typename,
+                                         va_list vargs,
+                                         Error **errp);
+
+/**
+ * object_new_with_props_from_qdict_parentless:
+ * @typename:  The name of the type of the object to instantiate.
+ * @props: dictionary of property names and values
+ * @v: visitor to iterate over @props
+ * @errp: pointer to error object
+ *
+ * Behaviour as object_new_with_props_from_qdict(), except the
+ * object will not be added to any parent and thus the caller
+ * will own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_props_from_qdict_parentless(const char *typename,
+                                                    QDict *props,
+                                                    Visitor *v,
+                                                    Error **errp);
+
 /**
  * object_set_props:
  * @obj: the object instance to set properties on
diff --git a/qom/object.c b/qom/object.c
index b35a3dff06..9dc2bdb603 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -750,6 +750,8 @@ Object *object_new_with_props(const char *typename,
     va_list vargs;
     Object *obj;
 
+    assert(parent != NULL);
+    assert(id != NULL);
     va_start(vargs, errp);
     obj = object_new_with_propv(typename, parent, id, vargs, errp);
     va_end(vargs);
@@ -767,10 +769,14 @@ object_new_with_props_helper(const char *typename,
                                               Error **errp),
                              Error **errp)
 {
+    ERRP_GUARD();
     Object *obj;
     ObjectClass *klass;
     UserCreatable *uc;
 
+    assert((id != NULL && parent != NULL) ||
+           (id == NULL && parent == NULL));
+
     if (id != NULL && !id_wellformed(id)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
         error_append_hint(errp, "Identifiers consist of letters, digits, "
@@ -795,7 +801,10 @@ object_new_with_props_helper(const char *typename,
     }
 
     if (id != NULL) {
-        object_property_add_child(parent, id, obj);
+        object_property_try_add_child(parent, id, obj, errp);
+        if (*errp) {
+            goto error;
+        }
     }
 
     uc = (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
@@ -808,7 +817,6 @@ object_new_with_props_helper(const char *typename,
         }
     }
 
-    object_unref(obj);
     return obj;
 
  error:
@@ -836,7 +844,8 @@ Object *object_new_with_propv(const char *typename,
 {
     Object *obj;
     struct ObjectNewVargsData data;
-
+    assert(parent != NULL);
+    assert(id != NULL);
     va_copy(data.vargs, vargs);
     obj = object_new_with_props_helper(typename,
                                        parent,
@@ -845,6 +854,9 @@ Object *object_new_with_propv(const char *typename,
                                        object_new_with_propv_setter,
                                        errp);
     va_end(data.vargs);
+    if (obj) {
+        object_unref(obj);
+    }
     return obj;
 }
 
@@ -869,12 +881,56 @@ Object *object_new_with_props_from_qdict(const char *typename,
                                          Error **errp)
 {
     struct ObjectNewQDictData data = { props, v };
-    return object_new_with_props_helper(typename,
-                                        parent,
-                                        id,
-                                        &data,
-                                        object_new_with_qdict_setter,
-                                        errp);
+    Object *obj;
+    assert(parent != NULL);
+    assert(id != NULL);
+    obj = object_new_with_props_helper(typename,
+                                       parent,
+                                       id,
+                                       &data,
+                                       object_new_with_qdict_setter,
+                                       errp);
+    if (obj) {
+        object_unref(obj);
+    }
+    return obj;
+}
+
+Object *object_new_with_props_parentless(const char *typename,
+                                         Error **errp,
+                                         ...)
+{
+    va_list vargs;
+    Object *obj;
+
+    va_start(vargs, errp);
+    obj = object_new_with_propv_parentless(typename, vargs, errp);
+    va_end(vargs);
+
+    return obj;
+}
+
+Object *object_new_with_propv_parentless(const char *typename,
+                                         va_list vargs,
+                                         Error **errp)
+{
+    Object *ret;
+    struct ObjectNewVargsData data;
+    va_copy(data.vargs, vargs);
+    ret = object_new_with_props_helper(typename, NULL, NULL, &data,
+                                       object_new_with_propv_setter, errp);
+    va_end(vargs);
+    return ret;
+}
+
+Object *object_new_with_props_from_qdict_parentless(const char *typename,
+                                                    QDict *props,
+                                                    Visitor *v,
+                                                    Error **errp)
+{
+    struct ObjectNewQDictData data = { props, v };
+    return object_new_with_props_helper(typename, NULL, NULL, &data,
+                                        object_new_with_qdict_setter, errp);
 }
 
 bool object_set_props(Object *obj,
diff --git a/tests/unit/check-qom-proplist.c b/tests/unit/check-qom-proplist.c
index 7f31735459..954c898ce1 100644
--- a/tests/unit/check-qom-proplist.c
+++ b/tests/unit/check-qom-proplist.c
@@ -336,7 +336,7 @@ static QemuOptsList qemu_object_opts = {
 };
 
 
-static void test_dummy_createv(void)
+static void test_dummy_createv_tree(void)
 {
     Error *err = NULL;
     Object *parent = object_get_objects_root();
@@ -351,6 +351,7 @@ static void test_dummy_createv(void)
                               NULL));
 
     g_assert(err == NULL);
+    g_assert_cmpint(dobj->parent_obj.ref, ==, 1);
     g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
     g_assert(dobj->bv == true);
     g_assert(dobj->av == DUMMY_PLATYPUS);
@@ -362,9 +363,30 @@ static void test_dummy_createv(void)
 }
 
 
-static Object *new_helper(Error **errp,
-                          Object *parent,
-                          ...)
+static void test_dummy_createv_parentless(void)
+{
+    Error *err = NULL;
+    DummyObject *dobj = DUMMY_OBJECT(
+        object_new_with_props_parentless(TYPE_DUMMY,
+                                         &err,
+                                         "bv", "yes",
+                                         "sv", "Hiss hiss hiss",
+                                         "av", "platypus",
+                                         NULL));
+
+    g_assert(err == NULL);
+    g_assert_cmpint(dobj->parent_obj.ref, ==, 1);
+    g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
+    g_assert(dobj->bv == true);
+    g_assert(dobj->av == DUMMY_PLATYPUS);
+
+    object_unref(OBJECT(dobj));
+}
+
+
+static Object *new_helper_tree(Error **errp,
+                               Object *parent,
+                               ...)
 {
     va_list vargs;
     Object *obj;
@@ -379,19 +401,20 @@ static Object *new_helper(Error **errp,
     return obj;
 }
 
-static void test_dummy_createlist(void)
+static void test_dummy_createlist_tree(void)
 {
     Error *err = NULL;
     Object *parent = object_get_objects_root();
     DummyObject *dobj = DUMMY_OBJECT(
-        new_helper(&err,
-                   parent,
-                   "bv", "yes",
-                   "sv", "Hiss hiss hiss",
-                   "av", "platypus",
-                   NULL));
+        new_helper_tree(&err,
+                        parent,
+                        "bv", "yes",
+                        "sv", "Hiss hiss hiss",
+                        "av", "platypus",
+                        NULL));
 
     g_assert(err == NULL);
+    g_assert_cmpint(dobj->parent_obj.ref, ==, 1);
     g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
     g_assert(dobj->bv == true);
     g_assert(dobj->av == DUMMY_PLATYPUS);
@@ -402,6 +425,39 @@ static void test_dummy_createlist(void)
     object_unparent(OBJECT(dobj));
 }
 
+static Object *new_helper_parentless(Error **errp,
+                                     ...)
+{
+    va_list vargs;
+    Object *obj;
+
+    va_start(vargs, errp);
+    obj = object_new_with_propv_parentless(TYPE_DUMMY,
+                                           vargs,
+                                           errp);
+    va_end(vargs);
+    return obj;
+}
+
+static void test_dummy_createlist_parentless(void)
+{
+    Error *err = NULL;
+    DummyObject *dobj = DUMMY_OBJECT(
+        new_helper_parentless(&err,
+                              "bv", "yes",
+                              "sv", "Hiss hiss hiss",
+                              "av", "platypus",
+                              NULL));
+
+    g_assert(err == NULL);
+    g_assert_cmpint(dobj->parent_obj.ref, ==, 1);
+    g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
+    g_assert(dobj->bv == true);
+    g_assert(dobj->av == DUMMY_PLATYPUS);
+
+    object_unref(OBJECT(dobj));
+}
+
 static bool test_create_obj(QDict *qdict, Error **errp)
 {
     Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
@@ -658,8 +714,14 @@ int main(int argc, char **argv)
     type_register_static(&dummy_bus_info);
     type_register_static(&dummy_backend_info);
 
-    g_test_add_func("/qom/proplist/createlist", test_dummy_createlist);
-    g_test_add_func("/qom/proplist/createv", test_dummy_createv);
+    g_test_add_func("/qom/proplist/createlist/tree",
+                    test_dummy_createlist_tree);
+    g_test_add_func("/qom/proplist/createlist/parentless",
+                    test_dummy_createlist_parentless);
+    g_test_add_func("/qom/proplist/createv/tree",
+                    test_dummy_createv_tree);
+    g_test_add_func("/qom/proplist/createv/parentless",
+                    test_dummy_createv_parentless);
     g_test_add_func("/qom/proplist/createcmdline", test_dummy_createcmdl);
     g_test_add_func("/qom/proplist/badenum", test_dummy_badenum);
     g_test_add_func("/qom/proplist/getenum", test_dummy_getenum);
-- 
2.54.0

[PATCH 9/9] qom: drop user_creatable_add_type method

[Daniel P. Berrangé]

This can be replaced by object_new_with_props_from_qdict, which does
functionally the same job, but the caller does not own the returned
reference, instead the parent object owns it.

In one case we can use object_new_with_props_from_qdict_owned instead
since the object is not intended to have any parent.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 authz/listfile.c                |  4 +-
 include/qom/object_interfaces.h | 18 ---------
 qom/object_interfaces.c         | 70 ++-------------------------------
 tests/unit/check-qom-proplist.c |  5 +--
 4 files changed, 7 insertions(+), 90 deletions(-)

diff --git a/authz/listfile.c b/authz/listfile.c
index 13741d5a72..23655f8663 100644
--- a/authz/listfile.c
+++ b/authz/listfile.c
@@ -79,8 +79,8 @@ qauthz_list_file_load(QAuthZListFile *fauthz, Error **errp)
 
     v = qobject_input_visitor_new(obj);
 
-    ret = (QAuthZ *)user_creatable_add_type(TYPE_QAUTHZ_LIST,
-                                            NULL, pdict, v, errp);
+    ret = QAUTHZ(object_new_with_props_from_qdict_parentless(
+                     TYPE_QAUTHZ_LIST, pdict, v, errp));
 
  cleanup:
     visit_free(v);
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 02b11a7ef0..e2b8615617 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -69,24 +69,6 @@ bool user_creatable_complete(UserCreatable *uc, Error **errp);
  */
 bool user_creatable_can_be_deleted(UserCreatable *uc);
 
-/**
- * user_creatable_add_type:
- * @type: the object type name
- * @id: the unique ID for the object
- * @qdict: the object properties
- * @v: the visitor
- * @errp: if an error occurs, a pointer to an area to store the error
- *
- * Create an instance of the user creatable object @type, placing
- * it in the object composition tree with name @id, initializing
- * it with properties from @qdict
- *
- * Returns: the newly created object or NULL on error
- */
-Object *user_creatable_add_type(const char *type, const char *id,
-                                const QDict *qdict,
-                                Visitor *v, Error **errp);
-
 /**
  * user_creatable_add_qapi:
  * @options: the object definition
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index e0a3cd8d0f..7080f85f95 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -44,75 +44,11 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
     }
 }
 
-Object *user_creatable_add_type(const char *type, const char *id,
-                                const QDict *qdict,
-                                Visitor *v, Error **errp)
-{
-    ERRP_GUARD();
-    Object *obj;
-    ObjectClass *klass;
-    Error *local_err = NULL;
-
-    if (id != NULL && !id_wellformed(id)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
-        error_append_hint(errp, "Identifiers consist of letters, digits, "
-                          "'-', '.', '_', starting with a letter.\n");
-        return NULL;
-    }
-
-    klass = module_object_class_by_name(type);
-    if (!klass) {
-        error_setg(errp, "invalid object type: %s", type);
-        return NULL;
-    }
-
-    if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
-        error_setg(errp, "object type '%s' isn't supported by object-add",
-                   type);
-        return NULL;
-    }
-
-    if (object_class_is_abstract(klass)) {
-        error_setg(errp, "object type '%s' is abstract", type);
-        return NULL;
-    }
-
-    assert(qdict);
-    obj = object_new_with_class(klass);
-    object_set_props_from_qdict(obj, qdict, v, &local_err);
-    if (local_err) {
-        goto out;
-    }
-
-    if (id != NULL) {
-        object_property_try_add_child(object_get_objects_root(),
-                                      id, obj, &local_err);
-        if (local_err) {
-            goto out;
-        }
-    }
-
-    if (!user_creatable_complete(USER_CREATABLE(obj), &local_err)) {
-        if (id != NULL) {
-            object_property_del(object_get_objects_root(), id);
-        }
-        goto out;
-    }
-out:
-    if (local_err) {
-        error_propagate(errp, local_err);
-        object_unref(obj);
-        return NULL;
-    }
-    return obj;
-}
-
 void user_creatable_add_qapi(ObjectOptions *options, Error **errp)
 {
     Visitor *v;
     QObject *qobj;
     QDict *props;
-    Object *obj;
 
     v = qobject_output_visitor_new(&qobj);
     visit_type_ObjectOptions(v, NULL, &options, &error_abort);
@@ -124,9 +60,9 @@ void user_creatable_add_qapi(ObjectOptions *options, Error **errp)
     qdict_del(props, "id");
 
     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);
+    object_new_with_props_from_qdict(ObjectType_str(options->qom_type),
+                                     object_get_objects_root(),
+                                     options->id, props, v, errp);
     qobject_unref(qobj);
     visit_free(v);
 }
diff --git a/tests/unit/check-qom-proplist.c b/tests/unit/check-qom-proplist.c
index 954c898ce1..89de92b7d9 100644
--- a/tests/unit/check-qom-proplist.c
+++ b/tests/unit/check-qom-proplist.c
@@ -461,10 +461,9 @@ static void test_dummy_createlist_parentless(void)
 static bool test_create_obj(QDict *qdict, Error **errp)
 {
     Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
-    Object *obj = user_creatable_add_type(TYPE_DUMMY, "dev0", qdict, v, errp);
-
+    Object *obj = object_new_with_props_from_qdict(
+        TYPE_DUMMY, object_get_objects_root(), "dev0", qdict, v, errp);
     visit_free(v);
-    object_unref(obj);
     return !!obj;
 }
 
-- 
2.54.0

Re: [PATCH 6/9] qom: move object_set_prop_keyval into object.c

[marcandre.lureau]

On Wed, 29 Apr 2026 22:08:27 +0100, Daniel P. Berrangé <berrange@redhat.com> wrote:
> diff --git a/qom/object.c b/qom/object.c
> index 2671c471ab2..c44e2b2eff4 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -23,7 +23,9 @@
>  #include "qapi/qobject-input-visitor.h"
>  #include "qapi/forward-visitor.h"
>  #include "qapi/qapi-builtin-visit.h"
> +#include "qobject/qdict.h"
>  #include "qobject/qjson.h"
> +#include "qobject/qdict.h"

double-included

-- 
Marc-André Lureau <marcandre.lureau@redhat.com>

Re: [PATCH 1/9] qom: add trace events for object/property lifecycle

[marcandre.lureau]

On Wed, 29 Apr 2026 22:08:22 +0100, Daniel P. Berrangé <berrange@redhat.com> wrote:
> diff --git a/qom/object.c b/qom/object.c
> index f981e270440..b99da7309db 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -714,6 +720,7 @@ static Object *object_new_with_type(Type type)
>      object_initialize_with_type(obj, size, type);
>      obj->free = obj_free;
>  
> +    trace_object_new(obj, obj ? obj->class->type->name : "(null)");

It looks like there should be an assert(obj != NULL) after qemu_memalign() instead.

-- 
Marc-André Lureau <marcandre.lureau@redhat.com>

Re: [PATCH 9/9] qom: drop user_creatable_add_type method

[marcandre.lureau]

On Wed, 29 Apr 2026 22:08:30 +0100, Daniel P. Berrangé <berrange@redhat.com> wrote:
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index e0a3cd8d0f7..7080f85f95f 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -44,75 +44,11 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
> [ ... skip 14 lines ... ]
> -        error_append_hint(errp, "Identifiers consist of letters, digits, "
> -                          "'-', '.', '_', starting with a letter.\n");
> -        return NULL;
> -    }
> -
> -    klass = module_object_class_by_name(type);

object_new_with_props*() does not load from module. I don't think it
matters for now (module objects are not user-creatable?), but is this
intended?

-- 
Marc-André Lureau <marcandre.lureau@redhat.com>

Re: [PATCH 9/9] qom: drop user_creatable_add_type method

[Daniel P. Berrangé]

On Thu, Apr 30, 2026 at 01:40:15AM +0400, marcandre.lureau@redhat.com wrote:
> On Wed, 29 Apr 2026 22:08:30 +0100, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> > index e0a3cd8d0f7..7080f85f95f 100644
> > --- a/qom/object_interfaces.c
> > +++ b/qom/object_interfaces.c
> > @@ -44,75 +44,11 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
> > [ ... skip 14 lines ... ]
> > -        error_append_hint(errp, "Identifiers consist of letters, digits, "
> > -                          "'-', '.', '_', starting with a letter.\n");
> > -        return NULL;
> > -    }
> > -
> > -    klass = module_object_class_by_name(type);
> 
> object_new_with_props*() does not load from module. I don't think it
> matters for now (module objects are not user-creatable?), but is this
> intended?

No, that is an over-sight. This was not supposed to loose any
functionality. We should allow user creatable objects to be
put into modules.

With regards,
Daniel

Re: [PATCH 1/9] qom: add trace events for object/property lifecycle

[Daniel P. Berrangé]

On Thu, Apr 30, 2026 at 01:40:15AM +0400, marcandre.lureau@redhat.com wrote:
> On Wed, 29 Apr 2026 22:08:22 +0100, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > diff --git a/qom/object.c b/qom/object.c
> > index f981e270440..b99da7309db 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -714,6 +720,7 @@ static Object *object_new_with_type(Type type)
> >      object_initialize_with_type(obj, size, type);
> >      obj->free = obj_free;
> >  
> > +    trace_object_new(obj, obj ? obj->class->type->name : "(null)");
> 
> It looks like there should be an assert(obj != NULL) after qemu_memalign() instead.

qemu_memalign aborts on OOM.  This "obj ? .. : .." check was a mistake,
copy+pasted from the dynamic cast trace events.

With regards,
Daniel

Re: [PATCH 3/9] qom: make errp last param in methods taking va_list

[Philippe Mathieu-Daudé]

On 29/4/26 23:08, Daniel P. Berrangé wrote:
> object_new_with_props can't put 'errp' last due to the use of
> variadic arguments. That constraint does not apply to the use
> of va_list with object_new_with_propv, so follow normal practice
> with 'errp' placement.
> 
> The same rationale applies to object_set_propv.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qom/object.h            | 10 +++++-----
>   qom/object.c                    | 20 ++++++++++----------
>   tests/unit/check-qom-proplist.c |  4 ++--
>   3 files changed, 17 insertions(+), 17 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 4/9] qom: shorten name of object_set_properties_from_keyval

[Philippe Mathieu-Daudé]

On 29/4/26 23:08, Daniel P. Berrangé wrote:
> This matches the convention established by the object_set_props and
> object_set_propv methods.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qom/object.h    |  6 +++---
>   qom/object_interfaces.c | 12 ++++++------
>   system/qdev-monitor.c   |  4 ++--
>   system/vl.c             |  7 ++++---
>   4 files changed, 15 insertions(+), 14 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>