[PATCH 2/2] qemu_capabilities.c: drop 'kvm_pr' support for non-Power8 hosts

[Richard W.M. Jones]

On Fri, Jun 19, 2020 at 06:04:33PM -0300, Daniel Henrique Barboza wrote:
> PPC64 has two KVM modules: kvm_hv and kvm_pr. The official supported
> module was always kvm_hv, while kvm_pr was used for internal testing
> or for very niche cases in Power 8 hosts, always without official
> IBM or distro support.
> 
> Problem is, QMP will report KVM supportfor PPC64 if any of these
> modules is loaded in the host, and kvm_pr is broken in everything
> but Power8 (and will remain broken, since kvm_pr is unmaintained).
> This can lead to situations like [1], where the tooling is misled to
> believe that the host has KVM capabilities when in reality it
> doesn't.
> 
> The first reaction would be to simply forsake kvm_pr support entirely
> and move on, but there is no reason for now to be disruptive with any
> Power8 guests in the wild that are using kvm_pr (somehow). A more
> subtle approach is to not claim QEMU_CAPS_KVM support in all cases
> that we know it's completely broken, allowing Power8 users to take
> their shot using kvm_pr in their VMs. We can remove kvm_pr support
> completely when the module is removed from the kernel.
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1843865

I'm sorry I don't have a way to test this patch.  However it looks
reasonable from here based on the description above and the changes to
the code below, so ACK.

Rich.

> CC: Leonardo Augusto Guimarães Garcia <lagarcia at br.ibm.com>
> CC: Greg Kurz <groug at kaod.org>
> CC: David Gibson <david at gibson.dropbear.id.au>
> CC: Richard W.M. Jones <rjones at redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
>  src/qemu/qemu_capabilities.c | 38 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 484fff99e5..b1c1d4dd70 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -49,6 +49,7 @@
>  #include "qemu_process.h"
>  #include "qemu_firmware.h"
>  #include "virutil.h"
> +#include "virkmod.h"
>  
>  #include <fcntl.h>
>  #include <sys/stat.h>
> @@ -3242,6 +3243,31 @@ virQEMUCapsProbeQMPTPM(virQEMUCapsPtr qemuCaps,
>  }
>  
>  
> +static void
> +virQEMUCapsSetPPC64KVMState(virQEMUCapsPtr qemuCaps, virArch hostArch)
> +{
> +    g_autoptr(virCPUDef) hostCPU = virCPUProbeHost(hostArch);
> +
> +    /* At this moment, PPC64 has two KVM modules: kvm_hv and kvm_pr.
> +     * QEMU will return KVM present and enabled = true if any of these
> +     * is loaded in the host. Thing is, kvm_pr was never officially
> +     * supported by IBM or any other distro, but people still ended
> +     * up using it in Power8 hosts regardless. It is also currently
> +     * unmaintained and broken on Power9, and will be sunset in the
> +     * future. kvm_hv is the only KVM module that is and will be
> +     * supported.
> +     *
> +     * Until then, do not claim QEMU_CAPS_KVM if there is only kvm_pr
> +     * loaded in the host. There is a good chance that there are
> +     * unsupported kvm_pr Power8 guests running in the wild, so let's
> +     * cut some slack for this case, for now. */
> +    if (STRNEQLEN(hostCPU->model, "POWER8", 6) && !virKModIsLoaded("kvm_hv"))
> +        return;
> +
> +    virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM);
> +}
> +
> +
>  static int
>  virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps,
>                              qemuMonitorPtr mon)
> @@ -3252,8 +3278,16 @@ virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps,
>      if (qemuMonitorGetKVMState(mon, &enabled, &present) < 0)
>          return -1;
>  
> -    if (present && enabled)
> -        virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM);
> +    if (present && enabled) {
> +        virArch hostArch = virArchFromHost();
> +
> +        if (ARCH_IS_PPC64(hostArch)) {
> +            virQEMUCapsSetPPC64KVMState(qemuCaps, hostArch);
> +            return 0;
> +        }
> +
> +       virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM);
> +    }
>  
>      return 0;
>  }
> -- 
> 2.26.2

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org