qom: more efficient object property handling

Some users of QOM need to be able to iterate over properties defined against an object instance. Currently they are just directly using the QTAIL macros against the object properties data structure.

This is bad because it exposes them to changes in the data structure used to store properties, as well as changes in functionality such as ability to register properties against the class.

Providing an explicit object_property_foreach method provides a layer of insulation between the QOM user and the QOM internal implementation.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 include/qom/object.h | 23 +++++++++++++++++++++++
 qom/object.c         | 17 +++++++++++++++++
 2 files changed, 40 insertions(+)

include/qom/object.h +0 -0

index be7280c..71503af 100644

include/qom/object.h +4 -0

+++ b/qom/object.c

@@ -917,6 +917,23 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
     return NULL;
 }
 
+void object_property_foreach(Object *obj,
+                             ObjectPropertyIterator iter,
+                             Error **errp,

Quoted 25 lines · Daniel P. Berrange

Some users of QOM need to be able to iterate over properties defined against an object instance. Currently they are just directly using the QTAIL macros against the object properties data structure.

This is bad because it exposes them to changes in the data structure used to store properties, as well as changes in functionality such as ability to register properties against the class.

Providing an explicit object_property_foreach method provides a layer of insulation between the QOM user and the QOM internal implementation.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 include/qom/object.h | 23 +++++++++++++++++++++++
 qom/object.c         | 17 +++++++++++++++++
 2 files changed, 40 insertions(+)

include/qom/object.h +0 -0

index be7280c..71503af 100644

include/qom/object.h void object_property_del(Object *obj, const char *name, Error **errp); · line 966

Quoted 7 lines

include/qom/object.h +4 -0

@@ -960,6 +960,29 @@ void object_property_del(Object *obj, const char *name, Error **errp);
 ObjectProperty *object_property_find(Object *obj, const char *name,
                                      Error **errp);
 
+typedef void (*ObjectPropertyIterator)(Object *obj,
+                                       ObjectProperty *prop,
+                                       Error **errp,
+                                       void *opaque);

Do we want the iterator to be able to return a value, and possibly allow a non-zero value to abort iteration? [1]

include/qom/object.h void object_property_del(Object *obj, const char *name, Error **errp); · line 976

Quoted 10 lines

include/qom/object.h +10 -0

+
+/**
+ * object_property_foreach:
+ * @obj: the object
+ * @iter: the iterator callback function
+ * @errp: returns an error if iterator function fails
+ * @opaque: opaque data to pass to @iter
+ *
+ * Iterates over all properties defined against the object
+ * instance calling @iter for each property.

Probably should mention that there is an early exit if error gets set [2]

include/qom/object.h void object_property_del(Object *obj, const char *name, Error **errp); · line 984

Quoted 8 lines

include/qom/object.h +8 -0

+ *
+ * It is forbidden to modify the property list from @iter
+ * whether removing or adding properties.
+ */
+void object_property_foreach(Object *obj,
+                             ObjectPropertyIterator iter,
+                             Error **errp,
+                             void *opaque);

[1] if we allow the iterator to return a non-zero value to abort iteration (particularly if it wants to abort iteration without setting an error, just to save CPU cycles), should this foreach() function return that value?

Of course, a caller can always use opaque to achieve the same purpose, but it gets more verbose (the caller has to reserve space in their opaque for tracking whether an interesting exit value is needed, as well as having an early check on whether the value was already set in a previous visit) and burns more CPU cycles (the iterator runs to completion, even though the later callbacks are doing nothing).

Quoted 8 lines

include/qom/object.h +4 -0

+++ b/qom/object.c

@@ -917,6 +917,23 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
     return NULL;
 }
 
