Re: [PATCH] accel: print an error message and exit if plugin not loaded

[Claudio Fontana <cfontana@suse.de>]

On 9/5/22 14:06, Richard Henderson wrote:
> On 9/5/22 11:13, Claudio Fontana wrote:
>> If module_load_one, module_load_file fail for any reason
>> (permissions, plugin not installed, ...), we need to provide some notification
>> to the user to understand that this is happening; otherwise the errors
>> reported on initialization will make no sense to the user.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>   accel/accel-softmmu.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
>> index 67276e4f52..807708ee86 100644
>> --- a/accel/accel-softmmu.c
>> +++ b/accel/accel-softmmu.c
>> @@ -66,15 +66,21 @@ 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));
>>       g_assert(ac_name != NULL);
>>   
>>       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 find module object of type \"%s\", "
>> +                     "plugin might not be loaded correctly", ops_name);
>> +        exit(EXIT_FAILURE);
>> +    }
> 
> The change is correct, in that we certainly cannot continue without the accelerator loaded.
> 
> But I'm very disappointed that the module interface does not use Error, so you have no 
> choice but to use an extremely vague message here.  I would much prefer to plumb down an 
> error parameter so that here one could simply pass &error_fatal.
> 
> 
> r~
> 

I agree. I see it as also connected to:

https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00578.html

module_load_file actually has the pertinent information of what it going wrong at the time it goes wrong, so I presume we should collect the Error there,
and find a way not to lose the return value along the way..

Claudio