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