Re: [PATCH 3/6] target/s390x/cpu_models: Make kvm_s390_get_host_cpu_model() return boolean

[Thomas Huth <thuth@redhat.com>]

On 19/04/2024 08.57, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> As error.h suggested, the best practice for callee is to return
> something to indicate success / failure.
> 
> So make kvm_s390_get_host_cpu_model() return boolean and check the
> returned boolean in get_max_cpu_model() instead of accessing @err.
> 
> Additionally, since now get_max_cpu_model() returns directly if
> kvm_s390_get_host_cpu_model() fills @err, so make
> kvm_s390_get_host_cpu_model() return true by default for the non-KVM
> case in target/s390x/cpu_models.h.

You could also argue the other way round that there should be something in 
*model if it returns "true" ... anyway, the stub should never be executed, 
so it likely doesn't matter too much, but I'd still prefer if we'd rather 
return "false" in the non-KVM stub instead.

> index d7b89129891a..5041c1e10fed 100644
> --- a/target/s390x/cpu_models.h
> +++ b/target/s390x/cpu_models.h
> @@ -116,12 +116,13 @@ S390CPUDef const *s390_find_cpu_def(uint16_t type, uint8_t gen, uint8_t ec_ga,
>   
>   #ifdef CONFIG_KVM
>   bool kvm_s390_cpu_models_supported(void);
> -void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp);
> +bool kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp);
>   void kvm_s390_apply_cpu_model(const S390CPUModel *model,  Error **errp);
>   #else
> -static inline void kvm_s390_get_host_cpu_model(S390CPUModel *model,
> +static inline bool kvm_s390_get_host_cpu_model(S390CPUModel *model,
>                                                  Error **errp)
>   {
> +    return true;
>   }

  Thomas