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 <cfont...@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
