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

[Thomas Huth <thuth@redhat.com>]

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