qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Thomas Huth <thuth@redhat.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Eric Blake <eblake@redhat.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>
Subject: Re: [PATCH] qapi: Make some ObjectTypes depend on the build settings
Date: Tue, 28 Sep 2021 19:39:34 +0200	[thread overview]
Message-ID: <c85745b0-d913-5f64-4c08-c2d03542107e@redhat.com> (raw)
In-Reply-To: <20210928160232.432980-1-thuth@redhat.com>

On 9/28/21 18:02, Thomas Huth wrote:
> Some of the ObjectType entries already depend on CONFIG_* switches.
> Some others also only make sense with certain configurations, but
> are currently always listed in the ObjectType enum. Let's make them
> depend on the correpsonding CONFIG_* switches, too, so that upper
> layers (like libvirt) have a better way to determine which features
> are available in QEMU.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  qapi/qom.json | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/qapi/qom.json b/qapi/qom.json
> index a25616bc7a..78b60433a9 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -777,7 +777,8 @@
>      'authz-pam',
>      'authz-simple',
>      'can-bus',
> -    'can-host-socketcan',
> +    { 'name': 'can-host-socketcan',
> +      'if': 'CONFIG_LINUX' },
>      'colo-compare',
>      'cryptodev-backend',
>      'cryptodev-backend-builtin',
> @@ -791,20 +792,24 @@
>      'filter-replay',
>      'filter-rewriter',
>      'input-barrier',
> -    'input-linux',
> +    { 'name': 'input-linux',
> +      'if': 'CONFIG_LINUX' },
>      'iothread',
>      'memory-backend-file',
>      { 'name': 'memory-backend-memfd',
>        'if': 'CONFIG_LINUX' },
>      'memory-backend-ram',
>      'pef-guest',
> -    'pr-manager-helper',
> +    { 'name': 'pr-manager-helper',
> +      'if': 'CONFIG_LINUX' },
>      'qtest',
>      'rng-builtin',
>      'rng-egd',
> -    'rng-random',
> +    { 'name': 'rng-random',
> +      'if': 'CONFIG_POSIX' },
>      'secret',
> -    'secret_keyring',
> +    { 'name': 'secret_keyring',
> +      'if': 'CONFIG_SECRET_KEYRING' },
>      'sev-guest',
>      's390-pv-guest',
>      'throttle-group',
> @@ -835,7 +840,8 @@
>        'authz-listfile':             'AuthZListFileProperties',
>        'authz-pam':                  'AuthZPAMProperties',
>        'authz-simple':               'AuthZSimpleProperties',
> -      'can-host-socketcan':         'CanHostSocketcanProperties',
> +      'can-host-socketcan':         { 'type': 'CanHostSocketcanProperties',
> +                                      'if': 'CONFIG_LINUX' },
>        'colo-compare':               'ColoCompareProperties',
>        'cryptodev-backend':          'CryptodevBackendProperties',
>        'cryptodev-backend-builtin':  'CryptodevBackendProperties',
> @@ -849,19 +855,23 @@
>        'filter-replay':              'NetfilterProperties',
>        'filter-rewriter':            'FilterRewriterProperties',
>        'input-barrier':              'InputBarrierProperties',
> -      'input-linux':                'InputLinuxProperties',
> +      'input-linux':                { 'type': 'InputLinuxProperties',
> +                                      'if': 'CONFIG_LINUX' },
>        'iothread':                   'IothreadProperties',
>        'memory-backend-file':        'MemoryBackendFileProperties',
>        'memory-backend-memfd':       { 'type': 'MemoryBackendMemfdProperties',
>                                        'if': 'CONFIG_LINUX' },
>        'memory-backend-ram':         'MemoryBackendProperties',
> -      'pr-manager-helper':          'PrManagerHelperProperties',
> +      'pr-manager-helper':          { 'type': 'PrManagerHelperProperties',
> +                                      'if': 'CONFIG_LINUX' },
>        'qtest':                      'QtestProperties',
>        'rng-builtin':                'RngProperties',
>        'rng-egd':                    'RngEgdProperties',
> -      'rng-random':                 'RngRandomProperties',
> +      'rng-random':                 { 'type': 'RngRandomProperties',
> +                                      'if': 'CONFIG_POSIX' },
>        'secret':                     'SecretProperties',
> -      'secret_keyring':             'SecretKeyringProperties',
> +      'secret_keyring':             { 'type': 'SecretKeyringProperties',
> +                                      'if': 'CONFIG_SECRET_KEYRING' },
>        'sev-guest':                  'SevGuestProperties',
>        'throttle-group':             'ThrottleGroupProperties',
>        'tls-creds-anon':             'TlsCredsAnonProperties',
> 

I quickly opened qapi/qom.json and spotted another one:

--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -870,3 +870,4 @@
       'tls-cipher-suites':          'TlsCredsProperties',
-      'x-remote-object':            'RemoteObjectProperties'
+      'x-remote-object':            { 'type': 'RemoteObjectProperties',
+                                      'if': 'CONFIG_MULTIPROCESS' },
   } }

While your change is correct, this isn't maintainable long term.
Not sure how we could improve that :/ But having to handle similar
information in 3 different places (configure, meson.build, qapi json)
is error prone. Thoughts?



  reply	other threads:[~2021-09-28 17:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28 16:02 [PATCH] qapi: Make some ObjectTypes depend on the build settings Thomas Huth
2021-09-28 17:39 ` Philippe Mathieu-Daudé [this message]
2021-09-29  6:17   ` Thomas Huth
2021-09-29 11:41     ` Markus Armbruster
2021-10-05  6:49       ` Thomas Huth
2021-10-05  8:24 ` Markus Armbruster
2021-10-08 12:01 ` Markus Armbruster
2021-10-08 17:13   ` Paolo Bonzini
2021-10-09 12:51     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c85745b0-d913-5f64-4c08-c2d03542107e@redhat.com \
    --to=philmd@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).