Re: [PATCH 1/1] spapr.c: set a 'kvm-type' default value instead of relying on NULL

[Greg Kurz <groug@kaod.org>]

On Thu, 10 Dec 2020 13:34:59 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> To sum up everything:
> 

LGTM

> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2d5aeeb45a..61f0963916 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3028,11 +3028,11 @@ static int spapr_kvm_type(MachineState *machine, 
> const char *vm_type)
>           return 0;
>       }
> 
> -    if (!strcmp(vm_type, "HV")) {
> +    if (!g_ascii_strcasecmp(vm_type, "hv")) {
>           return 1;
>       }
> 
> -    if (!strcmp(vm_type, "PR")) {
> +    if (!g_ascii_strcasecmp(vm_type, "pr")) {
>           return 2;
>       }
> 
> @@ -3132,16 +3132,6 @@ static char *spapr_get_kvm_type(Object *obj, 
> Error **errp)
>   {
>       SpaprMachineState *spapr = SPAPR_MACHINE(obj);
> 
> -    /*
> -     * In case the user didn't set 'kvm-type', return DEFAULT_KVM_TYPE
> -     * instead of NULL. This allows us to be more predictable with what
> -     * is expected to happen in spapr_kvm_type(), since we can stop relying
> -     * on receiving a 'NULL' parameter as a valid input there.
> -     */
> -    if (!spapr->kvm_type) {
> -        return g_strdup(DEFAULT_KVM_TYPE);
> -    }
> -
>       return g_strdup(spapr->kvm_type);
>   }
> 
> @@ -3294,11 +3284,13 @@ static void spapr_instance_init(Object *obj)
> 
>       spapr->htab_fd = -1;
>       spapr->use_hotplug_event_source = true;
> +
> +    spapr->kvm_type = g_strdup(DEFAULT_KVM_TYPE);
>       object_property_add_str(obj, "kvm-type",
>                               spapr_get_kvm_type, spapr_set_kvm_type);
>       object_property_set_description(obj, "kvm-type",
> -                                    "Specifies the KVM virtualization 
> mode (HV, PR)."
> -                                    " If not specified, defaults to any 
> available KVM"
> +                                    "Specifies the KVM virtualization 
> mode (hv, pr, auto)."
> +                                    " auto is the default and allows 
> any available KVM"
>                                       " module loaded in the host. In 
> case both kvm_hv"
>                                       " and kvm_pr are loaded, kvm_hv 
> takes precedence.");
>