Re: [PATCH V2 1/5] qom: qom-tree-get
Steven Sistare writes:
> On 7/4/2025 8:22 AM, Markus Armbruster wrote:
>> Steve Sistare writes:
>>
>>> Define the qom-tree-get QAPI command, which fetches an entire tree of
>>> properties and values with a single QAPI call. This is much faster
>>> than using qom-list plus qom-get for every node and property of the
>>> tree. See qom.json for details.
>>>
>>> Signed-off-by: Steve Sistare
>>> ---
>>> qapi/qom.json | 56 ++
>>> qom/qom-qmp-cmds.c | 72
>>> ++
>>> 2 files changed, 128 insertions(+)
>>>
>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>> index 28ce24c..94662ad 100644
>>> --- a/qapi/qom.json
>>> +++ b/qapi/qom.json
>>> @@ -46,6 +46,38 @@
>>> '*default-value': 'any' } }
>>>
>>> ##
>>> +# @ObjectPropertyValue:
>>> +#
>>> +# @name: the name of the property
>>> +#
>>> +# @type: the type of the property, as described in @ObjectPropertyInfo
>>
>> That description is crap. In part because what it tries to describe is
>> crap. Neither is this patch's problem.
>>
>>> +#
>>> +# @value: the value of the property. Omitted if cannot be read.
>>
>> Suggest "Absent when the property cannot be read."
>
> OK.
>
>>> +#
>>> +# Since 10.1
>>> +##
>>> +{ 'struct': 'ObjectPropertyValue',
>>> + 'data': { 'name': 'str',
>>> +'type': 'str',
>>> +'*value': 'any' } }
>>
>> ObjectPropertyValue suggests this describes a property's value.
>
> I would agree with you if the name included "info" or "desc", but it
> does not. To me, "ObjectPropertyValue" says this is an object's
> property and value. But it's subjective.
Naming is hard.
> Perhaps: ObjectPropertyWithValue
I'd be tempted by ObjectProperty if it wasn't already taken by
qom/object.h.
Let's converge on the code, and maybe revisit naming at the end.
>> It does
>> not. It includes the name, i.e. it describes the *property*.
>>
>> So does ObjectPropertyInfo.
>>
>> The two overlap: both habe name and type. Only ObjectPropertyValue has
>> the current value. Only ObjectPropertyInfo has the default value and
>> description (I suspect the latter is useless in practice).
>>
>> ObjectPropertyInfo is used with qom-list and qom-list-properties.
>>
>> qom-list takes a QOM path, like your qom-tree-get and qom-list-getv.
>> I'd expect your commands to supersede qom-list in practice.
>>
>> qom-list-properties is unlike your qom-tree-get and qom-list-getv: it
>> takes a type name. It's unreliable for non-abstract types: it can miss
>> dynamically created properties.
>>
>> Let's ignore all this for now.
>>
>>> +
>>> +##
>>> +# @ObjectNode:
>>> +#
>>> +# @name: the name of the node
>>> +#
>>> +# @children: child nodes
>>> +#
>>> +# @properties: properties of the node
>>> +#
>>> +# Since 10.1
>>> +##
>>> +{ 'struct': 'ObjectNode',
>>> + 'data': { 'name': 'str',
>>> +'children': [ 'ObjectNode' ],
>>> +'properties': [ 'ObjectPropertyValue' ] }}
>>> +
>>> +##
>>> # @qom-list:
>>> #
>>> # This command will list any properties of a object given a path in
>>> @@ -126,6 +158,30 @@
>>> 'allow-preconfig': true }
>>>
>>> ##
>>> +# @qom-tree-get:
>>> +#
>>> +# This command returns a tree of objects and their properties,
>>> +# rooted at the specified path.
>>> +#
>>> +# @path: The absolute or partial path within the object model, as
>>> +# described in @qom-get
>>> +#
>>> +# Errors:
>>> +# - If path is not valid or is ambiguous, returns an error.
>>
>> By convention, we use "If , , where is a
>> member of QapiErrorClass.
>
> OK. I was following the minimal Errors examples from this same file.
Yup. I'll clean them up.
>> What are the possible error classes? As far as I can tell:
>>
>> - If path is ambiguous, GenericError
>> - If path cannot be resolved, DeviceNotFound
>>
>> However, use of error classes other than GenericError is strongly
>> discouraged (see error_set() in qapi/error.h).
>>
>> Is the ability to distinguish between these two errors useful?
>>
>> Existing related commands such as qom-get also use DeviceNotFound.
>> Entirely undocumented, exact error conditions unclear. Awesome.
>>
>> Libvirt seems to rely on this undocumented behavior: I can see code
>> checking for DeviceNotFound. Hyrum's law strikes.
>>
>> qom-get fails with DeviceNotFound in both of the above cases. It fails
>> with GenericError when @property doesn't exist or cannot be read. Your
>> qom-tree-get fails differently. Awesome again.
>>
>> Choices:
>>
>> 1. Leave errors undocumented and inconsistent.
>>
>> 2. Document errors for all related commands. Make the new ones as
>> consistent as we can.
>
> Ignoring qom-tree-get since we are dropping it.
>
> Do you prefer that qom-list-getv be consistent with qom-list (GenericError
> and DeviceNotFound, as created by the common subroutine qom_resolve_path),
> or only return GenericError with a customized message per be
Re: [PATCH V2 1/5] qom: qom-tree-get
On 12/5/25 15:47, Steve Sistare wrote: Define the qom-tree-get QAPI command, which fetches an entire tree of properties and values with a single QAPI call. This is much faster than using qom-list plus qom-get for every node and property of the tree. See qom.json for details. Signed-off-by: Steve Sistare --- qapi/qom.json | 56 ++ qom/qom-qmp-cmds.c | 72 ++ 2 files changed, 128 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH V2 1/5] qom: qom-tree-get
On 7/8/2025 3:14 AM, Philippe Mathieu-Daudé wrote: On 12/5/25 15:47, Steve Sistare wrote: Define the qom-tree-get QAPI command, which fetches an entire tree of properties and values with a single QAPI call. This is much faster than using qom-list plus qom-get for every node and property of the tree. See qom.json for details. Signed-off-by: Steve Sistare --- qapi/qom.json | 56 ++ qom/qom-qmp-cmds.c | 72 ++ 2 files changed, 128 insertions(+) Reviewed-by: Philippe Mathieu-Daudé Hi Philippe, thank you for reviewing this. Markus has requested that I drop qom-tree-get, and only provide qom-list-getv, to simplify things. Do you prefer that we keep qom-tree-get? - Steve
Re: [PATCH V2 1/5] qom: qom-tree-get
Markus Armbruster writes: > Steven Sistare writes: > >> On 7/4/2025 8:22 AM, Markus Armbruster wrote: >>> Steve Sistare writes: >>> Define the qom-tree-get QAPI command, which fetches an entire tree of properties and values with a single QAPI call. This is much faster than using qom-list plus qom-get for every node and property of the tree. See qom.json for details. Signed-off-by: Steve Sistare --- qapi/qom.json | 56 ++ qom/qom-qmp-cmds.c | 72 ++ 2 files changed, 128 insertions(+) diff --git a/qapi/qom.json b/qapi/qom.json index 28ce24c..94662ad 100644 --- a/qapi/qom.json +++ b/qapi/qom.json [...] ## +# @qom-tree-get: +# +# This command returns a tree of objects and their properties, +# rooted at the specified path. +# +# @path: The absolute or partial path within the object model, as +# described in @qom-get +# +# Errors: +# - If path is not valid or is ambiguous, returns an error. >>> >>> By convention, we use "If , , where is a >>> member of QapiErrorClass. >> >> OK. I was following the minimal Errors examples from this same file. > > Yup. I'll clean them up. I changed my mind. Omitting ", " is fairly common, actually. I don't feel like chasing down the actual error classes. Moreover, documenting error classes we don't want people to use seems counterproductive. Feel free to just delete ", returns an error." and call it a day. [...]
Re: [PATCH V2 1/5] qom: qom-tree-get
On 8/7/25 13:50, Steven Sistare wrote: On 7/8/2025 3:14 AM, Philippe Mathieu-Daudé wrote: On 12/5/25 15:47, Steve Sistare wrote: Define the qom-tree-get QAPI command, which fetches an entire tree of properties and values with a single QAPI call. This is much faster than using qom-list plus qom-get for every node and property of the tree. See qom.json for details. Signed-off-by: Steve Sistare --- qapi/qom.json | 56 ++ qom/qom-qmp-cmds.c | 72 +++ +++ 2 files changed, 128 insertions(+) Reviewed-by: Philippe Mathieu-Daudé Hi Philippe, thank you for reviewing this. Markus has requested that I drop qom-tree-get, and only provide qom- list-getv, to simplify things. Do you prefer that we keep qom-tree-get? Doh I missed that, I only noticed his "QOM maintainers please review" at the end. If you already addressed Markus comments, please respin to get a chance to get it merged for 10.1, otherwise I'll try to look at getv in the next days. Regards, Phil.
Re: [PATCH V2 1/5] qom: qom-tree-get
On 7/4/2025 8:22 AM, Markus Armbruster wrote:
Steve Sistare writes:
Define the qom-tree-get QAPI command, which fetches an entire tree of
properties and values with a single QAPI call. This is much faster
than using qom-list plus qom-get for every node and property of the
tree. See qom.json for details.
Signed-off-by: Steve Sistare
---
qapi/qom.json | 56 ++
qom/qom-qmp-cmds.c | 72 ++
2 files changed, 128 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 28ce24c..94662ad 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -46,6 +46,38 @@
'*default-value': 'any' } }
##
+# @ObjectPropertyValue:
+#
+# @name: the name of the property
+#
+# @type: the type of the property, as described in @ObjectPropertyInfo
That description is crap. In part because what it tries to describe is
crap. Neither is this patch's problem.
+#
+# @value: the value of the property. Omitted if cannot be read.
Suggest "Absent when the property cannot be read."
OK.
+#
+# Since 10.1
+##
+{ 'struct': 'ObjectPropertyValue',
+ 'data': { 'name': 'str',
+'type': 'str',
+'*value': 'any' } }
ObjectPropertyValue suggests this describes a property's value.
I would agree with you if the name included "info" or "desc", but it
does not. To me, "ObjectPropertyValue" says this is an object's
property and value. But it's subjective.
Perhaps: ObjectPropertyWithValue
It does
not. It includes the name, i.e. it describes the *property*.
So does ObjectPropertyInfo.
The two overlap: both habe name and type. Only ObjectPropertyValue has
the current value. Only ObjectPropertyInfo has the default value and
description (I suspect the latter is useless in practice).
ObjectPropertyInfo is used with qom-list and qom-list-properties.
qom-list takes a QOM path, like your qom-tree-get and qom-list-getv.
I'd expect your commands to supersede qom-list in practice.
qom-list-properties is unlike your qom-tree-get and qom-list-getv: it
takes a type name. It's unreliable for non-abstract types: it can miss
dynamically created properties.
Let's ignore all this for now.
+
+##
+# @ObjectNode:
+#
+# @name: the name of the node
+#
+# @children: child nodes
+#
+# @properties: properties of the node
+#
+# Since 10.1
+##
+{ 'struct': 'ObjectNode',
+ 'data': { 'name': 'str',
+'children': [ 'ObjectNode' ],
+'properties': [ 'ObjectPropertyValue' ] }}
+
+##
# @qom-list:
#
# This command will list any properties of a object given a path in
@@ -126,6 +158,30 @@
'allow-preconfig': true }
##
+# @qom-tree-get:
+#
+# This command returns a tree of objects and their properties,
+# rooted at the specified path.
+#
+# @path: The absolute or partial path within the object model, as
+# described in @qom-get
+#
+# Errors:
+# - If path is not valid or is ambiguous, returns an error.
By convention, we use "If , , where is a
member of QapiErrorClass.
OK. I was following the minimal Errors examples from this same file.
What are the possible error classes? As far as I can tell:
- If path is ambiguous, GenericError
- If path cannot be resolved, DeviceNotFound
However, use of error classes other than GenericError is strongly
discouraged (see error_set() in qapi/error.h).
Is the ability to distinguish between these two errors useful?
Existing related commands such as qom-get also use DeviceNotFound.
Entirely undocumented, exact error conditions unclear. Awesome.
Libvirt seems to rely on this undocumented behavior: I can see code
checking for DeviceNotFound. Hyrum's law strikes.
qom-get fails with DeviceNotFound in both of the above cases. It fails
with GenericError when @property doesn't exist or cannot be read. Your
qom-tree-get fails differently. Awesome again.
Choices:
1. Leave errors undocumented and inconsistent.
2. Document errors for all related commands. Make the new ones as
consistent as we can.
Ignoring qom-tree-get since we are dropping it.
Do you prefer that qom-list-getv be consistent with qom-list (GenericError
and DeviceNotFound, as created by the common subroutine qom_resolve_path),
or only return GenericError with a customized message per best practices?
(Regardless, it will still succeed when @property cannot be read).
+# - If a property cannot be read, the value field is omitted in
+# the corresponding @ObjectPropertyValue.
This is not an error, and therefore doesn't belong here.
ObjectPropertyValue's documentation also mentions it. Good enough?
OK.
+#
+# Returns: A tree of @ObjectNode. Each node contains its name, list
+# of properties, and list of child nodes.
Hmm.
A struct Object has no name. Only properties have a name.
An ObjectNode has a name, and an ObjectPropertyValue has a name.
I may get back to this in a later message.
+#
+# Since 10.1
+##
+{ 'command':
Re: [PATCH V2 1/5] qom: qom-tree-get
Steve Sistare writes:
> Define the qom-tree-get QAPI command, which fetches an entire tree of
> properties and values with a single QAPI call. This is much faster
> than using qom-list plus qom-get for every node and property of the
> tree. See qom.json for details.
>
> Signed-off-by: Steve Sistare
> ---
> qapi/qom.json | 56 ++
> qom/qom-qmp-cmds.c | 72
> ++
> 2 files changed, 128 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 28ce24c..94662ad 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -46,6 +46,38 @@
> '*default-value': 'any' } }
>
> ##
> +# @ObjectPropertyValue:
> +#
> +# @name: the name of the property
> +#
> +# @type: the type of the property, as described in @ObjectPropertyInfo
That description is crap. In part because what it tries to describe is
crap. Neither is this patch's problem.
> +#
> +# @value: the value of the property. Omitted if cannot be read.
Suggest "Absent when the property cannot be read."
> +#
> +# Since 10.1
> +##
> +{ 'struct': 'ObjectPropertyValue',
> + 'data': { 'name': 'str',
> +'type': 'str',
> +'*value': 'any' } }
ObjectPropertyValue suggests this describes a property's value. It does
not. It includes the name, i.e. it describes the *property*.
So does ObjectPropertyInfo.
The two overlap: both habe name and type. Only ObjectPropertyValue has
the current value. Only ObjectPropertyInfo has the default value and
description (I suspect the latter is useless in practice).
ObjectPropertyInfo is used with qom-list and qom-list-properties.
qom-list takes a QOM path, like your qom-tree-get and qom-list-getv.
I'd expect your commands to supersede qom-list in practice.
qom-list-properties is unlike your qom-tree-get and qom-list-getv: it
takes a type name. It's unreliable for non-abstract types: it can miss
dynamically created properties.
Let's ignore all this for now.
> +
> +##
> +# @ObjectNode:
> +#
> +# @name: the name of the node
> +#
> +# @children: child nodes
> +#
> +# @properties: properties of the node
> +#
> +# Since 10.1
> +##
> +{ 'struct': 'ObjectNode',
> + 'data': { 'name': 'str',
> +'children': [ 'ObjectNode' ],
> +'properties': [ 'ObjectPropertyValue' ] }}
> +
> +##
> # @qom-list:
> #
> # This command will list any properties of a object given a path in
> @@ -126,6 +158,30 @@
>'allow-preconfig': true }
>
> ##
> +# @qom-tree-get:
> +#
> +# This command returns a tree of objects and their properties,
> +# rooted at the specified path.
> +#
> +# @path: The absolute or partial path within the object model, as
> +# described in @qom-get
> +#
> +# Errors:
> +# - If path is not valid or is ambiguous, returns an error.
By convention, we use "If , , where is a
member of QapiErrorClass.
What are the possible error classes? As far as I can tell:
- If path is ambiguous, GenericError
- If path cannot be resolved, DeviceNotFound
However, use of error classes other than GenericError is strongly
discouraged (see error_set() in qapi/error.h).
Is the ability to distinguish between these two errors useful?
Existing related commands such as qom-get also use DeviceNotFound.
Entirely undocumented, exact error conditions unclear. Awesome.
Libvirt seems to rely on this undocumented behavior: I can see code
checking for DeviceNotFound. Hyrum's law strikes.
qom-get fails with DeviceNotFound in both of the above cases. It fails
with GenericError when @property doesn't exist or cannot be read. Your
qom-tree-get fails differently. Awesome again.
Choices:
1. Leave errors undocumented and inconsistent.
2. Document errors for all related commands. Make the new ones as
consistent as we can.
> +# - If a property cannot be read, the value field is omitted in
> +# the corresponding @ObjectPropertyValue.
This is not an error, and therefore doesn't belong here.
ObjectPropertyValue's documentation also mentions it. Good enough?
> +#
> +# Returns: A tree of @ObjectNode. Each node contains its name, list
> +# of properties, and list of child nodes.
Hmm.
A struct Object has no name. Only properties have a name.
An ObjectNode has a name, and an ObjectPropertyValue has a name.
I may get back to this in a later message.
> +#
> +# Since 10.1
> +##
> +{ 'command': 'qom-tree-get',
> + 'data': { 'path': 'str' },
> + 'returns': 'ObjectNode',
> + '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..b876681 100644
> --- a/qom/qom-qmp-cmds.c
> +++ b/qom/qom-qmp-cmds.c
> @@ -69,6 +69,78 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path,
> Error **errp)
> return props;
> }
>
> +static void qom_list_add_property_value(Object *obj, ObjectProperty *prop,
> +
