qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Cc: qemu-devel@nongnu.org, richard.henderson@linaro.org,
	stefanha@redhat.com, "Michael Roth" <michael.roth@amd.com>,
	pbonzini@redhat.com, peter.maydell@linaro.org, thuth@redhat.com,
	jsnow@redhat.com, philmd@linaro.org,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Markus Armbruster" <armbru@redhat.com>
Subject: Re: [RFC PATCH 0/3] single-binary: make QAPI generated files common
Date: Fri, 25 Apr 2025 08:35:10 +0100	[thread overview]
Message-ID: <aAs6Q2GiBUbUCc2I@redhat.com> (raw)
In-Reply-To: <20250424183350.1798746-1-pierrick.bouvier@linaro.org>

On Thu, Apr 24, 2025 at 11:33:47AM -0700, Pierrick Bouvier wrote:
> Feedback
> ========
> 
> The goal of this series is to be spark a conversation around following topics:
> 
> - Would you be open to such an approach? (expose all code, and restrict commands
>   registered at runtime only for specific targets)

QMP defines a public API between QEMU and external mgmt apps, and personally I
like the idea that the API exposed is identical across all binaries and thus
the API becomes independent of the impl choice of combined vs separate binaries,.

> - Are there unexpected consequences for libvirt or other consumers to expose
>   more definitions than what we have now?

QEMU used the selective hiding of commands in the QMP schema as a mechanism
to allow mgmt apps to probe for supported features. We need to check usage
of each QMP API feature that's behind a TARGET_* condition and identify
which libvirt uses as a feature probe, then come up with a strategy for how
best to handle each case in libvirt in future. We might need some additional
runtime mechanism to probe for certain things, but we won't know until we
look at things in more detail.

> - Would you recommend another approach instead? I experimented with having per
>   target generated files, but we still need to expose quite a lot in headers, so
>   my opinion is that it's much more complicated for zero benefit. As well, the
>   code size impact is more than negligible, so the simpler, the better.

IMHO it is unfortunate that the API we currently expose has a dependency on
a specific impl choice that mgmt apps are expected to rely on for feature
probing. An ideal API design is not so closely coupled to impl choice
(separate vs combined binaries), and would expose enough functionality
such that mgmt apps continue to work regardless of the impl choices.

We thought the conditionals were a good thing when we first designed QMP
this way. We ended up using two distinct classes of conditionals, one
reflecting build time features and one reflecting which target binary is
used. I don't think we fully contemplated the implications that the latter
set of conditionals would have on our ability to change our impl approach
in future. I think the proposal here is taking us in a good direction
given what we now know.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  parent reply	other threads:[~2025-04-25  7:36 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-24 18:33 [RFC PATCH 0/3] single-binary: make QAPI generated files common Pierrick Bouvier
2025-04-24 18:33 ` [RFC PATCH 1/3] qapi: add weak stubs for target specific commands Pierrick Bouvier
2025-04-24 18:52   ` Pierrick Bouvier
2025-04-24 18:33 ` [RFC PATCH 2/3] qapi: always expose TARGET_* or CONFIG_KVM code Pierrick Bouvier
2025-04-24 18:33 ` [RFC PATCH 3/3] qapi: make all generated files common Pierrick Bouvier
2025-04-24 20:31   ` Philippe Mathieu-Daudé
2025-04-24 21:08   ` Richard Henderson
2025-04-24 20:43 ` [RFC PATCH 0/3] single-binary: make QAPI " Philippe Mathieu-Daudé
2025-04-24 21:15   ` Richard Henderson
2025-04-24 22:22   ` Pierrick Bouvier
2025-04-24 20:44 ` Philippe Mathieu-Daudé
2025-04-25  7:35 ` Daniel P. Berrangé [this message]
2025-04-25 20:39   ` Pierrick Bouvier
2025-04-28  8:37     ` Daniel P. Berrangé
2025-04-28 15:54       ` Pierrick Bouvier
2025-04-25 22:16   ` Philippe Mathieu-Daudé
2025-04-26  4:53     ` Markus Armbruster
2025-04-25 15:38 ` Markus Armbruster
2025-04-25 21:07   ` Pierrick Bouvier
2025-04-25 21:13     ` Pierrick Bouvier
2025-04-26  6:21     ` Markus Armbruster
2025-04-28 16:05       ` Pierrick Bouvier
2025-04-29  7:43         ` Markus Armbruster
2025-04-29  8:37           ` Daniel P. Berrangé
2025-04-29 19:26             ` Pierrick Bouvier
2025-05-07 11:21             ` Markus Armbruster
2025-04-29 19:15           ` Pierrick Bouvier
2025-05-07  7:55             ` Markus Armbruster
2025-05-07 11:32               ` Daniel P. Berrangé
2025-05-07 19:00                 ` Pierrick Bouvier
2025-05-07 18:54               ` Pierrick Bouvier
2025-04-28 10:25     ` Peter Krempa
2025-04-28 16:18       ` Pierrick Bouvier
2025-04-28  8:55   ` Peter Krempa
2025-04-28 11:07     ` Markus Armbruster
2025-04-28 12:48       ` Philippe Mathieu-Daudé
2025-04-28 16:35       ` Pierrick Bouvier
2025-04-29  8:23         ` Markus Armbruster
2025-04-29  9:20           ` Thomas Huth
2025-04-29  9:32             ` Daniel P. Berrangé
2025-04-29  9:39               ` Philippe Mathieu-Daudé
2025-04-29 19:48             ` Pierrick Bouvier
2025-04-30  5:40               ` Thomas Huth
2025-04-30  6:18                 ` Pierrick Bouvier
2025-04-29  9:35           ` Philippe Mathieu-Daudé
2025-04-29  9:47             ` Daniel P. Berrangé
2025-04-29 19:57             ` Pierrick Bouvier
2025-04-29 20:11               ` Pierrick Bouvier
2025-04-29 12:04           ` BALATON Zoltan
2025-04-28 18:14 ` Stefan Hajnoczi
2025-04-28 19:25   ` Pierrick Bouvier
2025-04-28 19:54     ` Stefan Hajnoczi
2025-04-28 21:35       ` Pierrick Bouvier
2025-05-07 23:19 ` Pierrick Bouvier

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=aAs6Q2GiBUbUCc2I@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=pierrick.bouvier@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=stefanha@redhat.com \
    --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).