On Thu, Oct 23, 2025 at 12:58:27PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <[email protected]> writes:
>
> > This adds:
> >
> > * a new boolean 'secure' field to the type info returned by
> > qom-list-types, which will be set if the type provides a
> > security boundary
> >
> > * a new boolean 'secure' parameter to the arguments of
> > qom-list-types, which can be used to filter types based
> > on their security status
> >
> > Signed-off-by: Daniel P. Berrangé <[email protected]>
>
> I was about to ask for this feature in reply to PATCH 2 when I found
> this patch. Consider moving it right after PATCH 2, or
> forward-referencing it in PATCH 2's commit message.
>
> > ---
> > qapi/qom.json | 10 ++++++++--
> > qom/qom-qmp-cmds.c | 30 ++++++++++++++++++++++++------
> > 2 files changed, 32 insertions(+), 8 deletions(-)
> >
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index 830cb2ffe7..3e5e7e6f6f 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -210,12 +210,15 @@
> > # @abstract: the type is abstract and can't be directly instantiated.
> > # Omitted if false. (since 2.10)
> > #
> > +# @secure: the type provides a security boundary.
> > +# Omitted if false. (since 10.2)
>
> Please wrap like this:
>
> # @secure: the type provides a security boundary. Omitted if false.
> # (since 10.2)
>
> > +#
> > # @parent: Name of parent type, if any (since 2.10)
> > #
> > # Since: 1.1
> > ##
> > { 'struct': 'ObjectTypeInfo',
> > - 'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
> > + 'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str',
> > '*secure': 'bool' } }
>
> Long line. I think it's time to put each member on its own line.
>
> >
> > ##
> > # @qom-list-types:
> > @@ -227,12 +230,15 @@
> > #
> > # @abstract: if true, include abstract types in the results
> > #
> > +# @secure: if set, filter to only include types with matching security
> > status
> > +# (since 10.2)
>
> Hmm.
>
> absent false true
> @abstract only concrete only concrete all
> @secure all only insecure only secure (I think)
>
> The difference is grating. Any ideas?
I considered the current handling of @abstract to be flawed,
because there are three possible data sets you might want,
and the behaviour of @abstract only lets you query two of
the three - requires a second call to qom-list-types to
get the union of abstract and non-abstract.
Ideally we would fix @abstract but we can't do that without
back-compatibility fallout.
To avoid changing the default behaviour of qom-list-types
we need @secure==absent to return 'all', so that pretty
much forces us down this route of different behaviours
for @abstract vs @secure, unless we deprecate @abstract
and invent something completely new.
> > ObjectTypeInfoList *qmp_qom_list_types(const char *implements,
> > bool has_abstract,
> > bool abstract,
> > + bool has_secure,
> > + bool secure,
> > Error **errp)
> > {
> > - ObjectTypeInfoList *ret = NULL;
> > + ObjectTypeInfoData data = {
> > + .list = NULL,
> > + .has_secure = has_secure,
> > + .secure = secure,
> > + };
> >
> > module_load_qom_all();
> > - object_class_foreach(qom_list_types_tramp, implements, abstract, &ret);
> > + object_class_foreach(qom_list_types_tramp, implements, abstract,
> > &data);
> >
> > - return ret;
> > + return data.list;
> > }
> >
> > ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
>
> This fuses a change of how the list value is built with the addition of
> a new list element member. I'd prefer them separate.
Sure, will change.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