+void object_property_foreach(Object *obj,
+                             ObjectPropertyIterator iter,
+                             Error **errp,

include/qom/object.h ObjectProperty *object_property_find(Object *obj, const char *name, · line 932

Quoted 10 lines

include/qom/object.h +10 -0

+                             void *opaque)
+{
+    ObjectProperty *prop;
+    Error *local_err = NULL;
+
+    QTAILQ_FOREACH(prop, &obj->properties, node) {
+        iter(obj, prop, &local_err, opaque);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;

[2] there's the early exit if an error is set.

The code looks fine, but I think we need a documentation improvement for issue [2], and we may want a design change for issue [1] if a non-void return would be useful to any client later in the series.

Quoted 92 lines · Eric Blake

Quoted 25 lines · Daniel P. Berrange

Some users of QOM need to be able to iterate over properties defined against an object instance. Currently they are just directly using the QTAIL macros against the object properties data structure.

This is bad because it exposes them to changes in the data structure used to store properties, as well as changes in functionality such as ability to register properties against the class.

Providing an explicit object_property_foreach method provides a layer of insulation between the QOM user and the QOM internal implementation.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 include/qom/object.h | 23 +++++++++++++++++++++++
 qom/object.c         | 17 +++++++++++++++++
 2 files changed, 40 insertions(+)

include/qom/object.h +0 -0

index be7280c..71503af 100644

include/qom/object.h void object_property_del(Object *obj, const char *name, Error **errp); · line 966

Quoted 7 lines

include/qom/object.h +4 -0

@@ -960,6 +960,29 @@ void object_property_del(Object *obj, const char *name, Error **errp);
 ObjectProperty *object_property_find(Object *obj, const char *name,
                                      Error **errp);
 
+typedef void (*ObjectPropertyIterator)(Object *obj,
+                                       ObjectProperty *prop,
+                                       Error **errp,
+                                       void *opaque);

Do we want the iterator to be able to return a value, and possibly allow a non-zero value to abort iteration? [1]

include/qom/object.h void object_property_del(Object *obj, const char *name, Error **errp); · line 976

Quoted 10 lines

include/qom/object.h +10 -0

+
+/**
+ * object_property_foreach:
+ * @obj: the object
+ * @iter: the iterator callback function
+ * @errp: returns an error if iterator function fails
+ * @opaque: opaque data to pass to @iter
+ *
+ * Iterates over all properties defined against the object
+ * instance calling @iter for each property.

Probably should mention that there is an early exit if error gets set [2]

include/qom/object.h void object_property_del(Object *obj, const char *name, Error **errp); · line 984

Quoted 8 lines

include/qom/object.h +8 -0

+ *
+ * It is forbidden to modify the property list from @iter
+ * whether removing or adding properties.
+ */
+void object_property_foreach(Object *obj,
+                             ObjectPropertyIterator iter,
+                             Error **errp,
+                             void *opaque);

[1] if we allow the iterator to return a non-zero value to abort iteration (particularly if it wants to abort iteration without setting an error, just to save CPU cycles), should this foreach() function return that value?

Of course, a caller can always use opaque to achieve the same purpose, but it gets more verbose (the caller has to reserve space in their opaque for tracking whether an interesting exit value is needed, as well as having an early check on whether the value was already set in a previous visit) and burns more CPU cycles (the iterator runs to completion, even though the later callbacks are doing nothing).

Quoted 8 lines

include/qom/object.h +4 -0

+++ b/qom/object.c

@@ -917,6 +917,23 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
     return NULL;
 }
 
+void object_property_foreach(Object *obj,
+                             ObjectPropertyIterator iter,
+                             Error **errp,

include/qom/object.h ObjectProperty *object_property_find(Object *obj, const char *name, · line 932

Quoted 10 lines

include/qom/object.h +10 -0

+                             void *opaque)
+{
+    ObjectProperty *prop;
+    Error *local_err = NULL;
+
+    QTAILQ_FOREACH(prop, &obj->properties, node) {
+        iter(obj, prop, &local_err, opaque);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;

[2] there's the early exit if an error is set.

Quoted 3 lines

The code looks fine, but I think we need a documentation improvement for issue [2], and we may want a design change for issue [1] if a non-void return would be useful to any client later in the series.

We have quite a few _foreach-functions to help iterate over various things. They are easy enough to write, but I find them awkward to use.

Implementing bdrv_next() is no harder than bdrv_iterate(). Compare:

    BlockDriverState *bdrv_next(BlockDriverState *bs)
    {
        if (!bs) {
            return QTAILQ_FIRST(&bdrv_states);
        }
        return QTAILQ_NEXT(bs, device_list);
    }

    void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque)
    {
        BlockDriverState *bs;

        QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
            it(opaque, bs);
        }
    }

Higher-order functions are a wonderful tool if the language is equipped for them. In Lisp, for instance, you'd have everything in one place and no need for the awkward marshalling and unmarshalling of arguments, thanks to nested functions.

In C, stick to loops. That's what the language supports.

I don't think your example here is a reasonable comparison when you consider the full extent of this patch series. You are only having to iterate over a single data structure here. At the end of this patch series we have to iterate over multiple data structures spread across the object instance and class hierarchy, so writing a 'next' like method is not as trivial as you suggest with this comparison.

Quoted 6 lines

Higher-order functions are a wonderful tool if the language is equipped for them. In Lisp, for instance, you'd have everything in one place and no need for the awkward marshalling and unmarshalling of arguments, thanks to nested functions.

In C, stick to loops. That's what the language supports.

You're really arguing against use of function callbacks in general with this comparison to LISP. I don't think that's really practical in the real world, as any integration with event loop needs callbacks, which share al the same limitations as callbacks used with this foreach() style iterator. Given this I don't think banning use of callbacks in one specific scenario is really beneficial in general - its really just a personal style choice.

Quoted 14 lines · Daniel P. Berrange

I don't think your example here is a reasonable comparison when you consider the full extent of this patch series. You are only having to iterate over a single data structure here. At the end of this patch series we have to iterate over multiple data structures spread across the object instance and class hierarchy, so writing a 'next' like method is not as trivial as you suggest with this comparison.

Quoted 6 lines

Higher-order functions are a wonderful tool if the language is equipped for them. In Lisp, for instance, you'd have everything in one place and no need for the awkward marshalling and unmarshalling of arguments, thanks to nested functions.

In C, stick to loops. That's what the language supports.

Quoted 7 lines

You're really arguing against use of function callbacks in general with this comparison to LISP. I don't think that's really practical in the real world, as any integration with event loop needs callbacks, which share al the same limitations as callbacks used with this foreach() style iterator. Given this I don't think banning use of callbacks in one specific scenario is really beneficial in general - its really just a personal style choice.

I agree with Eric that in general loops are better than callbacks.

However, there are cases where iterators are just as awkward to write. Considering that we have very few iterations on properties, I think your patches are fine.

Quoted 5 lines · Markus Armbruster

We have quite a few _foreach-functions to help iterate over various things. They are easy enough to write, but I find them awkward to use.

Implementing bdrv_next() is no harder than bdrv_iterate(). Compare:

Quoted 16 lines

    BlockDriverState *bdrv_next(BlockDriverState *bs)
    {
        if (!bs) {
            return QTAILQ_FIRST(&bdrv_states);
        }
        return QTAILQ_NEXT(bs, device_list);
    }

    void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque)
    {
        BlockDriverState *bs;

        QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
            it(opaque, bs);
        }
    }

I don't think your example here is a reasonable comparison when you consider the full extent of this patch series. You are only having to iterate over a single data structure here. At the end of this patch series we have to iterate over multiple data structures spread across the object instance and class hierarchy, so writing a 'next' like method is not as trivial as you suggest with this comparison.

Rather than directly traversing the object property list, use the object_property_foreach iterator. This removes a dependancy on the implementation approach for properties.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 qmp.c | 103 ++++++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 59 insertions(+), 44 deletions(-)

Rather than directly traversing the object property list, use the object_property_foreach iterator. This removes a dependancy on the implementation approach for properties.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 vl.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

-----Original Message-----
From: Daniel P. Berrange [mailto:[email protected]]
Sent: Thursday, October 08, 2015 5:09 PM
To: [email protected]
Cc: Andreas Färber; Pavel Fedin; Daniel P. Berrange
Subject: [PATCH v3 4/5] qom: replace object property list with GHashTable

From: Pavel Fedin <[email protected]>

ARM GICv3 systems with large number of CPUs create lots of IRQ pins. Since every pin is represented as a property, number of these properties becomes very large. Every property add first makes sure there's no duplicates. Traversing the list becomes very slow, therefore qemu initialization takes significant time (several seconds for e. g. 16 CPUs).

This patch replaces list with GHashTable, making lookup very fast. The only drawback is that object_child_foreach() and object_child_foreach_recursive() cannot modify their objects during traversal, since GHashTableIter does not have modify-safe version. However, the code seems not to modify objects via these functions.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 include/qom/object.h | 10 ++++--
 qom/object.c         | 94 +++++++++++++++++++++++++++++++---------------------
 2 files changed, 63 insertions(+), 41 deletions(-)

include/qom/object.h +7 -3

index 71503af..7645f5f 100644
@@ -344,8 +344,6 @@ typedef struct ObjectProperty
     ObjectPropertyResolve *resolve;
     ObjectPropertyRelease *release;
     void *opaque;
-
-    QTAILQ_ENTRY(ObjectProperty) node;
 } ObjectProperty;
 
 /**

@@ -405,7 +403,7 @@ struct Object
     /*< private >*/
     ObjectClass *class;
     ObjectFree *free;
