[libvirt] [PATCH] qemu: Refresh caps cache after booting a different kernel

Daniel P. Berrange berrange at redhat.com
Mon Jan 22 10:57:57 UTC 2018


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 at redhat.com>
> ---
> 
> 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 at redhat.com>

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 :|




More information about the libvir-list mailing list