[PATCH V3 0/3] fast qom tree get

[PATCH V3 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: given a path, return a list of properties and values.
  * qom-list-getv: 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         0.402     0.230     0.029   0.143
qom-list-getv        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-getv 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-getv.

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-getv
  python: use qom-list-getv
  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          | 64 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 245 insertions(+), 18 deletions(-)

-- 
1.8.3.1

[PATCH V3 1/3] qom: qom-list-getv

[Steve Sistare]

Define the qom-list-getv command, which fetches all the properties and
values for a list of paths.  This is faster than qom-tree-get when
fetching a subset of the QOM tree.  See qom.json for details.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 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..c16c2dd 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-getv:
+#
+# 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 of @ObjectPropertiesValues.  Each element contains
+#     the properties of the corresponding element in @paths.
+#
+# Since 10.1
+##
+{ 'command': 'qom-list-getv',
+  '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..fa44ae7 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_getv(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 V3 2/3] python: use qom-list-getv

[Steve Sistare]

Use qom-list-getv 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..6b2f1ab 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_getv(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..4db97ba 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-getv
+    """
+    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_getv(self, paths) -> List[ObjectPropertiesValues]:
+        """
+        :return: a strongly typed list from the 'qom-list-getv' command.
+        """
+        rsp = self.qmp.cmd('qom-list-getv', paths=paths)
+        # qom-list-getv 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 V3 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>
---
 tests/qtest/qom-test.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
index 27d70bc..4defff1 100644
--- a/tests/qtest/qom-test.c
+++ b/tests/qtest/qom-test.c
@@ -11,11 +11,72 @@
 
 #include "qobject/qdict.h"
 #include "qobject/qlist.h"
+#include "qobject/qstring.h"
 #include "qemu/cutils.h"
 #include "libqtest.h"
 
 static int verbosity_level;
 
+static void test_getv(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-getv',"
+                              "  'arguments': %p }", args);
+    g_assert(response);
+    g_assert(qdict_haskey(response, "return"));
+    return_list = qobject_to(QList, qdict_get(response, "return"));
+
+    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-getv call */
+            test_getv(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,6 +146,7 @@ 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);
 
@@ -100,6 +162,8 @@ static void test_machine(gconstpointer data)
     }
 
     test_properties(qts, "/machine", true);
+    qlist_append_str(paths, "/machine");
+    test_getv(qts, paths);
 
     response = qtest_qmp(qts, "{ 'execute': 'quit' }");
     g_assert(qdict_haskey(response, "return"));
-- 
1.8.3.1

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

[Fabiano Rosas]

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

> Add a unit test for qom-list-getv.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH V3 1/3] qom: qom-list-getv

[Philippe Mathieu-Daudé]

On 8/7/25 19:24, Steve Sistare wrote:
> Define the qom-list-getv command, which fetches all the properties and
> values for a list of paths.  This is faster than qom-tree-get when
> fetching a subset of the QOM tree.  See qom.json for details.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   qapi/qom.json      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   qom/qom-qmp-cmds.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 103 insertions(+)

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

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

[Philippe Mathieu-Daudé]

Hi Steve,

On 8/7/25 19:24, Steve Sistare wrote:
> Add a unit test for qom-list-getv.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   tests/qtest/qom-test.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 64 insertions(+)
> 
> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
> index 27d70bc..4defff1 100644
> --- a/tests/qtest/qom-test.c
> +++ b/tests/qtest/qom-test.c
> @@ -11,11 +11,72 @@
>   
>   #include "qobject/qdict.h"
>   #include "qobject/qlist.h"
> +#include "qobject/qstring.h"
>   #include "qemu/cutils.h"
>   #include "libqtest.h"
>   
>   static int verbosity_level;
>   
> +static void test_getv(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) {

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

But I note this doesn't assert anything except if you use V=3 and
look at the output.

Maybe stick it to a particular machine and check for a particular
path and its properties?

> +        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-getv',"
> +                              "  'arguments': %p }", args);
> +    g_assert(response);
> +    g_assert(qdict_haskey(response, "return"));
> +    return_list = qobject_to(QList, qdict_get(response, "return"));
> +
> +    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-getv call */
> +            test_getv(qts, child_paths);
> +        }
> +
> +        path_entry = QTAILQ_NEXT(path_entry, next);
> +    }
> +}

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

[Markus Armbruster]

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

> Hi Steve,
>
> On 8/7/25 19:24, Steve Sistare wrote:
>> Add a unit test for qom-list-getv.
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>   tests/qtest/qom-test.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
>> index 27d70bc..4defff1 100644
>> --- a/tests/qtest/qom-test.c
>> +++ b/tests/qtest/qom-test.c
>> @@ -11,11 +11,72 @@
>>     #include "qobject/qdict.h"
>>   #include "qobject/qlist.h"
>> +#include "qobject/qstring.h"
>>   #include "qemu/cutils.h"
>>   #include "libqtest.h"
>>     static int verbosity_level;
>>   +static void test_getv(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) {
>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> But I note this doesn't assert anything except if you use V=3 and
> look at the output.
>
> Maybe stick it to a particular machine and check for a particular
> path and its properties?

Or create some suitable thing with -object, and get that.  No machine
dependence.

[...]

Re: [PATCH V3 1/3] qom: qom-list-getv

[Markus Armbruster]

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

> Define the qom-list-getv command, which fetches all the properties and
> values for a list of paths.  This is faster than qom-tree-get when
> fetching a subset of the QOM tree.  See qom.json for details.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

You cover letter explains *why* we want this.  Please include the
relevant parts here, so the rationale gets captured in git.

> ---
>  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..c16c2dd 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.

Best to consistently end the descriptions with a period.

> +#
> +# 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-getv:
> +#
> +# 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

John Snow's "[PATCH 00/18] QAPI: add cross-references to qapi docs"
rewrites things so they become links in generated HTML.  @qom-get
beccomes `qom-get`.  Please use `qom-get` to avoid the semantic
conflict.

> +#
> +# Errors:
> +#     - If any path is not valid or is ambiguous
> +#
> +# Returns: A list of @ObjectPropertiesValues.  Each element contains
> +#     the properties of the corresponding element in @paths.

I understand you patterned this after qom-get.  It comes out like

   Return:
      "[""ObjectPropertiesValues""]" -- A list of
      "ObjectPropertiesValues".  Each element contains the properties
      of the corresponding element in "paths".

in the generated manual.  'A list of "ObjectPropertiesValues"' is
redundant.  John Snow's "[PATCH v5 4/4] qapi: rephrase return docs to
avoid type name" cleans up existing instances, including qom-get.

Perhaps something like "A list where each element contains information
on the properties of the object referenced by the corresponding element
in @paths."  Or shorter: "A list where each element is the result for
the corresponding element of @paths".

> +#
> +# Since 10.1
> +##
> +{ 'command': 'qom-list-getv',
> +  'data': { 'paths': [ 'str' ] },
> +  'returns': [ 'ObjectPropertiesValues' ],
> +  'allow-preconfig': true }
> +
> +##
>  # @qom-set:
>  #
>  # This command will set a property from a object model path.

The schema looks good.  Not entirely happy with the names.  Naming is
hard!  Type names are not part of the interface, so let's not worry
about them too much.  The command name will be set in stone, though.

When you named the command qom-list-getv, you also had qom-list-get.
qom-list-get works on a single path, and -getv on multiple paths.  The
"v" suffix feels like a natural choice among people used to C.  But does
it make sense without its buddy?

How do you feel about calling it qom-list-get?  qom-get-many?  Other
ideas?

> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c

[...]

The C code looks good to me.

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

[Steven Sistare]

On 7/8/2025 6:02 PM, Philippe Mathieu-Daudé wrote:
> Hi Steve,
> 
> On 8/7/25 19:24, Steve Sistare wrote:
>> Add a unit test for qom-list-getv.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>   tests/qtest/qom-test.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
>> index 27d70bc..4defff1 100644
>> --- a/tests/qtest/qom-test.c
>> +++ b/tests/qtest/qom-test.c
>> @@ -11,11 +11,72 @@
>>   #include "qobject/qdict.h"
>>   #include "qobject/qlist.h"
>> +#include "qobject/qstring.h"
>>   #include "qemu/cutils.h"
>>   #include "libqtest.h"
>>   static int verbosity_level;
>> +static void test_getv(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) {
> 
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> But I note this doesn't assert anything except if you use V=3 and
> look at the output.

I don't follow.  It unconditionally traverses the whole tree and asserts
that properties are present.  Plus, for V >= 2, it prints paths.
It is patterned after test_properties() in the same file.

- Steve

> Maybe stick it to a particular machine and check for a particular
> path and its properties?
> 
>> +        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-getv',"
>> +                              "  'arguments': %p }", args);
>> +    g_assert(response);
>> +    g_assert(qdict_haskey(response, "return"));
>> +    return_list = qobject_to(QList, qdict_get(response, "return"));
>> +
>> +    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-getv call */
>> +            test_getv(qts, child_paths);
>> +        }
>> +
>> +        path_entry = QTAILQ_NEXT(path_entry, next);
>> +    }
>> +}

Re: [PATCH V3 1/3] qom: qom-list-getv

[Steven Sistare]

On 7/9/2025 4:39 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> Define the qom-list-getv command, which fetches all the properties and
>> values for a list of paths.  This is faster than qom-tree-get when
>> fetching a subset of the QOM tree.  See qom.json for details.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> You cover letter explains *why* we want this.  Please include the
> relevant parts here, so the rationale gets captured in git.

Sure, and I forgot to delete the reference to qom-tree-get:

   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.

>> ---
>>   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..c16c2dd 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.
> 
> Best to consistently end the descriptions with a period.

OK.

The majority of short descriptions in this file do not end with a period,
so I inferred that was the recommended style.

>> +#
>> +# 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-getv:
>> +#
>> +# 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
> 
> John Snow's "[PATCH 00/18] QAPI: add cross-references to qapi docs"
> rewrites things so they become links in generated HTML.  @qom-get
> beccomes `qom-get`.  Please use `qom-get` to avoid the semantic
> conflict.

OK.  I see his pending changes in this file.

>> +#
>> +# Errors:
>> +#     - If any path is not valid or is ambiguous
>> +#
>> +# Returns: A list of @ObjectPropertiesValues.  Each element contains
>> +#     the properties of the corresponding element in @paths.
> 
> I understand you patterned this after qom-get.  It comes out like
> 
>     Return:
>        "[""ObjectPropertiesValues""]" -- A list of
>        "ObjectPropertiesValues".  Each element contains the properties
>        of the corresponding element in "paths".
> 
> in the generated manual.  'A list of "ObjectPropertiesValues"' is
> redundant.  John Snow's "[PATCH v5 4/4] qapi: rephrase return docs to
> avoid type name" cleans up existing instances, including qom-get.
> 
> Perhaps something like "A list where each element contains information
> on the properties of the object referenced by the corresponding element
> in @paths."  Or shorter: "A list where each element is the result for
> the corresponding element of @paths".

OK, I'll use the latter.

>> +#
>> +# Since 10.1
>> +##
>> +{ 'command': 'qom-list-getv',
>> +  'data': { 'paths': [ 'str' ] },
>> +  'returns': [ 'ObjectPropertiesValues' ],
>> +  'allow-preconfig': true }
>> +
>> +##
>>   # @qom-set:
>>   #
>>   # This command will set a property from a object model path.
> 
> The schema looks good.  Not entirely happy with the names.  Naming is
> hard!  Type names are not part of the interface, so let's not worry
> about them too much.  The command name will be set in stone, though.
> 
> When you named the command qom-list-getv, you also had qom-list-get.
> qom-list-get works on a single path, and -getv on multiple paths.  The
> "v" suffix feels like a natural choice among people used to C.  But does
> it make sense without its buddy?
> 
> How do you feel about calling it qom-list-get?  qom-get-many?  Other
> ideas?

qom-list-get is good, as long as we never add the single-path version of the
call.  No reason to IMO.

In the cover letter I will rename the existing qom-list-get to qom-list-get-one
for the purpose of describing the experiments.

> 
>> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
> 
> [...]
> 
> The C code looks good to me.

Cool.

- Steve

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

[Philippe Mathieu-Daudé]

On 9/7/25 17:17, Steven Sistare wrote:
> On 7/8/2025 6:02 PM, Philippe Mathieu-Daudé wrote:
>> Hi Steve,
>>
>> On 8/7/25 19:24, Steve Sistare wrote:
>>> Add a unit test for qom-list-getv.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>>   tests/qtest/qom-test.c | 64 +++++++++++++++++++++++++++++++++++++++ 
>>> +++++++++++
>>>   1 file changed, 64 insertions(+)
>>>
>>> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
>>> index 27d70bc..4defff1 100644
>>> --- a/tests/qtest/qom-test.c
>>> +++ b/tests/qtest/qom-test.c
>>> @@ -11,11 +11,72 @@
>>>   #include "qobject/qdict.h"
>>>   #include "qobject/qlist.h"
>>> +#include "qobject/qstring.h"
>>>   #include "qemu/cutils.h"
>>>   #include "libqtest.h"
>>>   static int verbosity_level;
>>> +static void test_getv(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) {
>>
>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> But I note this doesn't assert anything except if you use V=3 and
>> look at the output.
> 
> I don't follow.  It unconditionally traverses the whole tree and asserts
> that properties are present.  Plus, for V >= 2, it prints paths.
> It is patterned after test_properties() in the same file.

Indeed, sorry. Good enough for me then, so:

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

Re: [PATCH V3 1/3] qom: qom-list-getv

[Philippe Mathieu-Daudé]

On 8/7/25 23:54, Philippe Mathieu-Daudé wrote:
> On 8/7/25 19:24, Steve Sistare wrote:
>> Define the qom-list-getv command, which fetches all the properties and
>> values for a list of paths.  This is faster than qom-tree-get when
>> fetching a subset of the QOM tree.  See qom.json for details.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>   qapi/qom.json      | 50 ++++++++++++++++++++++++++++++++++++++++++++ 
>> ++++++
>>   qom/qom-qmp-cmds.c | 53 ++++++++++++++++++++++++++++++++++++++++++++ 
>> +++++++++
>>   2 files changed, 103 insertions(+)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

and:

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

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

[Markus Armbruster]

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

> On 7/8/2025 6:02 PM, Philippe Mathieu-Daudé wrote:
>> Hi Steve,
>> On 8/7/25 19:24, Steve Sistare wrote:
>>> Add a unit test for qom-list-getv.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>>   tests/qtest/qom-test.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 64 insertions(+)
>>>
>>> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
>>> index 27d70bc..4defff1 100644
>>> --- a/tests/qtest/qom-test.c
>>> +++ b/tests/qtest/qom-test.c
>>> @@ -11,11 +11,72 @@
>>>   #include "qobject/qdict.h"
>>>   #include "qobject/qlist.h"
>>> +#include "qobject/qstring.h"
>>>   #include "qemu/cutils.h"
>>>   #include "libqtest.h"
>>>   static int verbosity_level;
>>> +static void test_getv(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) {
>>
>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> But I note this doesn't assert anything except if you use V=3 and
>> look at the output.
>
> I don't follow.  It unconditionally traverses the whole tree and asserts
> that properties are present.  Plus, for V >= 2, it prints paths.

It starts with path "/machine".

For each property returned for that path, it checks there is a name and
type.

It collects the paths where the type starts with "child<" into a list,
and recurses.  So it walks the entire tree under "/machine".

It does not examine property values at all.  This is a gap in testing.

It does not check properties against expectations.  The test would
silently pass if qom-list-getv always returned [], or if it always
returned crap types like "".  Another gap.

Checking all this for the entire tree is entirely impractical.  But we
could check it for a suitable path of our choice.  I recommend to avoid
going into machine-dependent weeds there.  Maybe create some suitable
thing with -object, and check that.

If you can't afford implementing this, document the gaps in a TODO
comment.

> It is patterned after test_properties() in the same file.

Same gaps :)

[...]

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

[Steven Sistare]

On 7/10/2025 4:54 AM, Markus Armbruster wrote:
> Steven Sistare <steven.sistare@oracle.com> writes:
> 
>> On 7/8/2025 6:02 PM, Philippe Mathieu-Daudé wrote:
>>> Hi Steve,
>>> On 8/7/25 19:24, Steve Sistare wrote:
>>>> Add a unit test for qom-list-getv.
>>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>>> ---
>>>>    tests/qtest/qom-test.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 64 insertions(+)
>>>>
>>>> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
>>>> index 27d70bc..4defff1 100644
>>>> --- a/tests/qtest/qom-test.c
>>>> +++ b/tests/qtest/qom-test.c
>>>> @@ -11,11 +11,72 @@
>>>>    #include "qobject/qdict.h"
>>>>    #include "qobject/qlist.h"
>>>> +#include "qobject/qstring.h"
>>>>    #include "qemu/cutils.h"
>>>>    #include "libqtest.h"
>>>>    static int verbosity_level;
>>>> +static void test_getv(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) {
>>>
>>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>> But I note this doesn't assert anything except if you use V=3 and
>>> look at the output.
>>
>> I don't follow.  It unconditionally traverses the whole tree and asserts
>> that properties are present.  Plus, for V >= 2, it prints paths.
> 
> It starts with path "/machine".
> 
> For each property returned for that path, it checks there is a name and
> type.
> 
> It collects the paths where the type starts with "child<" into a list,
> and recurses.  So it walks the entire tree under "/machine".
> 
> It does not examine property values at all.  This is a gap in testing.
> 
> It does not check properties against expectations.  The test would
> silently pass if qom-list-getv always returned [], or if it always
> returned crap types like "".  Another gap.

I will assert that the returned list is not empty.

> Checking all this for the entire tree is entirely impractical.  But we
> could check it for a suitable path of our choice.  I recommend to avoid
> going into machine-dependent weeds there.  Maybe create some suitable
> thing with -object, and check that.

OK.  I will keep the existing traversal test, and add a test for
an object's property value.

- Steve

> If you can't afford implementing this, document the gaps in a TODO
> comment.
> 
>> It is patterned after test_properties() in the same file.
> 
> Same gaps :)
> 
> [...]
>