[PATCH V4 0/3] fast qom tree get

[PATCH V4 0/3] fast qom tree get

[Steve Sistare]

Using qom-list and qom-get to get all the nodes and property values in a
QOM tree can take multiple seconds because it requires 1000's of individual
QOM requests.  Some managers fetch the entire tree or a large subset
of it when starting a new VM, and this cost is a substantial fraction of
start up time.

To reduce this cost, consider QAPI calls that fetch more information in
each call:
  * qom-list-get-one: given a path, return a list of properties and values.
  * qom-list-get: given a list of paths, return a list of properties and
    values for each path.
  * qom-tree-get: given a path, return all descendant nodes rooted at that
    path, with properties and values for each.

In all cases, a returned property is represented by ObjectPropertyValue,
with fields name, type, and value.  If an error occurs when reading a value
the value field is omitted.  Thus an error for one property will not cause a
bulk fetch operation to fail.

To evaluate each method, I modified scripts/qmp/qom-tree to use the method,
verified all methods produce the same output, and timed each using:

  qemu-system-x86_64 -display none \
    -chardev socket,id=monitor0,path=/tmp/vm1.sock,server=on,wait=off \
    -mon monitor0,mode=control &

  time qom-tree -s /tmp/vm1.sock > /dev/null

I only measured once per method, but the variation is low after a warm up run.
The 'real - user - sys' column is a proxy for QEMU CPU time.

method               real(s)   user(s)   sys(s)  (real - user - sys)(s)
qom-list / qom-get   2.048     0.932     0.057   1.059
qom-list-get-one     0.402     0.230     0.029   0.143
qom-list-get         0.200     0.132     0.015   0.053
qom-tree-get         0.143     0.123     0.012   0.008

qom-tree-get is the fastest, reducing elapsed time by a factor of 14X,
and reducing QEMU CPU time by 132X.

qom-list-get is slower when fetching the entire tree, but can beat
qom-tree-get when only a subset of the tree needs to be fetched (not shown).
To keep things simple, provide only qom-list-get.

Changes in V4:
  * renamed qom-list-getv -> qom-list-get
  * expanded the qtest to verify a value
  * cosmetic changes in the docs
  * expanded the qom-list-get commit message

Changes in V3:
  * dropped qom-tree-get
  * modified the qom-tree script to use qom-list-getv
  * cosmetic changes in the docs and code.

Changes in V2:
  * removed "qom: qom_resolve_path", which was pulled separately
  * dropped the error member
  * fixed missing _list_tree in qom.py
  * updated 10.0 to 10.1

Steve Sistare (3):
  qom: qom-list-get
  python: use qom-list-get
  tests/qtest/qom-test: unit test for qom-list-getv

 python/qemu/utils/qom.py        |  43 ++++++++-------
 python/qemu/utils/qom_common.py |  53 ++++++++++++++++++
 qapi/qom.json                   |  50 +++++++++++++++++
 qom/qom-qmp-cmds.c              |  53 ++++++++++++++++++
 tests/qtest/qom-test.c          | 116 +++++++++++++++++++++++++++++++++++++++-
 5 files changed, 296 insertions(+), 19 deletions(-)

-- 
1.8.3.1

[PATCH V4 1/3] qom: qom-list-get

[Steve Sistare]

Define the qom-list-get command, which fetches all the properties and
values for a list of paths.  This is faster than qom-list plus qom-get,
especially when fetching a large subset of the QOM tree.  Some managers
do so when starting a new VM, and this cost can be a substantial fraction
of start-up time.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 qapi/qom.json      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qom/qom-qmp-cmds.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/qapi/qom.json b/qapi/qom.json
index b133b06..49214d2 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -46,6 +46,34 @@
             '*default-value': 'any' } }
 
 ##
+# @ObjectPropertyValue:
+#
+# @name: the name of the property.
+#
+# @type: the type of the property, as described in @ObjectPropertyInfo.
+#
+# @value: the value of the property.  Absent when the property cannot
+#     be read.
+#
+# Since 10.1
+##
+{ 'struct': 'ObjectPropertyValue',
+  'data': { 'name': 'str',
+            'type': 'str',
+            '*value': 'any' } }
+
+##
+# @ObjectPropertiesValues:
+#
+# @properties: a list of properties.
+#
+# Since 10.1
+##
+{ 'struct': 'ObjectPropertiesValues',
+  'data': { 'properties': [ 'ObjectPropertyValue' ] }}
+
+
+##
 # @qom-list:
 #
 # This command will list any properties of a object given a path in
@@ -126,6 +154,28 @@
   'allow-preconfig': true }
 
 ##
+# @qom-list-get:
+#
+# List properties and their values for each object path in the input
+# list.
+#
+# @paths: The absolute or partial path for each object, as described
+#     in `qom-get`.
+#
+# Errors:
+#     - If any path is not valid or is ambiguous
+#
+# Returns: A list where each element is the result for the
+#     corresponding element of @paths.
+#
+# Since 10.1
+##
+{ 'command': 'qom-list-get',
+  'data': { 'paths': [ 'str' ] },
+  'returns': [ 'ObjectPropertiesValues' ],
+  'allow-preconfig': true }
+
+##
 # @qom-set:
 #
 # This command will set a property from a object model path.
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 293755f..57f1898 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -69,6 +69,59 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
     return props;
 }
 
