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

[Philippe Mathieu-Daudé]

On 19/4/24 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.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   target/s390x/cpu_models.c |  9 ++++-----
>   target/s390x/cpu_models.h |  5 +++--
>   target/s390x/kvm/kvm.c    | 13 +++++++------
>   3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 052540a866ac..a0e4acb707d7 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -560,16 +560,15 @@ S390CPUModel *get_max_cpu_model(Error **errp)
>       }
>   
>       if (kvm_enabled()) {

Nitpicking, we could move @err declaration here:

          Error *err = NULL;

> -        kvm_s390_get_host_cpu_model(&max_model, &err);
> +        if (!kvm_s390_get_host_cpu_model(&max_model, &err)) {
> +            error_propagate(errp, err);
> +            return NULL;
> +        }
>       } else {
>           max_model.def = s390_find_cpu_def(QEMU_MAX_CPU_TYPE, QEMU_MAX_CPU_GEN,
>                                             QEMU_MAX_CPU_EC_GA, NULL);
>           bitmap_copy(max_model.features, qemu_max_cpu_feat, S390_FEAT_MAX);
>       }
> -    if (err) {
> -        error_propagate(errp, err);
> -        return NULL;
> -    }
>       cached = true;
>       return &max_model;
>   }