-    QTAILQ_HEAD(, ObjectProperty) properties;
+    GHashTable *properties;
     uint32_t ref;
     Object *parent;
 };

@@ -1511,6 +1509,9 @@ void object_property_set_description(Object *obj, const char *name,
  * Call @fn passing each child of @obj and @opaque to it, until @fn returns
  * non-zero.
  *
+ * It is forbidden to add or remove children from @obj from the @fn
+ * callback.
+ *
  * Returns: The last value returned by @fn, or 0 if there is no child.
  */
 int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),

@@ -1526,6 +1527,9 @@ int object_child_foreach(Object *obj, int (*fn)(Object *child, void
 *opaque),
  * non-zero. Calls recursively, all child nodes of @obj will also be passed
  * all the way down to the leaf nodes of the tree. Depth first ordering.
  *
+ * It is forbidden to add or remove children from @obj (or its
+ * child nodes) from the @fn callback.
+ *
  * Returns: The last value returned by @fn, or 0 if there is no child.
  */

int object_child_foreach_recursive(Object *obj,

qom/object.c +56 -39

index f8b27af..7cec1c9 100644
@@ -326,6 +326,16 @@ static void object_post_init_with_type(Object *obj, TypeImpl *ti)
     }
 }
 
+static void property_free(gpointer data)
+{
+    ObjectProperty *prop = data;
+
+    g_free(prop->name);
+    g_free(prop->type);
+    g_free(prop->description);
+    g_free(prop);
+}
+
 void object_initialize_with_type(void *data, size_t size, TypeImpl *type)
 {
     Object *obj = data;

@@ -340,7 +350,8 @@ void object_initialize_with_type(void *data, size_t size, TypeImpl *type)
     memset(obj, 0, type->instance_size);
     obj->class = type->class;
     object_ref(obj);
-    QTAILQ_INIT(&obj->properties);
+    obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal,
+                                            NULL, property_free);
     object_init_with_type(obj, type);
     object_post_init_with_type(obj, type);
 }

