On 7/23/21 12:09 AM, Jose R. Ziviani wrote:
> When a module is not found, specially accelerators, QEMU displays
> a error message that not easy to understand[1]. This patch improves
> the readability by offering a user-friendly message[2].
>
> This patch also moves the accelerator ops check to runtine (instead
> of the original g_assert) because it works better with dynamic
> modules.
>
> [1] qemu-system-x86_64 -accel tcg
> ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed:
> (ops != NULL)
> Bail out! ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces:
> assertion failed: (ops != NULL)
> 31964 IOT instruction (core dumped) ./qemu-system-x86_64 ...
>
> [2] qemu-system-x86_64 -accel tcg
> accel-tcg-x86_64 module is missing, install the package or config the library
> path correctly.
>
> Signed-off-by: Jose R. Ziviani <jzivi...@suse.de>
> ---
> accel/accel-softmmu.c | 5 ++++-
> util/module.c | 14 ++++++++------
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
> index 67276e4f52..52449ac2d0 100644
> --- a/accel/accel-softmmu.c
> +++ b/accel/accel-softmmu.c
> @@ -79,7 +79,10 @@ void accel_init_ops_interfaces(AccelClass *ac)
> * all accelerators need to define ops, providing at least a mandatory
> * non-NULL create_vcpu_thread operation.
> */
> - g_assert(ops != NULL);
> + if (ops == NULL) {
> + exit(1);
> + }
> +
Ah, again, why?
This change looks wrong to me,
the ops code should be present when ops interfaces are initialized:
it should be a code level assertion, as it has to do with the proper order of
initializations in QEMU,
why would we want to do anything else but to assert here?
Am I blind to something obvious?
> if (ops->ops_init) {
> ops->ops_init(ops);
> }
> diff --git a/util/module.c b/util/module.c
> index 6bb4ad915a..268a8563fd 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -206,13 +206,10 @@ static int module_load_file(const char *fname, bool
> mayfail, bool export_symbols
> out:
> return ret;
> }
> -#endif
>
> bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
> {
> bool success = false;
> -
> -#ifdef CONFIG_MODULES
> char *fname = NULL;
> #ifdef CONFIG_MODULE_UPGRADES
> char *version_dir;
> @@ -300,6 +297,9 @@ bool module_load_one(const char *prefix, const char
> *lib_name, bool mayfail)
>
> if (!success) {
> g_hash_table_remove(loaded_modules, module_name);
> + fprintf(stderr, "%s module is missing, install the "
> + "package or config the library path "
> + "correctly.\n", module_name);
> g_free(module_name);
> }
>
> @@ -307,12 +307,9 @@ bool module_load_one(const char *prefix, const char
> *lib_name, bool mayfail)
> g_free(dirs[i]);
> }
>
> -#endif
> return success;
> }
>
> -#ifdef CONFIG_MODULES
> -
> static bool module_loaded_qom_all;
>
> void module_load_qom_one(const char *type)
> @@ -384,4 +381,9 @@ void qemu_load_module_for_opts(const char *group) {}
> void module_load_qom_one(const char *type) {}
> void module_load_qom_all(void) {}
>
> +bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
> +{
> + return false;
> +}
> +
> #endif
>
