Re: [PATCH qemu-server v3 2/3] tests: improve multiarch build support by allowing re-init of cpu models

[Dominik Csapak <d.csapak@proxmox.com>]

On 2/10/26 2:20 PM, Fiona Ebner wrote:
> Am 05.02.26 um 3:18 PM schrieb Dominik Csapak:
>> instead of simply saving 'host_arch','all_cpu_models' and
>> 'cpu_models_by_arch' in the global package namespace, after initializing
>> directly in the module, use an initialization function and a getter to
>> generate this.
>>
>> This has two advantages:
>> * Only the first use actually fills the package wide variable
>>    (currently the $cpu_fmt uses this on module load)
>> * We can call the initialization manually in the tests where we need it
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> 
> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> 
> but one comment below
> 
>> +# returns the cpu models for the given architecture, or if $arch is not given, a
>> +# map from available architectures to models
>> +sub get_cpu_models_by_arch {
>> +    my ($arch) = @_;
>> +    initialize_cpu_models() if !defined($cpu_models_by_arch);
>> +    return $cpu_models_by_arch->{$arch} if defined($arch);
>> +    return $cpu_models_by_arch;
>> +}
> 
> I don't like having an overloaded return type here (depending on whether
> $arch is set or not, the returned hash has different structure). All
> callers pass the arch explicitly, so should we just require the argument
> to be set and avoid returning the full hash?

you're probably right, I wasn't so sure of this myself, but thought
why shouldn't it be possible to return the full hash.

requiring $arch here and just returning the per-architecture models 
makes more sense...