+static void qom_list_add_property_value(Object *obj, ObjectProperty *prop,
+                                        ObjectPropertyValueList **props)
+{
+    ObjectPropertyValue *item = g_new0(ObjectPropertyValue, 1);
+
+    QAPI_LIST_PREPEND(*props, item);
+
+    item->name = g_strdup(prop->name);
+    item->type = g_strdup(prop->type);
+    item->value = object_property_get_qobject(obj, prop->name, NULL);
+}
+
+static ObjectPropertyValueList *qom_get_property_value_list(const char *path,
+                                                            Error **errp)
+{
+    Object *obj;
+    ObjectProperty *prop;
+    ObjectPropertyIterator iter;
+    ObjectPropertyValueList *props = NULL;
+
+    obj = qom_resolve_path(path, errp);
+    if (obj == NULL) {
+        return NULL;
+    }
+
+    object_property_iter_init(&iter, obj);
+    while ((prop = object_property_iter_next(&iter))) {
+        qom_list_add_property_value(obj, prop, &props);
+    }
+
+    return props;
+}
+
+ObjectPropertiesValuesList *qmp_qom_list_get(strList *paths, Error **errp)
+{
+    ObjectPropertiesValuesList *head = NULL, **tail = &head;
+    strList *path;
+
+    for (path = paths; path; path = path->next) {
+        ObjectPropertiesValues *item = g_new0(ObjectPropertiesValues, 1);
+
+        QAPI_LIST_APPEND(tail, item);
+
+        item->properties = qom_get_property_value_list(path->value, errp);
+        if (!item->properties) {
+            qapi_free_ObjectPropertiesValuesList(head);
+            return NULL;
+        }
+    }
+
+    return head;
+}
+
 void qmp_qom_set(const char *path, const char *property, QObject *value,
                  Error **errp)
 {
-- 
1.8.3.1

[PATCH V4 2/3] python: use qom-list-get

[Steve Sistare]

Use qom-list-get to speed up the qom-tree command.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 python/qemu/utils/qom.py        | 43 +++++++++++++++++++--------------
 python/qemu/utils/qom_common.py | 53 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 18 deletions(-)

diff --git a/python/qemu/utils/qom.py b/python/qemu/utils/qom.py
index 426a0f2..337ae29 100644
--- a/python/qemu/utils/qom.py
+++ b/python/qemu/utils/qom.py
@@ -224,28 +224,35 @@ def __init__(self, args: argparse.Namespace):
         super().__init__(args)
         self.path = args.path
 
-    def _list_node(self, path: str) -> None:
-        print(path)
-        items = self.qom_list(path)
-        for item in items:
-            if item.child:
-                continue
-            try:
-                rsp = self.qmp.cmd('qom-get', path=path,
-                                   property=item.name)
-                print(f"  {item.name}: {rsp} ({item.type})")
-            except ExecuteError as err:
-                print(f"  {item.name}: <EXCEPTION: {err!s}> ({item.type})")
-        print('')
-        for item in items:
-            if not item.child:
-                continue
+    def _list_nodes(self, paths: [str]) -> None:
+        all_paths_props = self.qom_list_get(paths)
+        i = 0
+
+        for props in all_paths_props:
+            path = paths[i]
+            i = i + 1
+            print(path)
             if path == '/':
                 path = ''
-            self._list_node(f"{path}/{item.name}")
+            newpaths = []
+
+            for item in props.properties:
+                if item.child:
+                    newpaths += [ f"{path}/{item.name}" ]
+                else:
+                    value = item.value
+                    if value == None:
+                        value = f"<EXCEPTION: property could not be read>"
+                    print(f"  {item.name}: {value} ({item.type})")
+
+            print('')
+
+            if newpaths:
+                self._list_nodes(newpaths)
+
 
     def run(self) -> int:
-        self._list_node(self.path)
+        self._list_nodes([self.path])
         return 0
 
 
diff --git a/python/qemu/utils/qom_common.py b/python/qemu/utils/qom_common.py
index dd2c8b1..e471f1d 100644
--- a/python/qemu/utils/qom_common.py
+++ b/python/qemu/utils/qom_common.py
@@ -65,6 +65,50 @@ def link(self) -> bool:
         return self.type.startswith('link<')
 
 
+class ObjectPropertyValue:
+    """
+    Represents a property return from e.g. qom-tree-get
+    """
+    def __init__(self, name: str, type_: str, value: object):
+        self.name = name
+        self.type = type_
+        self.value = value
+
+    @classmethod
+    def make(cls, value: Dict[str, Any]) -> 'ObjectPropertyValue':
+        """
+        Build an ObjectPropertyValue from a Dict with an unknown shape.
+        """
+        assert value.keys() >= {'name', 'type'}
+        assert value.keys() <= {'name', 'type', 'value'}
+        return cls(value['name'], value['type'], value.get('value'))
+
+    @property
+    def child(self) -> bool:
+        """Is this property a child property?"""
+        return self.type.startswith('child<')
+
+
+class ObjectPropertiesValues:
+    """
+    Represents the return type from e.g. qom-list-get
+    """
+    def __init__(self, properties):
+        self.properties = properties
+
+    @classmethod
+    def make(cls, value: Dict[str, Any]) -> 'ObjectPropertiesValues':
+        """
+        Build an ObjectPropertiesValues from a Dict with an unknown shape.
+        """
+        assert value.keys() == {'properties'}
+        props = [ObjectPropertyValue(item['name'],
+                                     item['type'],
+                                     item.get('value'))
+                 for item in value['properties']]
+        return cls(props)
+
+
 CommandT = TypeVar('CommandT', bound='QOMCommand')
 
 
@@ -145,6 +189,15 @@ def qom_list(self, path: str) -> List[ObjectPropertyInfo]:
         assert isinstance(rsp, list)
         return [ObjectPropertyInfo.make(x) for x in rsp]
 
+    def qom_list_get(self, paths) -> List[ObjectPropertiesValues]:
+        """
+        :return: a strongly typed list from the 'qom-list-get' command.
+        """
+        rsp = self.qmp.cmd('qom-list-get', paths=paths)
+        # qom-list-get returns List[ObjectPropertiesValues]
+        assert isinstance(rsp, list)
+        return [ObjectPropertiesValues.make(x) for x in rsp]
+
     @classmethod
     def command_runner(
             cls: Type[CommandT],
-- 
1.8.3.1

[PATCH V4 3/3] tests/qtest/qom-test: unit test for qom-list-getv

[Steve Sistare]

Add a unit test for qom-list-getv.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/qom-test.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 115 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
index 27d70bc..4ade1c7 100644
--- a/tests/qtest/qom-test.c
+++ b/tests/qtest/qom-test.c
@@ -11,11 +11,119 @@
 
 #include "qobject/qdict.h"
 #include "qobject/qlist.h"
+#include "qobject/qstring.h"
 #include "qemu/cutils.h"
 #include "libqtest.h"
 
+#define RAM_NAME "node0"
+#define RAM_SIZE 65536
+
 static int verbosity_level;
 
+/*
+ * Verify that the /object/RAM_NAME 'size' property is RAM_SIZE.
+ */
+static void test_list_get_value(QTestState *qts)
+{
+    QDict *args = qdict_new();
+    g_autoptr(QDict) response = NULL;
+    g_autoptr(QList) paths = qlist_new();
+    QListEntry *entry, *prop_entry;
+    const char *prop_name;
+    QList *properties, *return_list;
+    QDict *obj;
+
+    qlist_append_str(paths, "/objects/" RAM_NAME);
+    qdict_put_obj(args, "paths", QOBJECT(qlist_copy(paths)));
+    response = qtest_qmp(qts, "{ 'execute': 'qom-list-get',"
+                              "  'arguments': %p }", args);
+    g_assert(response);
+    g_assert(qdict_haskey(response, "return"));
+    return_list = qobject_to(QList, qdict_get(response, "return"));
+
+    entry = QTAILQ_FIRST(&return_list->head);
+    obj = qobject_to(QDict, qlist_entry_obj(entry));
+    g_assert(qdict_haskey(obj, "properties"));
+    properties = qobject_to(QList, qdict_get(obj, "properties"));
+
+    QLIST_FOREACH_ENTRY(properties, prop_entry) {
+        QDict *prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
+
+        g_assert(qdict_haskey(prop, "name"));
+        g_assert(qdict_haskey(prop, "value"));
+
+        prop_name = qdict_get_str(prop, "name");
+        if (!strcmp(prop_name, "type")) {
+            g_assert_cmpstr(qdict_get_str(prop, "value"), ==,
+                            "memory-backend-ram");
+
+        } else if (!strcmp(prop_name, "size")) {
+            g_assert_cmpint(qdict_get_int(prop, "value"), ==, RAM_SIZE);
+        }
+    }
+}
+
+static void test_list_get(QTestState *qts, QList *paths)
+{
+    QListEntry *entry, *prop_entry, *path_entry;
+    g_autoptr(QDict) response = NULL;
+    QDict *args = qdict_new();
+    QDict *prop;
+    QList *return_list;
+
+    if (verbosity_level >= 2) {
+        g_test_message("Obtaining properties for paths:");
+        QLIST_FOREACH_ENTRY(paths, path_entry) {
+            QString *qstr = qobject_to(QString, qlist_entry_obj(path_entry));
+            g_test_message("  %s", qstring_get_str(qstr));
+        }
+    }
+
+    qdict_put_obj(args, "paths", QOBJECT(qlist_copy(paths)));
+    response = qtest_qmp(qts, "{ 'execute': 'qom-list-get',"
+                              "  'arguments': %p }", args);
+    g_assert(response);
+    g_assert(qdict_haskey(response, "return"));
+    return_list = qobject_to(QList, qdict_get(response, "return"));
+    g_assert(!qlist_empty(return_list));
+
+    path_entry = QTAILQ_FIRST(&paths->head);
+    QLIST_FOREACH_ENTRY(return_list, entry) {
+        QDict *obj = qobject_to(QDict, qlist_entry_obj(entry));
+        g_assert(qdict_haskey(obj, "properties"));
+        QList *properties = qobject_to(QList, qdict_get(obj, "properties"));
+        bool has_child = false;
+
+        QLIST_FOREACH_ENTRY(properties, prop_entry) {
+            prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
+            g_assert(qdict_haskey(prop, "name"));
+            g_assert(qdict_haskey(prop, "type"));
+            has_child |= strstart(qdict_get_str(prop, "type"), "child<", NULL);
+        }
+
+        if (has_child) {
+            /* build a list of child paths */
+            QString *qstr = qobject_to(QString, qlist_entry_obj(path_entry));
+            const char *path = qstring_get_str(qstr);
+            g_autoptr(QList) child_paths = qlist_new();
+
+            QLIST_FOREACH_ENTRY(properties, prop_entry) {
+                prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
+                if (strstart(qdict_get_str(prop, "type"), "child<", NULL)) {
+                    g_autofree char *child_path = g_strdup_printf(
+                        "%s/%s", path, qdict_get_str(prop, "name"));
+                    qlist_append_str(child_paths, child_path);
+                }
+            }
+
+            /* fetch props for all children with one qom-list-get call */
+            test_list_get(qts, child_paths);
+        }
+
+        path_entry = QTAILQ_NEXT(path_entry, next);
+    }
+}
+
 static void test_properties(QTestState *qts, const char *path, bool recurse)
 {
     char *child_path;
@@ -85,8 +193,10 @@ static void test_machine(gconstpointer data)
     const char *machine = data;
     QDict *response;
     QTestState *qts;
+    g_autoptr(QList) paths = qlist_new();
 
-    qts = qtest_initf("-machine %s", machine);
+    qts = qtest_initf("-machine %s -object memory-backend-ram,id=%s,size=%d",
+                      machine, RAM_NAME, RAM_SIZE);
 
     if (g_test_slow()) {
         /* Make sure we can get the machine class properties: */
@@ -101,6 +211,10 @@ static void test_machine(gconstpointer data)
 
     test_properties(qts, "/machine", true);
 
+    qlist_append_str(paths, "/machine");
+    test_list_get(qts, paths);
+    test_list_get_value(qts);
+
     response = qtest_qmp(qts, "{ 'execute': 'quit' }");
     g_assert(qdict_haskey(response, "return"));
     qobject_unref(response);
-- 
1.8.3.1

Re: [PATCH V4 1/3] qom: qom-list-get

[Markus Armbruster]

Steve Sistare <steven.sistare@oracle.com> writes:

> Define the qom-list-get command, which fetches all the properties and
> values for a list of paths.  This is faster than qom-list plus qom-get,
> especially when fetching a large subset of the QOM tree.  Some managers
> do so when starting a new VM, and this cost can be a substantial fraction
> of start-up time.

You give such nice rationale in your cover letter...  What about this:

  Using qom-list and qom-get to get all the nodes and property values in
  a QOM tree can take multiple seconds because it requires 1000's of
  individual QOM requests.  Some managers fetch the entire tree or a
  large subset of it when starting a new VM, and this cost is a
  substantial fraction of start up time.

  Define the qom-list-get command, which fetches all the properties and
  values for a list of paths.  This can be much faster than qom-list
  plus qom-get.  When getting an entire QOM tree, I measured a 10x
  speedup in elapsed time.

> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I think you missed

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

> ---
>  qapi/qom.json      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  qom/qom-qmp-cmds.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index b133b06..49214d2 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -46,6 +46,34 @@
>              '*default-value': 'any' } }
>  
>  ##
> +# @ObjectPropertyValue:
> +#
> +# @name: the name of the property.
> +#
> +# @type: the type of the property, as described in @ObjectPropertyInfo.

`ObjectPropertyInfo`

See John Snow's "[PATCH 00/18] QAPI: add cross-references to qapi docs"
rewrites things so they become links in generated HTML.

> +#
> +# @value: the value of the property.  Absent when the property cannot
> +#     be read.
> +#
> +# Since 10.1
> +##
> +{ 'struct': 'ObjectPropertyValue',
> +  'data': { 'name': 'str',
> +            'type': 'str',
> +            '*value': 'any' } }
> +
> +##
> +# @ObjectPropertiesValues:
> +#
> +# @properties: a list of properties.
> +#
> +# Since 10.1
> +##
> +{ 'struct': 'ObjectPropertiesValues',
> +  'data': { 'properties': [ 'ObjectPropertyValue' ] }}
> +
> +
> +##
>  # @qom-list:
>  #
>  # This command will list any properties of a object given a path in
> @@ -126,6 +154,28 @@
>    'allow-preconfig': true }
>  
>  ##
> +# @qom-list-get:
> +#
> +# List properties and their values for each object path in the input
> +# list.
> +#
> +# @paths: The absolute or partial path for each object, as described
> +#     in `qom-get`.
> +#
> +# Errors:
> +#     - If any path is not valid or is ambiguous
> +#
> +# Returns: A list where each element is the result for the
> +#     corresponding element of @paths.
> +#
> +# Since 10.1
> +##
> +{ 'command': 'qom-list-get',
> +  'data': { 'paths': [ 'str' ] },
> +  'returns': [ 'ObjectPropertiesValues' ],
> +  'allow-preconfig': true }
> +
> +##
>  # @qom-set:
>  #
>  # This command will set a property from a object model path.
> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
> index 293755f..57f1898 100644
> --- a/qom/qom-qmp-cmds.c
> +++ b/qom/qom-qmp-cmds.c
> @@ -69,6 +69,59 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
>      return props;
>  }
>  
> +static void qom_list_add_property_value(Object *obj, ObjectProperty *prop,
> +                                        ObjectPropertyValueList **props)
> +{
> +    ObjectPropertyValue *item = g_new0(ObjectPropertyValue, 1);
> +
> +    QAPI_LIST_PREPEND(*props, item);
> +
> +    item->name = g_strdup(prop->name);
> +    item->type = g_strdup(prop->type);
> +    item->value = object_property_get_qobject(obj, prop->name, NULL);
> +}
> +
> +static ObjectPropertyValueList *qom_get_property_value_list(const char *path,
> +                                                            Error **errp)
> +{
> +    Object *obj;
> +    ObjectProperty *prop;
> +    ObjectPropertyIterator iter;
> +    ObjectPropertyValueList *props = NULL;
> +
> +    obj = qom_resolve_path(path, errp);
> +    if (obj == NULL) {
> +        return NULL;
> +    }
> +
> +    object_property_iter_init(&iter, obj);
> +    while ((prop = object_property_iter_next(&iter))) {
> +        qom_list_add_property_value(obj, prop, &props);
> +    }
> +
> +    return props;
> +}
> +
> +ObjectPropertiesValuesList *qmp_qom_list_get(strList *paths, Error **errp)
> +{
> +    ObjectPropertiesValuesList *head = NULL, **tail = &head;
> +    strList *path;
> +
> +    for (path = paths; path; path = path->next) {
> +        ObjectPropertiesValues *item = g_new0(ObjectPropertiesValues, 1);
> +
> +        QAPI_LIST_APPEND(tail, item);
> +
> +        item->properties = qom_get_property_value_list(path->value, errp);
> +        if (!item->properties) {
> +            qapi_free_ObjectPropertiesValuesList(head);
> +            return NULL;
> +        }
> +    }
> +
> +    return head;
> +}
> +
>  void qmp_qom_set(const char *path, const char *property, QObject *value,
>                   Error **errp)
>  {

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH V4 2/3] python: use qom-list-get

