Re: [PATCH v5 01/10] module: qom module support

[Christophe de Dinechin]

On 2020-06-24 at 15:10 CEST, Gerd Hoffmann wrote...
> Add support for qom types provided by modules.  For starters use a
> manually maintained list which maps qom type to module and prefix.
>
> Two load functions are added:  One to load the module for a specific
> type, and one to load all modules (needed for object/device lists as
> printed by -- for example -- qemu -device help).
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/qemu/module.h |  2 ++
>  util/module.c         | 55 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+)
>
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index 011ae1ae7605..9121a475c1b6 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -70,5 +70,7 @@ void register_dso_module_init(void (*fn)(void), 
> module_init_type type);
>
>  void module_call_init(module_init_type type);
>  bool module_load_one(const char *prefix, const char *lib_name);
> +void module_load_qom_one(const char *type);
> +void module_load_qom_all(void);
>
>  #endif
> diff --git a/util/module.c b/util/module.c
> index e48d9aacc05a..ee560a4b4269 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -245,3 +245,58 @@ bool module_load_one(const char *prefix, const char 
> *lib_name)
>  #endif
>      return success;
>  }
> +
> +/*
> + * Building devices and other qom objects modular is mostly useful in
> + * case they have dependencies to external shared libraries, so we can
> + * cut down the core qemu library dependencies.  Which is the case for
> + * only a very few devices & objects.
> + *
> + * So with the expectation that this will be rather the exception than
> + * to rule and the list will not gain that many entries go with a

Nit: Add a comma after "entries"

I would also indicate that this list needs to be sorted by prefix/module

> + * simple manually maintained list for now.
> + */
> +static struct {
> +    const char *type;
> +    const char *prefix;
> +    const char *module;

Because of the sorting rule, it is more intuitive to put the module first
and the type last, for cases where you have

    mod1 prefix1 type1
    mod1 prefix1 type2
    mod1 prefix1 type3
    mod2 prefix2 type1

Visually, I expect the constants to come first.


> +} const qom_modules[] = {
> +};
> +
> +static bool module_loaded_qom_all;
> +
> +void module_load_qom_one(const char *type)
> +{
> +    int i;
> +
> +    if (module_loaded_qom_all) {
> +        return;
> +    }
> +    for (i = 0; i < ARRAY_SIZE(qom_modules); i++) {
> +        if (strcmp(qom_modules[i].type, type) == 0) {
> +            module_load_one(qom_modules[i].prefix,
> +                            qom_modules[i].module);
> +            return;
> +        }
> +    }
> +}
> +
> +void module_load_qom_all(void)
> +{
> +    int i;
> +
> +    if (module_loaded_qom_all) {
> +        return;
> +    }
> +    for (i = 0; i < ARRAY_SIZE(qom_modules); i++) {
> +        if (i > 0 && (strcmp(qom_modules[i - 1].module,
> +                             qom_modules[i].module) == 0 &&
> +                      strcmp(qom_modules[i - 1].prefix,
> +                             qom_modules[i].prefix) == 0)) {
> +            /* one module implementing multiple types -> load only once */

This is the sorting requirement I'm talking about

> +            continue;
> +        }
> +        module_load_one(qom_modules[i].prefix, qom_modules[i].module);
> +    }
> +    module_loaded_qom_all = true;
> +}


--
Cheers,
Christophe de Dinechin (IRC c3d)