Re: [PATCH v5 4/4] accel: abort if we fail to load the accelerator plugin

[Claudio Fontana <cfontana@suse.de>]

On 9/23/22 17:28, Philippe Mathieu-Daudé via wrote:
> On 23/9/22 16:51, 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 <cfontana@suse.de>
>> Reviewed-by: Richard Henderson <richard.henderson@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'm not sure aborting is helpful here: QEMU process state is alright,
> we only miss a module...
> 
> Aren't we good with a simple 'exit(1);'?

we are I think...

Thanks,

C

> 
>> +    }
>>       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.
> 
> Preferably calling exit():
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>