Re: [RFC PATCH-for-10.1 19/19] system/vl: Filter machine list for binary using machine_binary_filter()

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 4/3/25 16:49, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/vl.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/system/vl.c b/system/vl.c
> index d8a0fe713c9..554f5f2a467 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -27,6 +27,8 @@
>   #include "qemu/datadir.h"
>   #include "qemu/units.h"
>   #include "qemu/module.h"
> +#include "qemu/target_info.h"
> +#include "qemu/target_info-qom.h"
>   #include "exec/cpu-common.h"
>   #include "exec/page-vary.h"
>   #include "hw/qdev-properties.h"
> @@ -833,14 +835,29 @@ static bool usb_parse(const char *cmdline, Error **errp)
>   /***********************************************************/
>   /* machine registration */
>   
> +static char *machine_binary_filter(void)
> +{
> +    if (target_info_is_stub()) {
> +        return NULL;
> +    }
> +    return g_strconcat(TYPE_LEGACY_BINARY_PREFIX,
> +                       "qemu-system-", target_name(), NULL);

No, we should not have such things.
We can make it work with proper QOM types, defined by target, instead of 
relying on string construction/compare like this.

> +}
> +
>   static MachineClass *find_machine(const char *name, GSList *machines)
>   {
>       GSList *el;
> +    g_autofree char *binary_filter = machine_binary_filter();
>   
>       for (el = machines; el; el = el->next) {
>           MachineClass *mc = el->data;
>   
>           if (!strcmp(mc->name, name) || !g_strcmp0(mc->alias, name)) {
> +            if (binary_filter && !object_class_dynamic_cast(el->data,
> +                                                            binary_filter)) {
> +                /* Machine is not for this binary: fail */
> +                return NULL;
> +            }
>               return mc;
>           }
>       }
> @@ -1563,6 +1580,7 @@ static void machine_help_func(const QDict *qdict)
>       g_autoptr(GSList) machines = NULL;
>       GSList *el;
>       const char *type = qdict_get_try_str(qdict, "type");
> +    g_autofree char *binary_filter = machine_binary_filter();
>   
>       machines = object_class_get_list(TYPE_MACHINE, false);

If we define a proper TYPE_TARGET_MACHINE per target, and we add this to 
TargetInfo, this can become:

machines = object_class_get_list(target_machine_type(), false);

And we don't need any other string hack to detect what is the correct type.

>       if (type) {
> @@ -1577,6 +1595,12 @@ static void machine_help_func(const QDict *qdict)
>       machines = g_slist_sort(machines, machine_class_cmp);
>       for (el = machines; el; el = el->next) {
>           MachineClass *mc = el->data;
> +
> +        if (binary_filter && !object_class_dynamic_cast(el->data,
> +                                                        binary_filter)) {
> +            /* Machine is not for this binary: skip */
> +            continue;
> +        }

With the approach above, this is not needed anymore.

>           if (mc->alias) {
>               printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
>           }

I think we are missing a commit here, defining a proper 
TYPE_TARGET_MACHINE_PREFIX, that is target dependent, instead of the 
TYPE_LEGACY_BINARY_PREFIX.

And we should include in this type in TargetInfo, the same way it was 
done for cpus.