Re: [PATCH v3 13/24] modules: use modinfo for dependencies

[Paolo Bonzini]

On 18/06/21 06:53, Gerd Hoffmann wrote:
> Use module database for module dependencies.
> Drop hard-coded dependency list.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   util/module.c | 55 ++++++++++++++++++++-------------------------------
>   1 file changed, 21 insertions(+), 34 deletions(-)
>
> diff --git a/util/module.c b/util/module.c
> index 8d3e8275b9f7..7d7b69cbdaca 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -182,28 +182,6 @@ static int module_load_file(const char *fname, bool 
> mayfail, bool export_symbols
>   out:
>       return ret;
>   }
> -
> -static const struct {
> -    const char *name;
> -    const char *dep;
> -} module_deps[] = {
> -    { "audio-spice",    "ui-spice-core" },
> -    { "chardev-spice",  "ui-spice-core" },
> -    { "hw-display-qxl", "ui-spice-core" },
> -    { "ui-spice-app",   "ui-spice-core" },
> -    { "ui-spice-app",   "chardev-spice" },
> -
> -    { "hw-display-virtio-gpu-gl", "hw-display-virtio-gpu" },
> -    { "hw-display-virtio-gpu-pci-gl", "hw-display-virtio-gpu-pci" },
> -    { "hw-display-virtio-vga-gl", "hw-display-virtio-vga" },
> -
> -#ifdef CONFIG_OPENGL
> -    { "ui-egl-headless", "ui-opengl"    },
> -    { "ui-gtk",          "ui-opengl"    },
> -    { "ui-sdl",          "ui-opengl"    },
> -    { "ui-spice-core",   "ui-opengl"    },
> -#endif
> -};
>   #endif
>   
>   bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
> @@ -219,9 +197,11 @@ bool module_load_one(const char *prefix, const char 
> *lib_name, bool mayfail)
>       char *dirs[5];
>       char *module_name;
>       int i = 0, n_dirs = 0;
> -    int ret, dep;
> +    int ret;
>       bool export_symbols = false;
>       static GHashTable *loaded_modules;
> +    const QemuModinfo *modinfo;
> +    const char **sl;
>   
>       if (!g_module_supported()) {
>           fprintf(stderr, "Module is not supported by system.\n");
> @@ -234,23 +214,30 @@ bool module_load_one(const char *prefix, const char 
> *lib_name, bool mayfail)
>   
>       module_name = g_strdup_printf("%s%s", prefix, lib_name);
>   
> -    for (dep = 0; dep < ARRAY_SIZE(module_deps); dep++) {
> -        if (strcmp(module_name, module_deps[dep].name) == 0) {
> -            /* we depend on another module */
> -            module_load_one("", module_deps[dep].dep, false);
> -        }
> -        if (strcmp(module_name, module_deps[dep].dep) == 0) {
> -            /* another module depends on us */
> -            export_symbols = true;
> -        }
> -    }
> -
>       if (g_hash_table_contains(loaded_modules, module_name)) {
>           g_free(module_name);
>           return true;
>       }
>       g_hash_table_add(loaded_modules, module_name);
>   
> +    for (modinfo = module_info; modinfo->name != NULL; modinfo++) {
> +        if (modinfo->deps) {
> +            if (strcmp(modinfo->name, module_name) == 0) {
> +                /* we depend on other module(s) */
> +                for (sl = modinfo->deps; *sl != NULL; sl++) {
> +                    module_load_one("", *sl, false);
> +                }
> +            } else {
> +                for (sl = modinfo->deps; *sl != NULL; sl++) {
> +                    if (strcmp(module_name, *sl) == 0) {
> +                        /* another module depends on us */
> +                        export_symbols = true;
> +                    }
> +                }
> +            }
> +        }
> +    }
> +
>       search_dir = getenv("QEMU_MODULE_DIR");
>       if (search_dir != NULL) {
>           dirs[n_dirs++] = g_strdup_printf("%s", search_dir);

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>