[PATCH for-9.0] qapi: Add 'recurse-children' option to qom-list

Alberto Garcia posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231124162443.124654-1-berto@igalia.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
qapi/qom.json      | 10 +++++++++-
qom/qom-hmp-cmds.c |  4 ++--
qom/qom-qmp-cmds.c | 22 +++++++++++++++++++++-
3 files changed, 32 insertions(+), 4 deletions(-)
[PATCH for-9.0] qapi: Add 'recurse-children' option to qom-list
Posted by Alberto Garcia 1 year ago
This allows returning a tree of all object properties under a given
path, in a way similar to scripts/qmp/qom-tree.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 qapi/qom.json      | 10 +++++++++-
 qom/qom-hmp-cmds.c |  4 ++--
 qom/qom-qmp-cmds.c | 22 +++++++++++++++++++++-
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index c53ef978ff..dfe3a20725 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -33,6 +33,10 @@
 #        qdev device type name.  Link properties form the device model
 #        graph.
 #
+# @children: if specified, a list of @ObjectPropertyInfo describing
+#     the child properties. This requires that this property's @type
+#     is of the form 'child<subtype>' (since 9.0)
+#
 # @description: if specified, the description of the property.
 #
 # @default-value: the default value, if any (since 5.0)
@@ -42,6 +46,7 @@
 { 'struct': 'ObjectPropertyInfo',
   'data': { 'name': 'str',
             'type': 'str',
+            '*children' :  [ 'ObjectPropertyInfo' ],
             '*description': 'str',
             '*default-value': 'any' } }
 
@@ -54,6 +59,9 @@
 # @path: the path within the object model.  See @qom-get for a
 #     description of this parameter.
 #
+# @recurse-children: if true, include the child properties recursively
+#     in the return value (default: false) (since 9.0)
+#
 # Returns: a list of @ObjectPropertyInfo that describe the properties
 #     of the object.
 #
@@ -69,7 +77,7 @@
 #                  { "name": "mon0", "type": "child<chardev-stdio>" } ] }
 ##
 { 'command': 'qom-list',
-  'data': { 'path': 'str' },
+  'data': { 'path': 'str', '*recurse-children': 'bool' },
   'returns': [ 'ObjectPropertyInfo' ],
   'allow-preconfig': true }
 
diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
index 6e3a2175a4..7592184fc3 100644
--- a/qom/qom-hmp-cmds.c
+++ b/qom/qom-hmp-cmds.c
@@ -28,7 +28,7 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    list = qmp_qom_list(path, &err);
+    list = qmp_qom_list(path, false, false, &err);
     if (err == NULL) {
         ObjectPropertyInfoList *start = list;
         while (list != NULL) {
@@ -206,7 +206,7 @@ void object_del_completion(ReadLineState *rs, int nb_args, const char *str)
     len = strlen(str);
     readline_set_completion_index(rs, len);
 
-    start = list = qmp_qom_list("/objects", NULL);
+    start = list = qmp_qom_list("/objects", false, false, NULL);
     while (list) {
         ObjectPropertyInfo *info = list->value;
 
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 7c087299de..5c9cb8a09c 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -28,10 +28,15 @@
 #include "qom/object_interfaces.h"
 #include "qom/qom-qobject.h"
 
-ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
+ObjectPropertyInfoList *qmp_qom_list(const char *path,
+                                     bool has_recurse_children,
+                                     bool recurse_children,
+                                     Error **errp)
 {
+    ERRP_GUARD();
     Object *obj;
     bool ambiguous = false;
+    bool recurse = has_recurse_children && recurse_children;
     ObjectPropertyInfoList *props = NULL;
     ObjectProperty *prop;
     ObjectPropertyIterator iter;
@@ -55,8 +60,23 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
 
         value->name = g_strdup(prop->name);
         value->type = g_strdup(prop->type);
+
+        if (recurse && g_str_has_prefix(prop->type, "child<")) {
+            ObjectPropertyInfoList *children;
+            g_autofree char *childpath = g_strdup_printf("%s/%s", path,
+                                                         prop->name);
+            children = qmp_qom_list(childpath, true, true, errp);
+            if (*errp) {
+                qapi_free_ObjectPropertyInfoList(props);
+                props = NULL;
+                goto out;
+            }
+            value->has_children = true;
+            value->children = children;
+        }
     }
 
+out:
     return props;
 }
 
-- 
2.39.2
Re: [PATCH for-9.0] qapi: Add 'recurse-children' option to qom-list
Posted by Markus Armbruster 11 months, 1 week ago
Alberto Garcia <berto@igalia.com> writes:

> This allows returning a tree of all object properties under a given
> path, in a way similar to scripts/qmp/qom-tree.

Use case?  We already have that script, and also HMP info qom-tree.

> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  qapi/qom.json      | 10 +++++++++-
>  qom/qom-hmp-cmds.c |  4 ++--
>  qom/qom-qmp-cmds.c | 22 +++++++++++++++++++++-
>  3 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index c53ef978ff..dfe3a20725 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -33,6 +33,10 @@
>  #        qdev device type name.  Link properties form the device model
>  #        graph.
>  #
> +# @children: if specified, a list of @ObjectPropertyInfo describing

Suggest "if present".

> +#     the child properties. This requires that this property's @type
> +#     is of the form 'child<subtype>' (since 9.0)

But when will it be present?

In qom-list-properties's return value: never.

In qom-list's return value: if and only if passed recurse-children=true.

Awkward to document.

> +#
>  # @description: if specified, the description of the property.
>  #
>  # @default-value: the default value, if any (since 5.0)
> @@ -42,6 +46,7 @@
>  { 'struct': 'ObjectPropertyInfo',
>    'data': { 'name': 'str',
>              'type': 'str',
> +            '*children' :  [ 'ObjectPropertyInfo' ],
>              '*description': 'str',
>              '*default-value': 'any' } }
>  
> @@ -54,6 +59,9 @@
>  # @path: the path within the object model.  See @qom-get for a
>  #     description of this parameter.
>  #
> +# @recurse-children: if true, include the child properties recursively
> +#     in the return value (default: false) (since 9.0)

Insufficiently clear on the connection to ObjectPropertyInfo member
@children, I'm afraid.

> +#
>  # Returns: a list of @ObjectPropertyInfo that describe the properties
>  #     of the object.
>  #
> @@ -69,7 +77,7 @@
>  #                  { "name": "mon0", "type": "child<chardev-stdio>" } ] }
>  ##
>  { 'command': 'qom-list',
> -  'data': { 'path': 'str' },
> +  'data': { 'path': 'str', '*recurse-children': 'bool' },
>    'returns': [ 'ObjectPropertyInfo' ],
>    'allow-preconfig': true }
>  

[...]
Re: [PATCH for-9.0] qapi: Add 'recurse-children' option to qom-list
Posted by Alberto Garcia 10 months, 2 weeks ago
On Fri 22 Dec 2023 11:31:12 AM +01, Markus Armbruster wrote:
>> This allows returning a tree of all object properties under a given
>> path, in a way similar to scripts/qmp/qom-tree.
>
> Use case?  We already have that script, and also HMP info qom-tree.

The main use case is convenience.

Management tools need to manually check that the type starts with
"child<" and recursively make a new QMP call. That's what e.g libvirt
does:

   https://gitlab.com/libvirt/libvirt/-/blob/v9.10.0/src/qemu/qemu_monitor_json.c?ref_type=tags#L7367

Parsing the output of HMP info qom-tree is not an option in that case.

Berto