[PATCH v2 11/32] qom: report & filter on security status in qom-list-types

Daniel P. Berrangé posted 32 patches 4 months, 2 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Christian Schoenebeck <qemu_oss@crudebyte.com>, Greg Kurz <groug@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Gerd Hoffmann <kraxel@redhat.com>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>, Stefano Garzarella <sgarzare@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Amit Shah <amit@kernel.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Helge Deller <deller@gmx.de>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Samuel Tardieu <sam@rfc1149.net>, Alistair Francis <alistair@alistair23.me>, Igor Mitsyanko <i.mitsyanko@gmail.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Aleksandar Rikalo <arikalo@gmail.com>, Thomas Huth <huth@tuxfamily.org>, BALATON Zoltan <balaton@eik.bme.hu>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Sergio Lopez <slp@redhat.com>, John Snow <jsnow@redhat.com>, Jiri Slaby <jslaby@suse.cz>, Beniamino Galvani <b.galvani@gmail.com>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>, Jason Wang <jasowang@redhat.com>, Pavel Pisa <pisa@cmp.felk.cvut.cz>, Francisco Iglesias <francisco.iglesias@amd.com>, Vikram Garhwal <vikram.garhwal@bytedance.com>, Stefan Weil <sw@weilnetz.de>, Bernhard Beschow <shentey@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>, Subbaraya Sundeep <sundeep.lkml@gmail.com>, Jan Kiszka <jan.kiszka@web.de>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Max Filippov <jcmvbkbc@gmail.com>, Jiri Pirko <jiri@resnulli.us>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Sven Schnelle <svens@stackframe.org>, Rob Herring <robh@kernel.org>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Andrey Smirnov <andrew.smirnov@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Aditya Gupta <adityag@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Magnus Damm <magnus.damm@gmail.com>, Paul Burton <paulburton@kernel.org>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Fam Zheng <fam@euphon.net>, Hannes Reinecke <hare@suse.com>, Samuel Thibault <samuel.thibault@ens-lyon.org>, Tony Krowiak <akrowiak@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Alex Williamson <alex.williamson@redhat.com>, Tomita Moeko <tomitamoeko@gmail.com>, Viresh Kumar <viresh.kumar@linaro.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Eric Auger <eric.auger@redhat.com>, Alexander Graf <graf@amazon.com>, Dorjoy Chowdhury <dorjoychy111@gmail.com>, Radoslaw Biernacki <rad@semihalf.com>, Leif Lindholm <leif.lindholm@oss.qualcomm.com>, "Collin L. Walling" <walling@linux.ibm.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eric Blake <eblake@redhat.com>
[PATCH v2 11/32] qom: report & filter on security status in qom-list-types
Posted by Daniel P. Berrangé 4 months, 2 weeks ago
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é <berrange@redhat.com>
---
 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)
+#
 # @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' } }
 
 ##
 # @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)
+#
 # Returns: a list of types, or an empty list if no results are found
 #
 # Since: 1.1
 ##
 { 'command': 'qom-list-types',
-  'data': { '*implements': 'str', '*abstract': 'bool' },
+  'data': { '*implements': 'str', '*abstract': 'bool', '*secure': 'bool' },
   'returns': [ 'ObjectTypeInfo' ],
   'allow-preconfig': true }
 
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 57f1898cf6..9e221bb332 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -151,33 +151,51 @@ QObject *qmp_qom_get(const char *path, const char *property, Error **errp)
     return object_property_get_qobject(obj, property, errp);
 }
 
-static void qom_list_types_tramp(ObjectClass *klass, void *data)
+typedef struct {
+    ObjectTypeInfoList *list;
+    bool has_secure;
+    bool secure;
+} ObjectTypeInfoData;
+
+static void qom_list_types_tramp(ObjectClass *klass, void *opaque)
 {
-    ObjectTypeInfoList **pret = data;
+    ObjectTypeInfoData *data = opaque;
     ObjectTypeInfo *info;
     ObjectClass *parent = object_class_get_parent(klass);
 
+    if (data->has_secure &&
+        data->secure != object_class_is_secure(klass)) {
+        return;
+    }
+
     info = g_malloc0(sizeof(*info));
     info->name = g_strdup(object_class_get_name(klass));
     info->has_abstract = info->abstract = object_class_is_abstract(klass);
+    info->has_secure = info->secure = object_class_is_secure(klass);
     if (parent) {
         info->parent = g_strdup(object_class_get_name(parent));
     }
 
-    QAPI_LIST_PREPEND(*pret, info);
+    QAPI_LIST_PREPEND(data->list, info);
 }
 
 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,
-- 
2.50.1


Re: [PATCH v2 11/32] qom: report & filter on security status in qom-list-types
Posted by Markus Armbruster 3 months, 2 weeks ago
Daniel P. Berrangé <berrange@redhat.com> 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é <berrange@redhat.com>

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?

If we decide to keep it as is, please wrap like this:

   # @secure: if set, filter to only include types with matching security
   #     status (since 10.2)

> +#
>  # Returns: a list of types, or an empty list if no results are found
>  #
>  # Since: 1.1
>  ##
>  { 'command': 'qom-list-types',
> -  'data': { '*implements': 'str', '*abstract': 'bool' },
> +  'data': { '*implements': 'str', '*abstract': 'bool', '*secure': 'bool' },
>    'returns': [ 'ObjectTypeInfo' ],
>    'allow-preconfig': true }
>  
> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
> index 57f1898cf6..9e221bb332 100644
> --- a/qom/qom-qmp-cmds.c
> +++ b/qom/qom-qmp-cmds.c
> @@ -151,33 +151,51 @@ QObject *qmp_qom_get(const char *path, const char *property, Error **errp)
>      return object_property_get_qobject(obj, property, errp);
>  }
>  
> -static void qom_list_types_tramp(ObjectClass *klass, void *data)
> +typedef struct {
> +    ObjectTypeInfoList *list;
> +    bool has_secure;
> +    bool secure;
> +} ObjectTypeInfoData;
> +
> +static void qom_list_types_tramp(ObjectClass *klass, void *opaque)
>  {
> -    ObjectTypeInfoList **pret = data;
> +    ObjectTypeInfoData *data = opaque;
>      ObjectTypeInfo *info;
>      ObjectClass *parent = object_class_get_parent(klass);
>  
> +    if (data->has_secure &&
> +        data->secure != object_class_is_secure(klass)) {
> +        return;
> +    }
> +
>      info = g_malloc0(sizeof(*info));
>      info->name = g_strdup(object_class_get_name(klass));
>      info->has_abstract = info->abstract = object_class_is_abstract(klass);
> +    info->has_secure = info->secure = object_class_is_secure(klass);
>      if (parent) {
>          info->parent = g_strdup(object_class_get_name(parent));
>      }
>  
> -    QAPI_LIST_PREPEND(*pret, info);
> +    QAPI_LIST_PREPEND(data->list, info);
>  }
>  
>  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.
Re: [PATCH v2 11/32] qom: report & filter on security status in qom-list-types
Posted by Daniel P. Berrangé 3 months, 2 weeks ago
On Thu, Oct 23, 2025 at 12:58:27PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> 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é <berrange@redhat.com>
> 
> 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 :|