[Markus Armbruster]

Steve Sistare <steven.sistare@oracle.com> writes:

> Use qom-list-get to speed up the qom-tree command.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Quick test...  Differences in output before and after match expectations
(see appended diff).

New version:

    real    0m0.446s
    user    0m0.062s
    sys     0m0.017s

Old version barfs a stack backtrace (appended), and is ~18x slower:

    real    0m8.176s
    user    0m0.395s
    sys     0m0.126s

Did you see the stack backtrace, too?

Regardless
Acked-by: Markus Armbruster <armbru@redhat.com>



$ diff old new
59c59
<   rtc-time: {'tm_year': 125, 'tm_sec': 24, 'tm_hour': 14, 'tm_min': 39, 'tm_mon': 6, 'tm_mday': 11} (struct tm)
---
>   rtc-time: {'tm_year': 125, 'tm_sec': 28, 'tm_hour': 14, 'tm_min': 40, 'tm_mon': 6, 'tm_mday': 11} (struct tm)
486c486
<   date: {'tm_year': 125, 'tm_sec': 25, 'tm_hour': 14, 'tm_min': 39, 'tm_mon': 6, 'tm_mday': 11} (struct tm)
---
>   date: {'tm_year': 125, 'tm_sec': 28, 'tm_hour': 14, 'tm_min': 40, 'tm_mon': 6, 'tm_mday': 11} (struct tm)
832c832
<   legacy-memory: <EXCEPTION: Property 'qemu64-x86_64-cpu.legacy-memory' is not readable> (str)
---
>   legacy-memory: <EXCEPTION: property could not be read> (str)
1109c1109
<   crash-information: <EXCEPTION: No crash occurred> (GuestPanicInformation)
---
>   crash-information: <EXCEPTION: property could not be read> (GuestPanicInformation)
1554c1554
<   legacy-i8042: <EXCEPTION: Property 'vmmouse.legacy-i8042' is not readable> (str)
---
>   legacy-i8042: <EXCEPTION: property could not be read> (str)
2436c2436
<   legacy-iothread: <EXCEPTION: Property 'virtio-blk-device.legacy-iothread' is not readable> (str)
---
>   legacy-iothread: <EXCEPTION: property could not be read> (str)
2493c2493
<   legacy-iothread: <EXCEPTION: Property 'virtio-blk-device.legacy-iothread' is not readable> (str)
---
>   legacy-iothread: <EXCEPTION: property could not be read> (str)


