Re: [PATCH v3 2/7] qemu: check if s390 secure guest support is enabled

On Mon, Jun 15, 2020 at 04:49:30PM +0200, Boris Fiuczynski wrote: > On 6/15/20 4:17 PM, Erik Skultety wrote: >> On Mon, Jun 15, 2020 at 10:28:07AM +0200, Paulo de Rezende Pinatti wrote: >>> This patch introduces a common function to verify if the >>> availability of the so-called Secure Guest feature on the host >>> has changed in order to invalidate the qemu capabilities cache. >>> It can be used as an entry point for verification on different >>> architectures. >>> >>> For s390 the verification consists of: >>> - checking if /sys/firmware/uv is available: meaning the HW >>> facility is available and the host OS supports it; >>> - checking if the kernel cmdline contains 'prot_virt=1': meaning >>> the host OS wants to use the feature. >>> >>> Whenever the availability of the feature does not match the secure >>> guest flag in the cache then libvirt will re-build it in order to >>> pick up the new set of capabilities available. >>> >>> Signed-off-by: Paulo de Rezende Pinatti <ppinatti(a)linux.ibm.com&gt; >>> Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; >>> Tested-by: Viktor Mihajlovski <mihajlov(a)linux.ibm.com&gt; >>> Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com&gt; >>> --- >> >> Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; >> >> I'll squash the following in: >> >> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c >> index 0bade7e71b..54835f12a6 100644 >> --- a/src/qemu/qemu_capabilities.c >> +++ b/src/qemu/qemu_capabilities.c >> @@ -4699,12 +4699,8 @@ virQEMUCapsKVMSupportsSecureGuestS390(void) >> >> if (!virFileIsDir("/sys/firmware/uv")) >> return false; >> - >> if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0) >> return false; >> - >> - /* we're prefix matching rather than equality matching here, because kernel >> - * would treat even something like prot_virt='yFOO' as enabled */ >> if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kValues, >> G_N_ELEMENTS(kValues), >> VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST | >> > > Did you miss adding new lines before the last " return false;" lines in > virQEMUCapsKVMSupportsSecureGuestS390 and virQEMUCapsKVMSupportsSecureGuest > ? Good catch :). Again, sorry for the reverse diff. Erik