@@ -359,29 +370,35 @@ static inline bool object_property_is_child(ObjectProperty *prop)
 
 static void object_property_del_all(Object *obj)
 {
-    while (!QTAILQ_EMPTY(&obj->properties)) {
-        ObjectProperty *prop = QTAILQ_FIRST(&obj->properties);
-
-        QTAILQ_REMOVE(&obj->properties, prop, node);
+    ObjectProperty *prop;
+    GHashTableIter iter;
+    gpointer key, value;
 
+    g_hash_table_iter_init(&iter, obj->properties);
+    while (g_hash_table_iter_next(&iter, &key, &value)) {
+        prop = value;
         if (prop->release) {
             prop->release(obj, prop->name, prop->opaque);
         }
-
-        g_free(prop->name);
-        g_free(prop->type);
-        g_free(prop->description);
-        g_free(prop);
     }
+
+    g_hash_table_unref(obj->properties);
 }
 
 static void object_property_del_child(Object *obj, Object *child, Error **errp)
 {
     ObjectProperty *prop;
+    GHashTableIter iter;
+    gpointer key, value;
 
-    QTAILQ_FOREACH(prop, &obj->properties, node) {
+    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) {
-            object_property_del(obj, prop->name, errp);
+            if (prop->release) {
+                prop->release(obj, prop->name, prop->opaque);
+            }
+            g_hash_table_iter_remove(&iter);
             break;
         }
     }

@@ -779,10 +796,12 @@ static int do_object_child_foreach(Object *obj,
                                    int (*fn)(Object *child, void *opaque),
                                    void *opaque, bool recurse)
 {
-    ObjectProperty *prop, *next;
+    GHashTableIter iter;
+    ObjectProperty *prop;
     int ret = 0;
 
-    QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) {
+    g_hash_table_iter_init(&iter, obj->properties);
+    while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) {
         if (object_property_is_child(prop)) {
             Object *child = prop->opaque;
 

@@ -879,13 +898,11 @@ object_property_add(Object *obj, const char *name, const char *type,
         return ret;
     }
 
-    QTAILQ_FOREACH(prop, &obj->properties, node) {
-        if (strcmp(prop->name, name) == 0) {
-            error_setg(errp, "attempt to add duplicate property '%s'"
+    if (g_hash_table_contains(obj->properties, name)) {
+        error_setg(errp, "attempt to add duplicate property '%s'"
                        " to object (type '%s')", name,
                        object_get_typename(obj));
-            return NULL;
-        }
+        return NULL;
     }
 
     prop = g_malloc0(sizeof(*prop));

@@ -898,7 +915,7 @@ object_property_add(Object *obj, const char *name, const char *type,
     prop->release = release;
     prop->opaque = opaque;
 
-    QTAILQ_INSERT_TAIL(&obj->properties, prop, node);
+    g_hash_table_insert(obj->properties, prop->name, prop);
     return prop;
 }
 

@@ -907,10 +924,9 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
 {
     ObjectProperty *prop;
 
-    QTAILQ_FOREACH(prop, &obj->properties, node) {
-        if (strcmp(prop->name, name) == 0) {
-            return prop;
-        }
+    prop = g_hash_table_lookup(obj->properties, name);
+    if (prop) {
+        return prop;
     }
 
     error_setg(errp, "Property '.%s' not found", name);

@@ -922,11 +938,13 @@ void object_property_foreach(Object *obj,
                              Error **errp,
                              void *opaque)
 {
-    ObjectProperty *prop;
     Error *local_err = NULL;
+    GHashTableIter props;
+    gpointer key, value;
 
-    QTAILQ_FOREACH(prop, &obj->properties, node) {
-        iter(obj, prop, &local_err, opaque);
+    g_hash_table_iter_init(&props, obj->properties);
+    while (g_hash_table_iter_next(&props, &key, &value)) {
+        iter(obj, value, &local_err, opaque);
         if (local_err) {
             error_propagate(errp, local_err);
             return;

@@ -936,21 +954,17 @@ void object_property_foreach(Object *obj,
 
 void object_property_del(Object *obj, const char *name, Error **errp)
 {
-    ObjectProperty *prop = object_property_find(obj, name, errp);
-    if (prop == NULL) {
+    ObjectProperty *prop = g_hash_table_lookup(obj->properties, name);
+
+    if (!prop) {
+        error_setg(errp, "Property '.%s' not found", name);
         return;
     }
 
     if (prop->release) {
         prop->release(obj, name, prop->opaque);
     }
-
-    QTAILQ_REMOVE(&obj->properties, prop, node);
-
-    g_free(prop->name);
-    g_free(prop->type);
-    g_free(prop->description);
-    g_free(prop);
+    g_hash_table_remove(obj->properties, name);
 }
 
 void object_property_get(Object *obj, Visitor *v, const char *name,

@@ -1470,11 +1484,13 @@ void object_property_add_const_link(Object *obj, const char *name,
 gchar *object_get_canonical_path_component(Object *obj)
 {
     ObjectProperty *prop = NULL;
+    GHashTableIter iter;
 
     g_assert(obj);
     g_assert(obj->parent != NULL);
 
-    QTAILQ_FOREACH(prop, &obj->parent->properties, node) {
+    g_hash_table_iter_init(&iter, obj->parent->properties);
+    while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) {
         if (!object_property_is_child(prop)) {
             continue;
         }

@@ -1558,11 +1574,13 @@ static Object *object_resolve_partial_path(Object *parent,
                                               bool *ambiguous)
 {
     Object *obj;
+    GHashTableIter iter;
     ObjectProperty *prop;
 
     obj = object_resolve_abs_path(parent, parts, typename, 0);
 
-    QTAILQ_FOREACH(prop, &parent->properties, node) {
+    g_hash_table_iter_init(&iter, parent->properties);
+    while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) {
         Object *found;
 
         if (!object_property_is_child(prop)) {
--

2.4.3

From: Pavel Fedin <[email protected]>

ARM GICv3 systems with large number of CPUs create lots of IRQ pins. Since every pin is represented as a property, number of these properties becomes very large. Every property add first makes sure there's no duplicates. Traversing the list becomes very slow, therefore qemu initialization takes significant time (several seconds for e. g. 16 CPUs).

This patch replaces list with GHashTable, making lookup very fast. The only drawback is that object_child_foreach() and object_child_foreach_recursive() cannot modify their objects during traversal, since GHashTableIter does not have modify-safe version. However, the code seems not to modify objects via these functions.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 include/qom/object.h | 10 ++++--
 qom/object.c         | 94 +++++++++++++++++++++++++++++++---------------------
 2 files changed, 63 insertions(+), 41 deletions(-)

When there are many instances of a given class, registering properties against the instance is wasteful of resources. The majority of objects have a statically defined list of possible properties, so most of the properties are easily registerable against the class. Only those properties which are conditionally registered at runtime need be recorded against the klass.

Registering properties against classes also makes it possible to provide static introspection of QOM - currently introspection is only possible after creating an instance of a class, which severely limits its usefulness.

This impl only supports simple scalar properties. It does not attempt to allow child object / link object properties against the class. There are ways to support those too, but it would make this patch more complicated, so it is left as an exercise for the future.

There is no equivalent to object_property_del provided, since classes must be immutable once they are defined.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 include/qom/object.h |  47 +++++++++-
 qom/object.c         | 242 ++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 275 insertions(+), 14 deletions(-)

qom/object.c +0 -0

@@ -898,10 +903,11 @@ object_property_add(Object *obj, const char *name, const char *type,
         return ret;

This patch series is a combination of my own previous patch to add support for object properties against classes:

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05953.html

And Pavel Fedin's patch to use a hash table instead of list

https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg01455.html

I pulled Pavel's patch in to my series, since both our patches touch the same code and thus generate nasty merge conflicts.

In resolving these conflicts I decided we needed a new helper method object_property_foreach to deal with iteration over properties, hence we now have a short series of patches.

I made a few changes to Pavel's patch but the guts of it are still his work, so I left him as GIT author, but removed his Signed-off-by, since that would not apply to my own additions

Hi!

Just got test report from my colleague, he forgot to specify --target-list=aarch64-softmmu to configure and tried to build everything. Looks like there are more property iterations done by hand in PowerPC code:
--- cut ---
In file included from /home/igor/qemu/include/qom/object.h:20:0,

                  from /home/igor/qemu/include/hw/ppc/spapr_drc.h:15,
                  from /home/igor/qemu/hw/ppc/spapr_drc.c:13:
/home/igor/qemu/hw/ppc/spapr_drc.c: In function ‘spapr_drc_populate_dt’:
/home/igor/qemu/include/qemu/queue.h:419:29: error: request for member 
‘tqh_first’ in something not a structure or union
          for ((var) = ((head)->tqh_first);                               \
                              ^
/home/igor/qemu/hw/ppc/spapr_drc.c:683:5: note: in expansion of macro 
‘QTAILQ_FOREACH’
      QTAILQ_FOREACH(prop, &root_container->properties, node) {
      ^
/home/igor/qemu/include/qemu/queue.h:421:31: error: ‘ObjectProperty’ has 
no member named ‘node’
                  (var) = ((var)->field.tqe_next))
                                ^
/home/igor/qemu/hw/ppc/spapr_drc.c:683:5: note: in expansion of macro 
‘QTAILQ_FOREACH’
      QTAILQ_FOREACH(prop, &root_container->properties, node) {
      ^

/home/igor/qemu/rules.mak:57: recipe for target 'hw/ppc/spapr_drc.o' failed make[1]: *** [hw/ppc/spapr_drc.o] Error 1
Makefile:184: recipe for target 'subdir-ppc64-softmmu' failed --- cut ---

Actually, he was building version with my standalone patch, but i have checked your series, it also doesn't touch SPAPR code.