Exception ignored in: <function QEMUMonitorProtocol.__del__ at 0x7fcfcd080d60>
Traceback (most recent call last):
  File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 310, in __del__
    self.close()
  File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 281, in close
    self._sync(
  File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 102, in _sync
    return self._aloop.run_until_complete(
  File "/usr/lib64/python3.13/asyncio/base_events.py", line 719, in run_until_complete
    return future.result()
  File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
    return await fut
  File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 399, in disconnect
    await self._wait_disconnect()
  File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 719, in _wait_disconnect
    await all_defined_tasks  # Raise Exceptions from the bottom half.
  File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 870, in _bh_loop_forever
    await async_fn()
RuntimeError: cannot reuse already awaited coroutine

Re: [PATCH V4 3/3] tests/qtest/qom-test: unit test for qom-list-getv

[Markus Armbruster]

Steve Sistare <steven.sistare@oracle.com> writes:

> Add a unit test for qom-list-getv.

qom-list-get here and in subject.  Could scratch this line, subject
suffices.

>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/qtest/qom-test.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 115 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
> index 27d70bc..4ade1c7 100644
> --- a/tests/qtest/qom-test.c
> +++ b/tests/qtest/qom-test.c
> @@ -11,11 +11,119 @@
>  
>  #include "qobject/qdict.h"
>  #include "qobject/qlist.h"
> +#include "qobject/qstring.h"
>  #include "qemu/cutils.h"
>  #include "libqtest.h"
>  
> +#define RAM_NAME "node0"
> +#define RAM_SIZE 65536
> +
>  static int verbosity_level;
>  
> +/*
> + * Verify that the /object/RAM_NAME 'size' property is RAM_SIZE.
> + */
> +static void test_list_get_value(QTestState *qts)
> +{
> +    QDict *args = qdict_new();
> +    g_autoptr(QDict) response = NULL;
> +    g_autoptr(QList) paths = qlist_new();
> +    QListEntry *entry, *prop_entry;
> +    const char *prop_name;
> +    QList *properties, *return_list;
> +    QDict *obj;
> +
> +    qlist_append_str(paths, "/objects/" RAM_NAME);
> +    qdict_put_obj(args, "paths", QOBJECT(qlist_copy(paths)));

Could probably avoid copying @paths.  Good enough.

> +    response = qtest_qmp(qts, "{ 'execute': 'qom-list-get',"
> +                              "  'arguments': %p }", args);
> +    g_assert(response);
> +    g_assert(qdict_haskey(response, "return"));
> +    return_list = qobject_to(QList, qdict_get(response, "return"));
> +
> +    entry = QTAILQ_FIRST(&return_list->head);
> +    obj = qobject_to(QDict, qlist_entry_obj(entry));
> +    g_assert(qdict_haskey(obj, "properties"));
> +    properties = qobject_to(QList, qdict_get(obj, "properties"));
> +
> +    QLIST_FOREACH_ENTRY(properties, prop_entry) {
> +        QDict *prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
> +
> +        g_assert(qdict_haskey(prop, "name"));
> +        g_assert(qdict_haskey(prop, "value"));
> +
> +        prop_name = qdict_get_str(prop, "name");
> +        if (!strcmp(prop_name, "type")) {
> +            g_assert_cmpstr(qdict_get_str(prop, "value"), ==,
> +                            "memory-backend-ram");
> +
> +        } else if (!strcmp(prop_name, "size")) {
> +            g_assert_cmpint(qdict_get_int(prop, "value"), ==, RAM_SIZE);
> +        }
> +    }
> +}
> +
> +static void test_list_get(QTestState *qts, QList *paths)
> +{
> +    QListEntry *entry, *prop_entry, *path_entry;
> +    g_autoptr(QDict) response = NULL;
> +    QDict *args = qdict_new();
> +    QDict *prop;
> +    QList *return_list;
> +
> +    if (verbosity_level >= 2) {
> +        g_test_message("Obtaining properties for paths:");
> +        QLIST_FOREACH_ENTRY(paths, path_entry) {
> +            QString *qstr = qobject_to(QString, qlist_entry_obj(path_entry));
> +            g_test_message("  %s", qstring_get_str(qstr));
> +        }
> +    }
> +
> +    qdict_put_obj(args, "paths", QOBJECT(qlist_copy(paths)));
> +    response = qtest_qmp(qts, "{ 'execute': 'qom-list-get',"
> +                              "  'arguments': %p }", args);
> +    g_assert(response);
> +    g_assert(qdict_haskey(response, "return"));
> +    return_list = qobject_to(QList, qdict_get(response, "return"));
> +    g_assert(!qlist_empty(return_list));
> +
> +    path_entry = QTAILQ_FIRST(&paths->head);
> +    QLIST_FOREACH_ENTRY(return_list, entry) {
> +        QDict *obj = qobject_to(QDict, qlist_entry_obj(entry));
> +        g_assert(qdict_haskey(obj, "properties"));
> +        QList *properties = qobject_to(QList, qdict_get(obj, "properties"));
> +        bool has_child = false;
> +
> +        QLIST_FOREACH_ENTRY(properties, prop_entry) {
> +            prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
> +            g_assert(qdict_haskey(prop, "name"));
> +            g_assert(qdict_haskey(prop, "type"));
> +            has_child |= strstart(qdict_get_str(prop, "type"), "child<", NULL);
> +        }
> +
> +        if (has_child) {
> +            /* build a list of child paths */
> +            QString *qstr = qobject_to(QString, qlist_entry_obj(path_entry));
> +            const char *path = qstring_get_str(qstr);
> +            g_autoptr(QList) child_paths = qlist_new();
> +
> +            QLIST_FOREACH_ENTRY(properties, prop_entry) {
> +                prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
> +                if (strstart(qdict_get_str(prop, "type"), "child<", NULL)) {
> +                    g_autofree char *child_path = g_strdup_printf(
> +                        "%s/%s", path, qdict_get_str(prop, "name"));
> +                    qlist_append_str(child_paths, child_path);
> +                }
> +            }
> +
> +            /* fetch props for all children with one qom-list-get call */
> +            test_list_get(qts, child_paths);
> +        }
> +
> +        path_entry = QTAILQ_NEXT(path_entry, next);
> +    }
> +}
> +
>  static void test_properties(QTestState *qts, const char *path, bool recurse)
>  {
>      char *child_path;
> @@ -85,8 +193,10 @@ static void test_machine(gconstpointer data)
>      const char *machine = data;
>      QDict *response;
>      QTestState *qts;
> +    g_autoptr(QList) paths = qlist_new();
>  
> -    qts = qtest_initf("-machine %s", machine);
> +    qts = qtest_initf("-machine %s -object memory-backend-ram,id=%s,size=%d",
> +                      machine, RAM_NAME, RAM_SIZE);
>  
>      if (g_test_slow()) {
>          /* Make sure we can get the machine class properties: */
> @@ -101,6 +211,10 @@ static void test_machine(gconstpointer data)
>  
>      test_properties(qts, "/machine", true);
>  
> +    qlist_append_str(paths, "/machine");
> +    test_list_get(qts, paths);
> +    test_list_get_value(qts);
> +
>      response = qtest_qmp(qts, "{ 'execute': 'quit' }");
>      g_assert(qdict_haskey(response, "return"));
>      qobject_unref(response);

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH V4 0/3] fast qom tree get

[Markus Armbruster]

This feels ready.  Have a look at my (few) comments, and tell me what
you think.  Happy to apply finishing touches in my tree, without another
respin.

Thanks your your patience!

Re: [PATCH V4 1/3] qom: qom-list-get

[Steven Sistare]

On 7/11/2025 10:35 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> Define the qom-list-get command, which fetches all the properties and
>> values for a list of paths.  This is faster than qom-list plus qom-get,
>> especially when fetching a large subset of the QOM tree.  Some managers
>> do so when starting a new VM, and this cost can be a substantial fraction
>> of start-up time.
> 
> You give such nice rationale in your cover letter...  What about this:
> 
>    Using qom-list and qom-get to get all the nodes and property values in
>    a QOM tree can take multiple seconds because it requires 1000's of
>    individual QOM requests.  Some managers fetch the entire tree or a
>    large subset of it when starting a new VM, and this cost is a
>    substantial fraction of start up time.
> 
>    Define the qom-list-get command, which fetches all the properties and
>    values for a list of paths.  This can be much faster than qom-list
>    plus qom-get.  When getting an entire QOM tree, I measured a 10x
>    speedup in elapsed time.

Will do.  I'll send a V5 with the remaining tweaks and RBs.

>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> I think you missed
> 
>    Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Indeed, and I appreciate the testing effort. Will add it.

>> ---
>>   qapi/qom.json      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qom/qom-qmp-cmds.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 103 insertions(+)
>>
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index b133b06..49214d2 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -46,6 +46,34 @@
>>               '*default-value': 'any' } }
>>   
>>   ##
>> +# @ObjectPropertyValue:
>> +#
>> +# @name: the name of the property.
>> +#
>> +# @type: the type of the property, as described in @ObjectPropertyInfo.
> 
> `ObjectPropertyInfo`
> 
> See John Snow's "[PATCH 00/18] QAPI: add cross-references to qapi docs"
> rewrites things so they become links in generated HTML.

Thanks, I missed this one.

>> +#
>> +# @value: the value of the property.  Absent when the property cannot
>> +#     be read.
>> +#
>> +# Since 10.1
>> +##
>> +{ 'struct': 'ObjectPropertyValue',
>> +  'data': { 'name': 'str',
>> +            'type': 'str',
>> +            '*value': 'any' } }
>> +
>> +##
>> +# @ObjectPropertiesValues:
>> +#
>> +# @properties: a list of properties.
>> +#
>> +# Since 10.1
>> +##
>> +{ 'struct': 'ObjectPropertiesValues',
>> +  'data': { 'properties': [ 'ObjectPropertyValue' ] }}
>> +
>> +
>> +##
>>   # @qom-list:
>>   #
>>   # This command will list any properties of a object given a path in
>> @@ -126,6 +154,28 @@
>>     'allow-preconfig': true }
>>   
>>   ##
>> +# @qom-list-get:
>> +#
>> +# List properties and their values for each object path in the input
>> +# list.
>> +#
>> +# @paths: The absolute or partial path for each object, as described
>> +#     in `qom-get`.
>> +#
>> +# Errors:
>> +#     - If any path is not valid or is ambiguous
>> +#
>> +# Returns: A list where each element is the result for the
>> +#     corresponding element of @paths.
>> +#
>> +# Since 10.1
>> +##
>> +{ 'command': 'qom-list-get',
>> +  'data': { 'paths': [ 'str' ] },
>> +  'returns': [ 'ObjectPropertiesValues' ],
>> +  'allow-preconfig': true }
>> +
>> +##
>>   # @qom-set:
>>   #
>>   # This command will set a property from a object model path.
>> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
>> index 293755f..57f1898 100644
>> --- a/qom/qom-qmp-cmds.c
>> +++ b/qom/qom-qmp-cmds.c
>> @@ -69,6 +69,59 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
>>       return props;
>>   }
>>   
>> +static void qom_list_add_property_value(Object *obj, ObjectProperty *prop,
>> +                                        ObjectPropertyValueList **props)
>> +{
>> +    ObjectPropertyValue *item = g_new0(ObjectPropertyValue, 1);
>> +
>> +    QAPI_LIST_PREPEND(*props, item);
>> +
>> +    item->name = g_strdup(prop->name);
>> +    item->type = g_strdup(prop->type);
>> +    item->value = object_property_get_qobject(obj, prop->name, NULL);
>> +}
>> +
>> +static ObjectPropertyValueList *qom_get_property_value_list(const char *path,
>> +                                                            Error **errp)
>> +{
>> +    Object *obj;
>> +    ObjectProperty *prop;
>> +    ObjectPropertyIterator iter;
>> +    ObjectPropertyValueList *props = NULL;
>> +
>> +    obj = qom_resolve_path(path, errp);
>> +    if (obj == NULL) {
>> +        return NULL;
>> +    }
>> +
>> +    object_property_iter_init(&iter, obj);
>> +    while ((prop = object_property_iter_next(&iter))) {
>> +        qom_list_add_property_value(obj, prop, &props);
>> +    }
>> +
>> +    return props;
>> +}
>> +
>> +ObjectPropertiesValuesList *qmp_qom_list_get(strList *paths, Error **errp)
>> +{
>> +    ObjectPropertiesValuesList *head = NULL, **tail = &head;
>> +    strList *path;
>> +
>> +    for (path = paths; path; path = path->next) {
>> +        ObjectPropertiesValues *item = g_new0(ObjectPropertiesValues, 1);
>> +
>> +        QAPI_LIST_APPEND(tail, item);
>> +
>> +        item->properties = qom_get_property_value_list(path->value, errp);
>> +        if (!item->properties) {
>> +            qapi_free_ObjectPropertiesValuesList(head);
>> +            return NULL;
>> +        }
>> +    }
>> +
>> +    return head;
>> +}
>> +
>>   void qmp_qom_set(const char *path, const char *property, QObject *value,
>>                    Error **errp)
>>   {
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Cool.  Thanks!

- Steve

Re: [PATCH V4 2/3] python: use qom-list-get

[Steven Sistare]

On 7/11/2025 10:47 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> Use qom-list-get to speed up the qom-tree command.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> Quick test...  Differences in output before and after match expectations
> (see appended diff).
> 
> New version:
> 
>      real    0m0.446s
>      user    0m0.062s
>      sys     0m0.017s
> 
> Old version barfs a stack backtrace (appended), and is ~18x slower:
> 
>      real    0m8.176s
>      user    0m0.395s
>      sys     0m0.126s
> 
> Did you see the stack backtrace, too?
> 
> Regardless
> Acked-by: Markus Armbruster <armbru@redhat.com>

Thanks.  Do you want a tested-by credit, or is that too little testing
for you to stand behind that endorsement?

I observe the same expected differences.

My test did not backtrace.  Must be a different VM.

- Steve

> $ diff old new
> 59c59
> <   rtc-time: {'tm_year': 125, 'tm_sec': 24, 'tm_hour': 14, 'tm_min': 39, 'tm_mon': 6, 'tm_mday': 11} (struct tm)
> ---
>>    rtc-time: {'tm_year': 125, 'tm_sec': 28, 'tm_hour': 14, 'tm_min': 40, 'tm_mon': 6, 'tm_mday': 11} (struct tm)
> 486c486
> <   date: {'tm_year': 125, 'tm_sec': 25, 'tm_hour': 14, 'tm_min': 39, 'tm_mon': 6, 'tm_mday': 11} (struct tm)
> ---
>>    date: {'tm_year': 125, 'tm_sec': 28, 'tm_hour': 14, 'tm_min': 40, 'tm_mon': 6, 'tm_mday': 11} (struct tm)
> 832c832
> <   legacy-memory: <EXCEPTION: Property 'qemu64-x86_64-cpu.legacy-memory' is not readable> (str)
> ---
>>    legacy-memory: <EXCEPTION: property could not be read> (str)
> 1109c1109
> <   crash-information: <EXCEPTION: No crash occurred> (GuestPanicInformation)
> ---
>>    crash-information: <EXCEPTION: property could not be read> (GuestPanicInformation)
> 1554c1554
> <   legacy-i8042: <EXCEPTION: Property 'vmmouse.legacy-i8042' is not readable> (str)
> ---
>>    legacy-i8042: <EXCEPTION: property could not be read> (str)
> 2436c2436
> <   legacy-iothread: <EXCEPTION: Property 'virtio-blk-device.legacy-iothread' is not readable> (str)
> ---
>>    legacy-iothread: <EXCEPTION: property could not be read> (str)
> 2493c2493
> <   legacy-iothread: <EXCEPTION: Property 'virtio-blk-device.legacy-iothread' is not readable> (str)
> ---
>>    legacy-iothread: <EXCEPTION: property could not be read> (str)
> 
> 
> Exception ignored in: <function QEMUMonitorProtocol.__del__ at 0x7fcfcd080d60>
> Traceback (most recent call last):
>    File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 310, in __del__
>      self.close()
>    File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 281, in close
>      self._sync(
>    File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 102, in _sync
>      return self._aloop.run_until_complete(
>    File "/usr/lib64/python3.13/asyncio/base_events.py", line 719, in run_until_complete
>      return future.result()
>    File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
>      return await fut
>    File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 399, in disconnect
>      await self._wait_disconnect()
>    File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 719, in _wait_disconnect
>      await all_defined_tasks  # Raise Exceptions from the bottom half.
>    File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 870, in _bh_loop_forever
>      await async_fn()
> RuntimeError: cannot reuse already awaited coroutine
> 

Re: [PATCH V4 3/3] tests/qtest/qom-test: unit test for qom-list-getv

[Steven Sistare]

On 7/11/2025 11:02 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> Add a unit test for qom-list-getv.
> 
> qom-list-get here and in subject.  Could scratch this line, subject
> suffices.

Doh, I missed it.  Will fix.

(re)names are hard :)

>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/qtest/qom-test.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 115 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
>> index 27d70bc..4ade1c7 100644
>> --- a/tests/qtest/qom-test.c
>> +++ b/tests/qtest/qom-test.c
>> @@ -11,11 +11,119 @@
>>   
>>   #include "qobject/qdict.h"
>>   #include "qobject/qlist.h"
>> +#include "qobject/qstring.h"
>>   #include "qemu/cutils.h"
>>   #include "libqtest.h"
>>   
>> +#define RAM_NAME "node0"
>> +#define RAM_SIZE 65536
>> +
>>   static int verbosity_level;
>>   
>> +/*
>> + * Verify that the /object/RAM_NAME 'size' property is RAM_SIZE.
>> + */
>> +static void test_list_get_value(QTestState *qts)
>> +{
>> +    QDict *args = qdict_new();
>> +    g_autoptr(QDict) response = NULL;
>> +    g_autoptr(QList) paths = qlist_new();
>> +    QListEntry *entry, *prop_entry;
>> +    const char *prop_name;
>> +    QList *properties, *return_list;
>> +    QDict *obj;
>> +
>> +    qlist_append_str(paths, "/objects/" RAM_NAME);
>> +    qdict_put_obj(args, "paths", QOBJECT(qlist_copy(paths)));
> 
> Could probably avoid copying @paths.  Good enough.

Thanks, but to wrap this up for both of us, I'll leave it.

- Steve

>> +    response = qtest_qmp(qts, "{ 'execute': 'qom-list-get',"
>> +                              "  'arguments': %p }", args);
>> +    g_assert(response);
>> +    g_assert(qdict_haskey(response, "return"));
>> +    return_list = qobject_to(QList, qdict_get(response, "return"));
>> +
>> +    entry = QTAILQ_FIRST(&return_list->head);
>> +    obj = qobject_to(QDict, qlist_entry_obj(entry));
>> +    g_assert(qdict_haskey(obj, "properties"));
>> +    properties = qobject_to(QList, qdict_get(obj, "properties"));
>> +
>> +    QLIST_FOREACH_ENTRY(properties, prop_entry) {
>> +        QDict *prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
>> +
>> +        g_assert(qdict_haskey(prop, "name"));
>> +        g_assert(qdict_haskey(prop, "value"));
>> +
>> +        prop_name = qdict_get_str(prop, "name");
>> +        if (!strcmp(prop_name, "type")) {
>> +            g_assert_cmpstr(qdict_get_str(prop, "value"), ==,
>> +                            "memory-backend-ram");
>> +
>> +        } else if (!strcmp(prop_name, "size")) {
>> +            g_assert_cmpint(qdict_get_int(prop, "value"), ==, RAM_SIZE);
>> +        }
>> +    }
>> +}
>> +
>> +static void test_list_get(QTestState *qts, QList *paths)
>> +{
>> +    QListEntry *entry, *prop_entry, *path_entry;
>> +    g_autoptr(QDict) response = NULL;
>> +    QDict *args = qdict_new();
>> +    QDict *prop;
>> +    QList *return_list;
>> +
>> +    if (verbosity_level >= 2) {
>> +        g_test_message("Obtaining properties for paths:");
>> +        QLIST_FOREACH_ENTRY(paths, path_entry) {
>> +            QString *qstr = qobject_to(QString, qlist_entry_obj(path_entry));
>> +            g_test_message("  %s", qstring_get_str(qstr));
>> +        }
>> +    }
>> +
>> +    qdict_put_obj(args, "paths", QOBJECT(qlist_copy(paths)));
>> +    response = qtest_qmp(qts, "{ 'execute': 'qom-list-get',"
>> +                              "  'arguments': %p }", args);
>> +    g_assert(response);
>> +    g_assert(qdict_haskey(response, "return"));
>> +    return_list = qobject_to(QList, qdict_get(response, "return"));
>> +    g_assert(!qlist_empty(return_list));
>> +
>> +    path_entry = QTAILQ_FIRST(&paths->head);
>> +    QLIST_FOREACH_ENTRY(return_list, entry) {
>> +        QDict *obj = qobject_to(QDict, qlist_entry_obj(entry));
>> +        g_assert(qdict_haskey(obj, "properties"));
>> +        QList *properties = qobject_to(QList, qdict_get(obj, "properties"));
>> +        bool has_child = false;
>> +
>> +        QLIST_FOREACH_ENTRY(properties, prop_entry) {
>> +            prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
>> +            g_assert(qdict_haskey(prop, "name"));
>> +            g_assert(qdict_haskey(prop, "type"));
>> +            has_child |= strstart(qdict_get_str(prop, "type"), "child<", NULL);
>> +        }
>> +
>> +        if (has_child) {
>> +            /* build a list of child paths */
>> +            QString *qstr = qobject_to(QString, qlist_entry_obj(path_entry));
>> +            const char *path = qstring_get_str(qstr);
>> +            g_autoptr(QList) child_paths = qlist_new();
>> +
>> +            QLIST_FOREACH_ENTRY(properties, prop_entry) {
>> +                prop = qobject_to(QDict, qlist_entry_obj(prop_entry));
>> +                if (strstart(qdict_get_str(prop, "type"), "child<", NULL)) {
>> +                    g_autofree char *child_path = g_strdup_printf(
>> +                        "%s/%s", path, qdict_get_str(prop, "name"));
>> +                    qlist_append_str(child_paths, child_path);
>> +                }
>> +            }
>> +
>> +            /* fetch props for all children with one qom-list-get call */
>> +            test_list_get(qts, child_paths);
>> +        }
>> +
>> +        path_entry = QTAILQ_NEXT(path_entry, next);
>> +    }
>> +}
>> +
>>   static void test_properties(QTestState *qts, const char *path, bool recurse)
>>   {
>>       char *child_path;
>> @@ -85,8 +193,10 @@ static void test_machine(gconstpointer data)
>>       const char *machine = data;
>>       QDict *response;
>>       QTestState *qts;
>> +    g_autoptr(QList) paths = qlist_new();
>>   
>> -    qts = qtest_initf("-machine %s", machine);
>> +    qts = qtest_initf("-machine %s -object memory-backend-ram,id=%s,size=%d",
>> +                      machine, RAM_NAME, RAM_SIZE);
>>   
>>       if (g_test_slow()) {
>>           /* Make sure we can get the machine class properties: */
>> @@ -101,6 +211,10 @@ static void test_machine(gconstpointer data)
>>   
>>       test_properties(qts, "/machine", true);
>>   
>> +    qlist_append_str(paths, "/machine");
>> +    test_list_get(qts, paths);
>> +    test_list_get_value(qts);
>> +
>>       response = qtest_qmp(qts, "{ 'execute': 'quit' }");
>>       g_assert(qdict_haskey(response, "return"));
>>       qobject_unref(response);
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

Re: [PATCH V4 0/3] fast qom tree get

[Steven Sistare]

On 7/11/2025 11:06 AM, Markus Armbruster wrote:
> This feels ready.  Have a look at my (few) comments, and tell me what
> you think.  Happy to apply finishing touches in my tree, without another
> respin.
> 
> Thanks your your patience!

I'll send V5 with the final changes.

Thank you for your time Markus.  The result is more polished due to your
careful review.

- Steve

Re: [PATCH V4 2/3] python: use qom-list-get

[Markus Armbruster]

Steven Sistare <steven.sistare@oracle.com> writes:

> On 7/11/2025 10:47 AM, Markus Armbruster wrote:
>> Steve Sistare <steven.sistare@oracle.com> writes:
>> 
>>> Use qom-list-get to speed up the qom-tree command.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> 
>> Quick test...  Differences in output before and after match expectations
>> (see appended diff).
>> 
>> New version:
>> 
>>      real    0m0.446s
>>      user    0m0.062s
>>      sys     0m0.017s
>> 
>> Old version barfs a stack backtrace (appended), and is ~18x slower:
>> 
>>      real    0m8.176s
>>      user    0m0.395s
>>      sys     0m0.126s
>> 
>> Did you see the stack backtrace, too?
>> 
>> Regardless
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>
> Thanks.  Do you want a tested-by credit, or is that too little testing
> for you to stand behind that endorsement?

Tested-by: Markus Armbruster <armbru@redhat.com>

> I observe the same expected differences.
>
> My test did not backtrace.  Must be a different VM.

Let's not worry about it.

[...]

Re: [PATCH V4 2/3] python: use qom-list-get

[Steven Sistare]

On 7/11/2025 12:50 PM, Markus Armbruster wrote:
> Steven Sistare <steven.sistare@oracle.com> writes:
> 
>> On 7/11/2025 10:47 AM, Markus Armbruster wrote:
>>> Steve Sistare <steven.sistare@oracle.com> writes:
>>>
>>>> Use qom-list-get to speed up the qom-tree command.
>>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>>
>>> Quick test...  Differences in output before and after match expectations
>>> (see appended diff).
>>>
>>> New version:
>>>
>>>       real    0m0.446s
>>>       user    0m0.062s
>>>       sys     0m0.017s
>>>
>>> Old version barfs a stack backtrace (appended), and is ~18x slower:
>>>
>>>       real    0m8.176s
>>>       user    0m0.395s
>>>       sys     0m0.126s
>>>
>>> Did you see the stack backtrace, too?
>>>
>>> Regardless
>>> Acked-by: Markus Armbruster <armbru@redhat.com>
>>
>> Thanks.  Do you want a tested-by credit, or is that too little testing
>> for you to stand behind that endorsement?
> 
> Tested-by: Markus Armbruster <armbru@redhat.com>

You can add that when you pull V5 into your tree - steve

> 
>> I observe the same expected differences.
>>
>> My test did not backtrace.  Must be a different VM.
> 
> Let's not worry about it.
> 
> [...]
> 

Re: [PATCH V4 2/3] python: use qom-list-get

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 3751 bytes --]

On Fri, Jul 11, 2025, 10:47 AM Markus Armbruster <armbru@redhat.com> wrote:

> Steve Sistare <steven.sistare@oracle.com> writes:
>
> > Use qom-list-get to speed up the qom-tree command.
> >
> > Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>
> Quick test...  Differences in output before and after match expectations
> (see appended diff).
>
> New version:
>
>     real    0m0.446s
>     user    0m0.062s
>     sys     0m0.017s
>
> Old version barfs a stack backtrace (appended), and is ~18x slower:
>
>     real    0m8.176s
>     user    0m0.395s
>     sys     0m0.126s
>
> Did you see the stack backtrace, too?
>
> Regardless
> Acked-by: Markus Armbruster <armbru@redhat.com>
>
>
>
> $ diff old new
> 59c59
> <   rtc-time: {'tm_year': 125, 'tm_sec': 24, 'tm_hour': 14, 'tm_min': 39,
> 'tm_mon': 6, 'tm_mday': 11} (struct tm)
> ---
> >   rtc-time: {'tm_year': 125, 'tm_sec': 28, 'tm_hour': 14, 'tm_min': 40,
> 'tm_mon': 6, 'tm_mday': 11} (struct tm)
> 486c486
> <   date: {'tm_year': 125, 'tm_sec': 25, 'tm_hour': 14, 'tm_min': 39,
> 'tm_mon': 6, 'tm_mday': 11} (struct tm)
> ---
> >   date: {'tm_year': 125, 'tm_sec': 28, 'tm_hour': 14, 'tm_min': 40,
> 'tm_mon': 6, 'tm_mday': 11} (struct tm)
> 832c832
> <   legacy-memory: <EXCEPTION: Property 'qemu64-x86_64-cpu.legacy-memory'
> is not readable> (str)
> ---
> >   legacy-memory: <EXCEPTION: property could not be read> (str)
> 1109c1109
> <   crash-information: <EXCEPTION: No crash occurred>
> (GuestPanicInformation)
> ---
> >   crash-information: <EXCEPTION: property could not be read>
> (GuestPanicInformation)
> 1554c1554
> <   legacy-i8042: <EXCEPTION: Property 'vmmouse.legacy-i8042' is not
> readable> (str)
> ---
> >   legacy-i8042: <EXCEPTION: property could not be read> (str)
> 2436c2436
> <   legacy-iothread: <EXCEPTION: Property
> 'virtio-blk-device.legacy-iothread' is not readable> (str)
> ---
> >   legacy-iothread: <EXCEPTION: property could not be read> (str)
> 2493c2493
> <   legacy-iothread: <EXCEPTION: Property
> 'virtio-blk-device.legacy-iothread' is not readable> (str)
> ---
> >   legacy-iothread: <EXCEPTION: property could not be read> (str)
>
>
> Exception ignored in: <function QEMUMonitorProtocol.__del__ at
> 0x7fcfcd080d60>
> Traceback (most recent call last):
>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
> line 310, in __del__
>     self.close()
>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
> line 281, in close
>     self._sync(
>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
> line 102, in _sync
>     return self._aloop.run_until_complete(
>   File "/usr/lib64/python3.13/asyncio/base_events.py", line 719, in
> run_until_complete
>     return future.result()
>   File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
>     return await fut
>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
> line 399, in disconnect
>     await self._wait_disconnect()
>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
> line 719, in _wait_disconnect
>     await all_defined_tasks  # Raise Exceptions from the bottom half.
>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
> line 870, in _bh_loop_forever
>     await async_fn()
> RuntimeError: cannot reuse already awaited coroutine
>

Curious about this backtrace. It looks like something has gone
fundamentally wrong in the internals and the error is being raised by the
garbage collector which is not ideal.

Can you help me reproduce this? Even if it's old/bad code, I don't want
python-qemu-qmp faulting like this.

>

[-- Attachment #2: Type: text/html, Size: 5190 bytes --]

Re: [PATCH V4 2/3] python: use qom-list-get

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Fri, Jul 11, 2025, 10:47 AM Markus Armbruster <armbru@redhat.com> wrote:

[...]

>> Exception ignored in: <function QEMUMonitorProtocol.__del__ at
>> 0x7fcfcd080d60>
>> Traceback (most recent call last):
>>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
>> line 310, in __del__
>>     self.close()
>>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
>> line 281, in close
>>     self._sync(
>>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
>> line 102, in _sync
>>     return self._aloop.run_until_complete(
>>   File "/usr/lib64/python3.13/asyncio/base_events.py", line 719, in
>> run_until_complete
>>     return future.result()
>>   File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
>>     return await fut
>>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
>> line 399, in disconnect
>>     await self._wait_disconnect()
>>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
>> line 719, in _wait_disconnect
>>     await all_defined_tasks  # Raise Exceptions from the bottom half.
>>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
>> line 870, in _bh_loop_forever
>>     await async_fn()
>> RuntimeError: cannot reuse already awaited coroutine
>>
>
> Curious about this backtrace. It looks like something has gone
> fundamentally wrong in the internals and the error is being raised by the
> garbage collector which is not ideal.
>
> Can you help me reproduce this? Even if it's old/bad code, I don't want
> python-qemu-qmp faulting like this.

Reproducer for Fedora 41, current master c079d3a31e4:

Run

    $ qemu-system-x86_64 -S -display none -chardev socket,id=mon1,path=test-qmp,server=on,wait=off -mon mode=control,id=qmp,chardev=mon1

and

    $ scripts/qmp/qom-tree -s test-qmp >/dev/null

Questions?

Re: [PATCH V4 2/3] python: use qom-list-get

[John Snow]

On Wed, Jul 16, 2025 at 4:32 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> John Snow <jsnow@redhat.com> writes:
>
> > On Fri, Jul 11, 2025, 10:47 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> [...]
>
> >> Exception ignored in: <function QEMUMonitorProtocol.__del__ at
> >> 0x7fcfcd080d60>
> >> Traceback (most recent call last):
> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
> >> line 310, in __del__
> >>     self.close()
> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
> >> line 281, in close
> >>     self._sync(
> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py",
> >> line 102, in _sync
> >>     return self._aloop.run_until_complete(
> >>   File "/usr/lib64/python3.13/asyncio/base_events.py", line 719, in
> >> run_until_complete
> >>     return future.result()
> >>   File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
> >>     return await fut
> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
> >> line 399, in disconnect
> >>     await self._wait_disconnect()
> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
> >> line 719, in _wait_disconnect
> >>     await all_defined_tasks  # Raise Exceptions from the bottom half.
> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py",
> >> line 870, in _bh_loop_forever
> >>     await async_fn()
> >> RuntimeError: cannot reuse already awaited coroutine
> >>
> >
> > Curious about this backtrace. It looks like something has gone
> > fundamentally wrong in the internals and the error is being raised by the
> > garbage collector which is not ideal.
> >
> > Can you help me reproduce this? Even if it's old/bad code, I don't want
> > python-qemu-qmp faulting like this.
>
> Reproducer for Fedora 41, current master c079d3a31e4:
>
> Run
>
>     $ qemu-system-x86_64 -S -display none -chardev socket,id=mon1,path=test-qmp,server=on,wait=off -mon mode=control,id=qmp,chardev=mon1
>
> and
>
>     $ scripts/qmp/qom-tree -s test-qmp >/dev/null
>
> Questions?
>

Doesn't seem to trigger for me on Fedora 42 from the c079d3a31e4
build. Is this a Python version difference thing rearing its head?

Re: [PATCH V4 2/3] python: use qom-list-get

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Wed, Jul 16, 2025 at 4:32 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Fri, Jul 11, 2025, 10:47 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> [...]
>>
>> >> Exception ignored in: <function QEMUMonitorProtocol.__del__ at 0x7fcfcd080d60>
>> >> Traceback (most recent call last):
>> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 310, in __del__
>> >>     self.close()
>> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 281, in close
>> >>     self._sync(
>> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 102, in _sync
>> >>     return self._aloop.run_until_complete(
>> >>   File "/usr/lib64/python3.13/asyncio/base_events.py", line 719, in run_until_complete
>> >>     return future.result()
>> >>   File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
>> >>     return await fut
>> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 399, in disconnect
>> >>     await self._wait_disconnect()
>> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 719, in _wait_disconnect
>> >>     await all_defined_tasks  # Raise Exceptions from the bottom half.
>> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 870, in _bh_loop_forever
>> >>     await async_fn()
>> >> RuntimeError: cannot reuse already awaited coroutine
>> >
>> > Curious about this backtrace. It looks like something has gone
>> > fundamentally wrong in the internals and the error is being raised by the
>> > garbage collector which is not ideal.
>> >
>> > Can you help me reproduce this? Even if it's old/bad code, I don't want
>> > python-qemu-qmp faulting like this.
>>
>> Reproducer for Fedora 41, current master c079d3a31e4:
>>
>> Run
>>
>>     $ qemu-system-x86_64 -S -display none -chardev socket,id=mon1,path=test-qmp,server=on,wait=off -mon mode=control,id=qmp,chardev=mon1
>>
>> and
>>
>>     $ scripts/qmp/qom-tree -s test-qmp >/dev/null
>>
>> Questions?
>>
>
> Doesn't seem to trigger for me on Fedora 42 from the c079d3a31e4
> build. Is this a Python version difference thing rearing its head?

I have no idea.

Is there anything else I can do to help you?

Re: [PATCH V4 2/3] python: use qom-list-get

[John Snow]

On Fri, Aug 8, 2025 at 2:28 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> John Snow <jsnow@redhat.com> writes:
>
> > On Wed, Jul 16, 2025 at 4:32 AM Markus Armbruster <armbru@redhat.com> wrote:
> >>
> >> John Snow <jsnow@redhat.com> writes:
> >>
> >> > On Fri, Jul 11, 2025, 10:47 AM Markus Armbruster <armbru@redhat.com> wrote:
> >>
> >> [...]
> >>
> >> >> Exception ignored in: <function QEMUMonitorProtocol.__del__ at 0x7fcfcd080d60>
> >> >> Traceback (most recent call last):
> >> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 310, in __del__
> >> >>     self.close()
> >> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 281, in close
> >> >>     self._sync(
> >> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/legacy.py", line 102, in _sync
> >> >>     return self._aloop.run_until_complete(
> >> >>   File "/usr/lib64/python3.13/asyncio/base_events.py", line 719, in run_until_complete
> >> >>     return future.result()
> >> >>   File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
> >> >>     return await fut
> >> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 399, in disconnect
> >> >>     await self._wait_disconnect()
> >> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 719, in _wait_disconnect
> >> >>     await all_defined_tasks  # Raise Exceptions from the bottom half.
> >> >>   File "/work/armbru/qemu/scripts/qmp/../../python/qemu/qmp/protocol.py", line 870, in _bh_loop_forever
> >> >>     await async_fn()
> >> >> RuntimeError: cannot reuse already awaited coroutine
> >> >
> >> > Curious about this backtrace. It looks like something has gone
> >> > fundamentally wrong in the internals and the error is being raised by the
> >> > garbage collector which is not ideal.
> >> >
> >> > Can you help me reproduce this? Even if it's old/bad code, I don't want
> >> > python-qemu-qmp faulting like this.
> >>
> >> Reproducer for Fedora 41, current master c079d3a31e4:
> >>
> >> Run
> >>
> >>     $ qemu-system-x86_64 -S -display none -chardev socket,id=mon1,path=test-qmp,server=on,wait=off -mon mode=control,id=qmp,chardev=mon1
> >>
> >> and
> >>
> >>     $ scripts/qmp/qom-tree -s test-qmp >/dev/null
> >>
> >> Questions?
> >>
> >
> > Doesn't seem to trigger for me on Fedora 42 from the c079d3a31e4
> > build. Is this a Python version difference thing rearing its head?
>
> I have no idea.
>
> Is there anything else I can do to help you?

Hm. Can you still reproduce this stack trace as of today?

If so, can you please try modifying
qemu.git/python/qemu/utils/qom_common.py and add "import logging" up
at the top, and modify the entry_point classmethod:

>       @classmethod
>      def entry_point(cls) -> int:
>      """
>          Build this command's parser, parse arguments, and run the command.
>
>          :return: `run`'s return code.
>          """
> +    logging.basicConfig(level=logging.DEBUG)
>      parser = argparse.ArgumentParser(description=cls.help)
>      cls.configure_parser(parser)
>      args = parser.parse_args()
>       return cls.command_runner(args)

And then try running the crashy version again, and send me the (quite
likely very long) output?

Note: I really oughtta add a formal --debug/DEBUG=1 configuration to
the QOM scripts so this is possible without code modification.