Re: [PATCH for-6.1 3/4] qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code

[Thomas Huth]

On 14/04/2021 15.55, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
> > The ObjectType enum and ObjectOptions are included from qapi-types-qom.h
> > into common code. We should not use target-specific config switches like
> > CONFIG_VIRTIO_CRYPTO here, since this is not defined in common code and
> > thus the enum will look differently between common and target specific
> > code. For this case, it's hopefully enough to check for CONFIG_VHOST_CRYPTO
> > only (which is a host specific config switch, i.e. it's the same on all
> > targets).
>
> Drawback: introspection now claims cryptodev-vhost-user is among the
> values of qom-type, which is a lie when !defined(CONFIG_VIRTIO_CRYPTO).
>
> Is this the first lie about QOM object types?
>
> Do we care?

I don't think we really care, since there are other entries in the list 
which are obviously only available on certain targets or configurations, but 
not fenced with "if"s, e.g. s390-pv-guest, input-linux or rng-random.
Or do you see a special problem with cryptodev-vhost-user here?

> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   qapi/qom.json | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index db5ac419b1..cd0e76d564 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -752,7 +752,7 @@
> >       'cryptodev-backend',
> >       'cryptodev-backend-builtin',
> >       { 'name': 'cryptodev-vhost-user',
> > -      'if': 'defined(CONFIG_VIRTIO_CRYPTO) && defined(CONFIG_VHOST_CRYPTO)' },
> > +      'if': 'defined(CONFIG_VHOST_CRYPTO)' },
> >       'dbus-vmstate',
> >       'filter-buffer',
> >       'filter-dump',
> > @@ -809,7 +809,7 @@
> >         'cryptodev-backend':          'CryptodevBackendProperties',
> >         'cryptodev-backend-builtin':  'CryptodevBackendProperties',
> >         'cryptodev-vhost-user':       { 'type': 'CryptodevVhostUserProperties',
> > -                                      'if': 'defined(CONFIG_VIRTIO_CRYPTO) && 
> > defined(CONFIG_VHOST_CRYPTO)' },
> > +                                      'if': 'defined(CONFIG_VHOST_CRYPTO)' },
> >         'dbus-vmstate':               'DBusVMStateProperties',
> >         'filter-buffer':              'FilterBufferProperties',
> >         'filter-dump':                'FilterDumpProperties',
>
> Could CryptodevVhostUserProperties be conditional, too?

That's certainly a question for the QOM experts here...

 Thomas