On Mon, Jan 22, 2018 at 11:46:14AM +0100, Jiri Denemark wrote:
> Whenever a different kernel is booted, some capabilities related to KVM
> (such as CPUID bits) may change. We need to refresh the cache to see the
> changes.
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
>
> Notes:
> The capabilities may also change if a parameter passed to a kvm module
> changes (kvm_intel.nested is a good example) so this is not a complete
> solution, but we're hopefully getting closer to it :-)
You mean getting closer to a situation where we are effectively storing the
cache on tmpfs, because we invalidate it on every reboot !
I think sometime soon we're going to need to consider if our cache invalidation
approach is fundamentally broken. We have a huge amount of stuff we query from
QEMU, but only a tiny amount is dependant on host kernel / microcode / kvm mod
options. Should we go back to invalidating only when libvirt/qemu binary changes
but then do partial invalidation of specific data items for kernel/microcode
changes.
It might require QEMU help to give us "promise" of which QMP commands return
data that may be impacted by KVM / kernel.
> src/qemu/qemu_capabilities.c | 57 +++++++++++++++++++++++++++++++++++++-------
> src/qemu/qemu_capspriv.h | 1 +
> tests/qemucapsprobe.c | 2 +-
> 3 files changed, 50 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index ab0ea8ec0d..2c573827f1 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -52,6 +52,7 @@
> #include <unistd.h>
> #include <sys/wait.h>
> #include <stdarg.h>
> +#include <sys/utsname.h>
>
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
> @@ -510,6 +511,7 @@ struct _virQEMUCaps {
> unsigned int libvirtVersion;
> unsigned int microcodeVersion;
> char *package;
> + char *kernelVersion;
>
> virArch arch;
>
> @@ -2303,6 +2305,9 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
> if (VIR_STRDUP(ret->package, qemuCaps->package) < 0)
> goto error;
>
> + if (VIR_STRDUP(ret->kernelVersion, qemuCaps->kernelVersion) < 0)
> + goto error;
> +
> ret->arch = qemuCaps->arch;
>
> if (qemuCaps->kvmCPUModels) {
> @@ -2363,6 +2368,7 @@ void virQEMUCapsDispose(void *obj)
> virBitmapFree(qemuCaps->flags);
>
> VIR_FREE(qemuCaps->package);
> + VIR_FREE(qemuCaps->kernelVersion);
> VIR_FREE(qemuCaps->binary);
>
> VIR_FREE(qemuCaps->gicCapabilities);
> @@ -3834,6 +3840,7 @@ struct _virQEMUCapsCachePriv {
> gid_t runGid;
> virArch hostArch;
> unsigned int microcodeVersion;
> + char *kernelVersion;
> };
> typedef struct _virQEMUCapsCachePriv virQEMUCapsCachePriv;
> typedef virQEMUCapsCachePriv *virQEMUCapsCachePrivPtr;
> @@ -3845,6 +3852,7 @@ virQEMUCapsCachePrivFree(void *privData)
> virQEMUCapsCachePrivPtr priv = privData;
>
> VIR_FREE(priv->libDir);
> + VIR_FREE(priv->kernelVersion);
> VIR_FREE(priv);
> }
>
> @@ -3970,6 +3978,12 @@ virQEMUCapsLoadCache(virArch hostArch,
> goto cleanup;
> }
>
> + if (virXPathBoolean("boolean(./kernelVersion)", ctxt) > 0) {
> + qemuCaps->kernelVersion = virXPathString("string(./kernelVersion)", ctxt);
> + if (!qemuCaps->kernelVersion)
> + goto cleanup;
> + }
> +
> if (!(str = virXPathString("string(./arch)", ctxt))) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("missing arch in QEMU capabilities cache"));
> @@ -4248,6 +4262,10 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps)
> virBufferAsprintf(&buf, "<package>%s</package>\n",
> qemuCaps->package);
>
> + if (qemuCaps->kernelVersion)
> + virBufferAsprintf(&buf, "<kernelVersion>%s</kernelVersion>\n",
> + qemuCaps->kernelVersion);
> +
> virBufferAsprintf(&buf, "<arch>%s</arch>\n",
> virArchToString(qemuCaps->arch));
>
> @@ -4385,14 +4403,24 @@ virQEMUCapsIsValid(void *data,
> return false;
> }
>
> - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
> - priv->microcodeVersion != qemuCaps->microcodeVersion) {
> - VIR_DEBUG("Outdated capabilities for '%s': microcode version changed "
> - "(%u vs %u)",
> - qemuCaps->binary,
> - priv->microcodeVersion,
> - qemuCaps->microcodeVersion);
> - return false;
> + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
> + if (priv->microcodeVersion != qemuCaps->microcodeVersion) {
> + VIR_DEBUG("Outdated capabilities for '%s': microcode version "
> + "changed (%u vs %u)",
> + qemuCaps->binary,
> + priv->microcodeVersion,
> + qemuCaps->microcodeVersion);
> + return false;
> + }
> +
> + if (STRNEQ_NULLABLE(priv->kernelVersion, qemuCaps->kernelVersion)) {
> + VIR_DEBUG("Outdated capabilities for '%s': kernel version changed "
> + "('%s' vs '%s')",
> + qemuCaps->binary,
> + priv->kernelVersion,
> + qemuCaps->kernelVersion);
> + return false;
> + }
> }
>
> return true;
> @@ -5228,6 +5256,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
> uid_t runUid,
> gid_t runGid,
> unsigned int microcodeVersion,
> + const char *kernelVersion,
> bool qmpOnly)
> {
> virQEMUCapsPtr qemuCaps;
> @@ -5284,9 +5313,13 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
> virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
> virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
>
> - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
> qemuCaps->microcodeVersion = microcodeVersion;
>
> + if (VIR_STRDUP(qemuCaps->kernelVersion, kernelVersion) < 0)
> + goto error;
> + }
> +
> cleanup:
> VIR_FREE(qmperr);
> return qemuCaps;
> @@ -5309,6 +5342,7 @@ virQEMUCapsNewData(const char *binary,
> priv->runUid,
> priv->runGid,
> priv->microcodeVersion,
> + priv->kernelVersion,
> false);
> }
>
> @@ -5397,6 +5431,7 @@ virQEMUCapsCacheNew(const char *libDir,
> char *capsCacheDir = NULL;
> virFileCachePtr cache = NULL;
> virQEMUCapsCachePrivPtr priv = NULL;
> + struct utsname uts = { 0 };
>
> if (virAsprintf(&capsCacheDir, "%s/capabilities", cacheDir) < 0)
> goto error;
> @@ -5417,6 +5452,10 @@ virQEMUCapsCacheNew(const char *libDir,
> priv->runGid = runGid;
> priv->microcodeVersion = microcodeVersion;
>
> + if (uname(&uts) == 0 &&
> + virAsprintf(&priv->kernelVersion, "%s %s", uts.release, uts.version) < 0)
> + goto error;
> +
> cleanup:
> VIR_FREE(capsCacheDir);
> return cache;
> diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
> index 98d163b920..222f3368e3 100644
> --- a/src/qemu/qemu_capspriv.h
> +++ b/src/qemu/qemu_capspriv.h
> @@ -37,6 +37,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
> uid_t runUid,
> gid_t runGid,
> unsigned int microcodeVersion,
> + const char *kernelVersion,
> bool qmpOnly);
>
> int virQEMUCapsLoadCache(virArch hostArch,
> diff --git a/tests/qemucapsprobe.c b/tests/qemucapsprobe.c
> index a5f5a38b16..7d60246949 100644
> --- a/tests/qemucapsprobe.c
> +++ b/tests/qemucapsprobe.c
> @@ -72,7 +72,7 @@ main(int argc, char **argv)
> return EXIT_FAILURE;
>
> if (!(caps = virQEMUCapsNewForBinaryInternal(VIR_ARCH_NONE, argv[1], "/tmp",
> - -1, -1, 0, true)))
> + -1, -1, 0, NULL, true)))
> return EXIT_FAILURE;
>
> virObjectUnref(caps);
Reviewed-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
