Re: [PATCH 8/8] qom: pass TargetInfo to is_available for later use

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Oct 07, 2026 at 05:00:06PM -0700, Pierrick Bouvier wrote:
> On 10/7/2026 2:42 PM, Yonggang Luo wrote:
> > On Thu, Oct 8, 2026 at 4:44 AM Pierrick Bouvier
> > <pierrick.bouvier@oss.qualcomm.com> wrote:
> >>
> >> On 10/7/2026 4:48 AM, Yonggang Luo wrote:
> >>> Change TypeInfo.is_available to the new TypeIsAvailable type, which
> >>> takes a const TargetInfo *. Registration passes target_info(), so
> >>> behavior is unchanged. object.h only forward-declares TargetInfo.
> >>>
> >>> Switch the existing users (raspi, raspi4b, ast27x0 evb and fc) and the
> >>> generated machine parents to the matching target_is_<arch> helpers.
> >>>
> >>> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> >>> ---
> >>>  hw/arm/aspeed_ast27x0-fc.c  |  2 +-
> >>>  hw/arm/aspeed_ast27x0_evb.c |  4 ++--
> >>>  hw/arm/raspi.c              |  4 ++--
> >>>  hw/arm/raspi4b.c            |  2 +-
> >>>  hw/arm/virt.c               |  4 ++--
> >>>  hw/m68k/virt.c              |  4 ++--
> >>>  hw/or1k/virt.c              |  2 +-
> >>>  include/qom/object.h        | 12 +++++++++++-
> >>>  qom/object.c                |  3 ++-
> >>>  9 files changed, 24 insertions(+), 13 deletions(-)

snip

> >>> @@ -167,7 +168,7 @@ static TypeImpl *type_register_internal(const TypeInfo *info)
> >>>          abort();
> >>>      }
> >>>
> >>> -    if (info->is_available && !info->is_available()) {
> >>> +    if (info->is_available && !info->is_available(target_info())) {
> >>>          return NULL;
> >>>      }
> >>>
> >>
> >> ```
> >> info->is_available(target_info())
> >> ```
> >>
> >> or
> >>
> >> ```
> >> is_available() {
> >>   TargetInfo ti * = target_info();
> >>   ...
> >> }
> >> ```
> >>
> >> Are doing the exact same thing today.
> >>
> >> So what is the future goal requiring to introduce this?
> > 
> > From Daniel P. Berrangé's
> > requesthttps://lore.kernel.org/qemu-devel/apAszS1ewxM0Y1XU@redhat.com/
> >         I forgot to mention that this is also pretty important for making
> >         introspection work, as we need to be able to say
> > 
> >           'qemu-system --machine none -qmp ...'
> > 
> >         and connect to QMP to introspect the binary. Having to add a
> >         '--target <name>' into that mix is undesirable, as it creates
> >         a chicken & egg problem. You don't know what targets are built
> >         into qemu-system without introspecting it and you can't
> >         introspect it without selecting a target to introspect.
> > 
> > From Pierrick's review
> > https://lore.kernel.org/qemu-devel/557262df-b8e7-49b5-a245-ae46b8885b30@oss.qualcomm.com/So,
> > for existing binary: -M virt works as before.
> > 
> >         For new multi binary:
> >         $ qemu-system -M virt:
> >         Multiple machines were found, select one among:
> >         - aarch64-virt
> >         - riscv64-virt
> > 
> >         Does it matches what you would expect?
> > 
> > From Daniel's review
> > https://lore.kernel.org/qemu-devel/aro93NuZBXYGbD33@redhat.com/
> >         We already have  'QemuTargetInfo' struct which is used
> >         with 'query-target', that should also be used with the
> >         new 'query-targets' command.
> > 
> >         We need to add the 'endianness' field to QemuTargetInfo.
> > 
> >         I don't think that we need to report machines though.
> >         We already have 'query-machines' which gives much broader
> >         information on the machines. "MachineInfo" needs to
> >         gain
> > 
> >         "targets": ["SysEmuTarget"]
> > 
> >         to list which target(s) a machine can be used with.
> > 
> > 
> > To achieve these, we needs add TypeInfo parameter to it so that we can do
> > 
> > static void report_unsupported_machine_type(const char *name, Error **errp)
> > {
> >     g_autoptr(GSList) machines = machine_class_get_list(true);
> >     g_autoptr(GString) targets = g_string_new(NULL);
> >     TargetInfoNode *node;
> >     GSList *el;
> > 
> >     error_setg(errp, "unsupported machine type: \"%s\" for target \"%s\"",
> >                name, target_name());
> >     QLIST_FOREACH(node, target_info_list(), next) {
> >         for (el = machines; el; el = el->next) {
> >             MachineClass *mc = el->data;
> > 
> >             if ((!strcmp(mc->name, name) || !g_strcmp0(mc->alias, name)) &&
> >                 object_class_get_is_available(OBJECT_CLASS(mc))(node->info)) {
> >                 g_string_append_printf(targets, "  %s\n",
> >                                        node->info->target_name);
> >                 break;
> >             }
> >         }
> >     }
> >     if (targets->len) {
> >         error_append_hint(errp, "Available for targets:\n%s", targets->str);
> >     }
> > }
> > 
> > That is we can iterate on all target-list, and check if it's can be fitlered.
> >
> 
> Ok, I finally see where this is going.
> 
> So, based on all the previous conversations, we agreed to have a
> qemu-system binary with all the QOM types available (for all targets).

ack.

> I personally think that if we go this way, we should not have any
> -target option, and even less a -target none option.

If we don't have a compelling need for -target to exist now, then
dropping it as a concept is good.

> The current is_available() approach was made with filtering per target
> in mind, which predates where things are now. So, it's a good time to
> reevaluate how to do it.
> 
> This leaves us with (at least) two options:
> - keep the callback and add TargetInfo parameter (what is done here)
> - remove is_available, and adopt a more declarative approach
> 
> How about we (re)-explore the second one?
> 
> It's what we did in the past
> (https://gitlab.com/qemu-project/qemu/-/commit/5de9216064d32eaac125dac4de1878486c16d73e),
> and that I removed recently, in favor of is_available.
> My rationale was that it was incomplete, since we needed to filter types
> per config also, and not only target.
> Now that direction is to not filter anything, filter per config is
> unneeded, and switching back to declarative instead of callback makes
> total sense. Using interfaces like we did has its own issues, and I
> would not like to see this coming back.
> 
> I'll do one very concrete proposal:
> 
> ```
> How about letting machines (and machines only first), have a new field
> 'targets', being a uint32_t mask with an array of EMU_TARGET?
> machine->targets = EMU_TARGET_X | EMU_TARGET_Y;
> ```
> 
> @Daniel, IIUC, this is exactly the approach you proposed a few weeks
> ago, just before I got frustrated and lost my patience against you. I
> think it's more clear for me where you wanted to go. Sorry it took such
> a large detour.

Not a problem. We were coming at the debate with different ideas of
where we were heading, so talking past each other at times.

> 
> Yonggang, does that sounds good for you?
> Daniel, does that sounds good for you also?

Works for me.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|