On 9/24/22 14:35, Philippe Mathieu-Daudé via wrote:
> On 24/9/22 01:21, Claudio Fontana wrote:
>> if QEMU is configured with modules enabled, it is possible that the
>> load of an accelerator module will fail.
>> Abort in this case, relying on module_object_class_by_name to report
>> the specific load error if any.
>>
>> Signed-off-by: Claudio Fontana <cfont...@suse.de>
>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>> ---
>> accel/accel-softmmu.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
>> index 67276e4f52..9fa4849f2c 100644
>> --- a/accel/accel-softmmu.c
>> +++ b/accel/accel-softmmu.c
>> @@ -66,6 +66,7 @@ void accel_init_ops_interfaces(AccelClass *ac)
>> {
>> const char *ac_name;
>> char *ops_name;
>> + ObjectClass *oc;
>> AccelOpsClass *ops;
>>
>> ac_name = object_class_get_name(OBJECT_CLASS(ac));
>> @@ -73,8 +74,13 @@ void accel_init_ops_interfaces(AccelClass *ac)
>>
>> ops_name = g_strdup_printf("%s" ACCEL_OPS_SUFFIX, ac_name);
>> ops = ACCEL_OPS_CLASS(module_object_class_by_name(ops_name));
>> + oc = module_object_class_by_name(ops_name);
>> + if (!oc) {
>> + error_report("fatal: could not load module for type '%s'",
>> ops_name);
>> + abort();
>
> I still think a coredump won't help at all to figure the problem here: a
I can change this from abort to exit(1), the issue I am seeing is, usually when
we fail to create or initialize objects
we seem to be using abort(), the most prominent examples are in qom/object.c:
static TypeImpl *type_new(const TypeInfo *info)
{
TypeImpl *ti = g_malloc0(sizeof(*ti));
int i;
g_assert(info->name != NULL);
if (type_table_lookup(info->name) != NULL) {
fprintf(stderr, "Registering `%s' which already exists\n", info->name);
abort();
}
...
void object_initialize(void *data, size_t size, const char *typename)
{
TypeImpl *type = type_get_by_name(typename);
#ifdef CONFIG_MODULES
if (!type) {
Error *local_err = NULL;
int rv = module_load_qom(typename, &local_err);
if (rv > 0) {
type = type_get_by_name(typename);
} else if (rv < 0) {
error_report_err(local_err);
}
}
#endif
if (!type) {
error_report("missing object type '%s'", typename);
abort();
}
object_initialize_with_type(data, size, type);
}
Do you propose to change only the assert in accel_init_ops_interfaces to
exit(1)?
Or the other case as well in the series? (ie hw/core/qdev.c qdev_new() ?)
Do you propose to change this consistently through the codebase including the
object.c snippets above?
> module is missing, we know its name. Anyhow I don't mind much, and this
> can be cleaned later, so:
Sure this could be fixed later with a series that tries to use exit() vs
abort() consistently throughout the codebase when initializing and creating
objects.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>
Thanks!
Claudio
>> + }
>> g_free(ops_name);
>> -
>> + ops = ACCEL_OPS_CLASS(oc);
>> /*
>> * all accelerators need to define ops, providing at least a mandatory
>> * non-NULL create_vcpu_thread operation.
>
>
