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

[Claudio Fontana]

On 7/23/21 1:20 PM, Claudio Fontana wrote:
> On 7/23/21 11:52 AM, Gerd Hoffmann wrote:
>>>> -    g_assert(ops != NULL);
>>>> +    if (ops == NULL) {
>>>> +        exit(1);
>>>> +    }
>>>> +
>>>
>>>
>>> Ah, again, why?
>>> This change looks wrong to me, 
>>>
>>> the ops code should be present when ops interfaces are initialized:
>>> it should be a code level assertion, as it has to do with the proper order 
>>> of initializations in QEMU,
>>>
>>> why would we want to do anything else but to assert here?
>>>
>>> Am I blind to something obvious?
>>
>> Building tcg accel ops modular moves that from coding error to possible
>> user error (user wants use tcg but has qemu-accel-tcg-$arch.rpm not
>> installed).
> 
> Sorry but without more background I don't buy it.
> 
> If ops is null at the time accel_init_interfaces is called,
> it means that we are trying to initialize the board (for softmmu)
> with an accelerator already selected, and without an accelerator actually 
> available.
> 
> The problem has happened already a long time before we get here.
> 
> When we check for viable accelerators, in configure_accelerators,
> we should check that the code is actually there, before choosing it as a 
> viable accelerator.
> 
> If we march on and start initializing the machine with an accelerator that is 
> not available,
> of course things will start failing left and right.
> 
> If things like:
> 
> bool have_tcg = accel_find("tcg");
> 
> return true when the code is actually not there, there seems to be a larger 
> issue to solve.
> 
> I think we need to think more broadly about this.

Overall, building the whole code base to be modular,
and then _not_ including unwanted modules in the base distro package,

is the whole point of going through this at all.

QEMU should gracefully figure out that indeed, the module is not there -> TCG 
is not there.

So we need more work to make this actually work right.

> Thanks,
> 
> Claudio
> 
> 
>>
>> The second part of the patch makes qemu print a message on the failed
>> module load, so the user would have a chance to figure where the assert
>> comes from, but replacing the assert with a more friendly message still
>> makes sense to me.
>>
>> take care,
>>   Gerd
>>
> 
>