Re: [PATCH for-6.0] qapi: qom: do not use target-specific conditionals

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> ObjectType and ObjectOptions are defined in a target-independent file,
> therefore they do not have access to target-specific configuration
> symbols such as CONFIG_PSERIES or CONFIG_SEV.  For this reason,
> pef-guest and sev-guest are currently omitted when compiling the
> generated QAPI files.  In addition, this causes ObjectType to have
> different definitions depending on the file that is including
> qapi-types-qom.h (currently this is not causing any issues, but it
> is wrong).
>
> Define the two enum entries and the SevGuestProperties type
> unconditionally to avoid the issue.  We do not expect to have
> many target-dependent user-creatable classes, so it is not
> particularly problematic.
>
> Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qapi/qom.json | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 2056edc072..db5ac419b1 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -733,8 +733,7 @@
>              '*policy': 'uint32',
>              '*handle': 'uint32',
>              '*cbitpos': 'uint32',
> -            'reduced-phys-bits': 'uint32' },
> -  'if': 'defined(CONFIG_SEV)' }
> +            'reduced-phys-bits': 'uint32' } }
>  
>  ##
>  # @ObjectType:
> @@ -768,14 +767,14 @@
>      { 'name': 'memory-backend-memfd',
>        'if': 'defined(CONFIG_LINUX)' },
>      'memory-backend-ram',
> -    {'name': 'pef-guest', 'if': 'defined(CONFIG_PSERIES)' },
> +    'pef-guest',
>      'pr-manager-helper',
>      'rng-builtin',
>      'rng-egd',
>      'rng-random',
>      'secret',
>      'secret_keyring',
> -    {'name': 'sev-guest', 'if': 'defined(CONFIG_SEV)' },
> +    'sev-guest',
>      's390-pv-guest',
>      'throttle-group',
>      'tls-creds-anon',
> @@ -831,8 +830,7 @@
>        'rng-random':                 'RngRandomProperties',
>        'secret':                     'SecretProperties',
>        'secret_keyring':             'SecretKeyringProperties',
> -      'sev-guest':                  { 'type': 'SevGuestProperties',
> -                                      'if': 'defined(CONFIG_SEV)' },
> +      'sev-guest':                  'SevGuestProperties',
>        'throttle-group':             'ThrottleGroupProperties',
>        'tls-creds-anon':             'TlsCredsAnonProperties',
>        'tls-creds-psk':              'TlsCredsPskProperties',

No branch for tag value 'pef-guest', i.e. no tag-specific members.
There are two more: can_bus, s390_pv_guest.  I assume this is
intentional.

Links a bit of dead code into the other qemu-system-FOO, but that's
okay.

If we genuinely needed (or wanted) target-dependent -object, we'd split
qom-target.json off qom.json, and put the target-dependent parts there,
including the enum and the union, with the obvious ripple effects.  Not
now, maybe not ever.

Would adding "only for CONFIG_MUMBLE" to the doc comments make sense?
It's what we did before we had 'if'.