Re: [PATCH 2/2] qemu_capabilities.c: drop 'kvm_pr' support for non-Power8 hosts
On 6/30/20 2:35 PM, Andrea Bolognani wrote:
On Fri, 2020-06-19 at 18:04 -0300, Daniel Henrique Barboza wrote:
> +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"))
The macro you're looking for is STRPREFIX() :)
Thanks!
Anyway, the patch overall makes sense to me, but I'm wondering: since
generally we get KVM availability information from QEMU, is libvirt
the right place for this kind of check, or should it rather happen
one layer down the stack so that QEMU reports correct[*] KVM status
information to clients other than libvirt as well? David, what do you
think?
The reason I did it in Libvirt was to make the tooling and customers aware
that kvm_pr is not officially supported, but at the same time not hinder the
usage of kvm_pr completely for users using it via QEMU command line. People are
still using kvm_pr eventually in niche cases, such as internal tests and
development, specially because hvm_hv does not replace kvm_pr entirely yet
(e.g. kvm_hv does not work in a LPAR).
[*] It's fairly questionable whether reporting KVM as not available
when only kvm_pr is loaded is correct. It is certainly helpful,
but correct? Not entirely sure.
Yeah, this is not ideal.
Thing is that, in the Libvirt level, I can cater to customers/tooling and filter
kvm_pr out of the game (i.e. Bugzillas). In QEMU what would happen is either (1) more
people will be impacted and hating my guts because I blocked kvm_pr for unsupported
but valid usage or (2) I would be NACKed in QEMU (I mean, what's the technical reason
for me to claim "kvm_pr is not real KVM"?) and everything will stay as is,
everyone
thinking that kvm_pr is usable in both QEMU and Libvirt - and opening bugs - until
kvm_pr is removed.
TBH I don't see any pleasant solution for this kvm_pr situation. Leaving it
as is will keep bugs flowing in until it gets removed, filtering it out under
Libvirt/QEMU is kind of lame and you won't see me denying it. This patch is not
my finest hour, that's for sure :)
Thanks,
DHB