Re: [PATCH v2 1/1] modules: Improve error message when module is not found

From: Claudio Fontana <cfontana@suse.de>
To: Gerd Hoffmann <kraxel@redhat.com>, "Jose R. Ziviani" <jziviani@suse.de>
Cc: pbonzini@redhat.com, richard.henderson@linaro.org, qemu-devel@nongnu.org
Subject: Re: [PATCH v2 1/1] modules: Improve error message when module is not found
Date: Fri, 23 Jul 2021 10:04:03 +0200	[thread overview]
Message-ID: <aad125f4-7bd2-2a61-17d9-48e1dc909196@suse.de> (raw)
In-Reply-To: <20210723062520.hp6qux5enrwyw56u@sirius.home.kraxel.org>

On 7/23/21 8:25 AM, Gerd Hoffmann wrote:
>   Hi,
> 
>> --- 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) {
> 
> Error message here?> Also split accel bits into a separate patch?

If ops is NULL there is something seriously wrong with the code assumptions.
Asserts are in my view the proper way to handle these.

I do not think there is any reason to change this.

> 
>>          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);
> 
> This should be error_report(), or maybe warn_report() as this
> isn't a fatal error in all cases.  Otherwise looks good to me.
> 
> take care,
>   Gerd
> 