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

[Daniel Henrique Barboza]

spapr_kvm_type() is considering 'vm_type=NULL' as a valid input, where
the function returns 0. This is relying on the current QEMU machine
options handling logic, where the absence of the 'kvm-type' option
will be reflected as 'vm_type=NULL' in this function.

This is not robust, and will break if QEMU options code decides to propagate
something else in the case mentioned above (e.g. an empty string instead
of NULL).

Let's avoid this entirely by setting a non-NULL default value in case of
no user input for 'kvm-type'. spapr_kvm_type() was changed to handle 3 fixed
values of kvm-type: "HV", "PR" and, if no kvm-type was set by the user,
DEFAULT_KVM_TYPE. This allows us always be predictable regardless of any
enhancements/changes made in QEMU options mechanics.

While we're at it, let's also document in 'kvm-type' description what
happens if the user does not set this option. This information is based
on how the pseries kernel handles the KVM_CREATE_VM ioctl(), where the
default value '0' makes the kernel choose an available KVM module to
use, giving precedence to kvm_hv. This logic is described in the kernel
source file arch/powerpc/kvm/powerpc.c, function kvm_arch_init_vm().

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---

The case I mentioned in the second paragraph is happening when we try to
execute a pseries guest with '--enable-kvm' using Paolo's patch 
"vl: make qemu_get_machine_opts static" [1]:

$ sudo ./ppc64-softmmu/qemu-system-ppc64 -nographic -nodefaults -machine 
pseries --enable-kvm
qemu-system-ppc64: Unknown kvm-type specified '' 

This happens due to the differences between how qemu_opt_get() and
object_property_get_str() works when there is no 'kvm-type' specified.
See [2] for more info about the issue found.


[1] https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg00471.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg01578.html 


 hw/ppc/spapr.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b7e0894019..32d938dc6a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3021,9 +3021,10 @@ static void spapr_machine_init(MachineState *machine)
     qemu_cond_init(&spapr->fwnmi_machine_check_interlock_cond);
 }
 
+#define DEFAULT_KVM_TYPE "auto"
 static int spapr_kvm_type(MachineState *machine, const char *vm_type)
 {
-    if (!vm_type) {
+    if (!strcmp(vm_type, DEFAULT_KVM_TYPE)) {
         return 0;
     }
 
@@ -3131,6 +3132,16 @@ 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);
 }
 
@@ -3273,7 +3284,11 @@ static void spapr_instance_init(Object *obj)
     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)");
+                                    "Specifies the KVM virtualization mode 
(HV, PR)."
+                                    " If not specified, defaults to any 
available KVM"
+                                    " module loaded in the host. In case both 
kvm_hv"
+                                    " and kvm_pr are loaded, kvm_hv takes 
precedence.");
+
     object_property_add_bool(obj, "modern-hotplug-events",
                             spapr_get_modern_hotplug_events,
                             spapr_set_modern_hotplug_events);
-- 
2.26.2