Re: [PATCH V2 4/5] qom: qom-list-getv

[Markus Armbruster <armbru@redhat.com>]

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>
> ---
>  qapi/qom.json      | 34 ++++++++++++++++++++++++++++++++++
>  qom/qom-qmp-cmds.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 94662ad..dc710d6 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -62,6 +62,16 @@
>              '*value': 'any' } }
>  
>  ##
> +# @ObjectPropertiesValues:
> +#
> +# @properties: a list of properties.
> +#
> +# Since 10.1
> +##
> +{ 'struct': 'ObjectPropertiesValues',
> +  'data': { 'properties': [ 'ObjectPropertyValue' ] }}
> +
> +##
>  # @ObjectNode:
>  #
>  # @name: the name of the node
> @@ -158,6 +168,30 @@
>    'allow-preconfig': true }
>  
>  ##
> +# @qom-list-getv:
> +#
> +# This command returns a list of properties and their values for
> +# each object path in the input list.

Imperative mood, please: "Return a list of ..."

> +#
> +# @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 an error.
> +#     - If a property cannot be read, the value field is omitted in
> +#       the corresponding @ObjectPropertyValue.

My comment on qom-tree-get's Errors: section applies.

> +#
> +# Returns: A list of @ObjectPropertiesValues.  Each element contains
> +#     the properties of the corresponding element in @paths.

Again, ObjectPropertiesValues is an unfortunate name.

> +#
> +# Since 10.1
> +##
> +{ 'command': 'qom-list-getv',
> +  'data': { 'paths': [ 'str' ] },
> +  'returns': [ 'ObjectPropertiesValues' ],
> +  'allow-preconfig': true }
> +
> +##
>  # @qom-tree-get:
>  #
>  # This command returns a tree of objects and their properties,

I find this command *much* simpler than qom-tree-get.

qom-list-getv treats all properties the same.  References, whether they
are children and links, are the same: a QOM path.

qom-tree-get separates properties into children and non-children.
Children become nested ObjectNodes, links remain QOM paths.

> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
> index b876681..1f05956 100644
> --- a/qom/qom-qmp-cmds.c
> +++ b/qom/qom-qmp-cmds.c
> @@ -90,6 +90,46 @@ static void qom_list_add_property_value(Object *obj, ObjectProperty *prop,
>      }
>  }
>  
> +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;
> +
> +    for ( ; paths ; paths = paths->next) {

I'd prefer a separate variable:

       for (tail = paths; tail; tail = tail->next) {

> +        ObjectPropertiesValues *item = g_new0(ObjectPropertiesValues, 1);
> +
> +        QAPI_LIST_APPEND(tail, item);
> +
> +        item->properties = qom_get_property_value_list(paths->value, errp);
> +        if (!item->properties) {
> +            qapi_free_ObjectPropertiesValuesList(head);
> +            return NULL;
> +        }
> +    }
> +
> +    return head;
> +}
> +
>  static ObjectNode *qom_tree_get(const char *path, Error **errp)
>  {
>      Object *obj;

The implementation is simpler than qom-tree's, too.