If qemu binary exposes kvm then caps depends heavily on host state.
We are already check explicitly some host features - kernel version,
microcode version, nested flag state. This won't nesserily help if somebody
change CPU on host. Let's instead invalidate such caps on every daemon
restart. This makes kernel version and microcode version check
excessive but nested flag is still need to be checked.

Cache invalidating on restart is archived by keeping list of
binaries that whose caps cache was validated at least once.
If binary in not yet validated and supports kvm then we need to
invalidate it's caps cache.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
---

If the patch will be accepted then I'll send a patch which removes mentioned
excessive checks also.

 src/qemu/qemu_capabilities.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 51bf97b..649aba2 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3264,6 +3264,7 @@ struct _virQEMUCapsCachePriv {
     virArch hostArch;
     unsigned int microcodeVersion;
     char *kernelVersion;
+    virHashTablePtr validated;
 
     /* cache whether /dev/kvm is usable as runUid:runGuid */
     virTristateBool kvmUsable;
@@ -3280,6 +3281,7 @@ virQEMUCapsCachePrivFree(void *privData)
 
     VIR_FREE(priv->libDir);
     VIR_FREE(priv->kernelVersion);
+    virHashFree(priv->validated);
     VIR_FREE(priv);
 }
 
@@ -3952,10 +3954,19 @@ virQEMUCapsIsValid(void *data,
     bool kvmUsable;
     struct stat sb;
     bool kvmSupportsNesting;
+    bool validated = true;
 
     if (!qemuCaps->binary)
         return true;
 
+    if (!virHashLookup(priv->validated, qemuCaps->binary)) {
+        validated = false;
+
+        /* If we fail to remember this binary is validated it is not problem,
+         * it will be validated again next time once again */
+        virHashAddEntry(priv->validated, qemuCaps->binary, (void*)1);
+    }
+
     if (qemuCaps->libvirtCtime != virGetSelfLastChanged() ||
         qemuCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) {
         VIR_DEBUG("Outdated capabilities for '%s': libvirt changed "
@@ -4011,6 +4022,12 @@ virQEMUCapsIsValid(void *data,
     }
 
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
+        if (!validated) {
+            VIR_DEBUG("Capabilities for QEMU that supports KVM need to be "
+                      "updated after daemon restart");
+            return false;
+        }
+
         if (priv->microcodeVersion != qemuCaps->microcodeVersion) {
             VIR_DEBUG("Outdated capabilities for '%s': microcode version "
                       "changed (%u vs %u)",
@@ -4809,6 +4826,9 @@ virQEMUCapsCacheNew(const char *libDir,
     if (VIR_STRDUP(priv->libDir, libDir) < 0)
         goto error;
 
+    if (!(priv->validated = virHashCreate(1, NULL)))
+        goto error;
+
     priv->hostArch = virArchFromHost();
 
     priv->runUid = runUid;
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Dec 28, 2018 at 15:06:36 +0300, Nikolay Shirokovskiy wrote:
> If qemu binary exposes kvm then caps depends heavily on host state.
> We are already check explicitly some host features - kernel version,
> microcode version, nested flag state. This won't nesserily help if somebody
> change CPU on host. Let's instead invalidate such caps on every daemon

Well, that isn't something that happens regularly. Also usually in that
case the OS is reinstalled as well.

> restart. This makes kernel version and microcode version check
> excessive but nested flag is still need to be checked.
> 
> Cache invalidating on restart is archived by keeping list of
> binaries that whose caps cache was validated at least once.
> If binary in not yet validated and supports kvm then we need to
> invalidate it's caps cache.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> ---
> 
> If the patch will be accepted then I'll send a patch which removes mentioned
> excessive checks also.

Well, the whole point of this is to shorten the startup time of the
daemon if nothing changed.

In some cases I would agree that we can do it on a host restart, but
dropping caches on daemon restart is too excessive. Also in that case
all caching on disk can be removed.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list