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

[Steven Sistare <steven.sistare@oracle.com>]

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