[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.0] qapi: qom: do not use target-specific conditionals
From: |
Markus Armbruster |
Subject: |
Re: [PATCH for-6.0] qapi: qom: do not use target-specific conditionals |
Date: |
Fri, 26 Mar 2021 11:57:07 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
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'.