qemu: Don't cache NUMA caps

In v6.0.0-rc1~439 (and friends) we tried to cache NUMA
capabilities because we assumed they are immutable. And to some
extent they are (NUMA hotplug is not a thing, is it). However,
our capabilities contain also some runtime info that can change,
e.g. hugepages pool allocation sizes or total amount of memory
per node (host side memory hotplug might change the value).

Because of the caching we might not be reporting the correct
runtime info in 'virsh capabilities'.

The NUMA caps are used in three places:

  1) 'virsh capabilities'
  2) domain startup, when parsing numad reply
  3) parsing domain private data XML

In cases 2) and 3) we need NUMA caps to construct list of
physical CPUs that belong to NUMA nodes from numad reply. And
while this may seem static, it's not really because of possible
CPU hotplug on physical host.

There are two possible approaches:

  1) build a validation mechanism that would invalidate the
     cached NUMA caps, or
  2) drop the caching and construct NUMA caps from scratch on
     each use.

In this commit, the latter approach is implemented, because it's
easier.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1819058
Fixes: 1a1d848694f6c2f1d98a371124928375bc3bb4a3
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a33e9923b42f2f936be6c1319e0e56ebe18e8fe1..151bea01ebbf53af3a1ece758d72d282734c221d 100644 (file)
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1288,27 +1288,6 @@ virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver,
 }
 
 
-virCapsHostNUMAPtr
-virQEMUDriverGetHostNUMACaps(virQEMUDriverPtr driver)
-{
-    virCapsHostNUMAPtr hostnuma;
-
-    qemuDriverLock(driver);
-
-    if (!driver->hostnuma)
-        driver->hostnuma = virCapabilitiesHostNUMANewHost();
-
-    hostnuma = driver->hostnuma;
-
-    qemuDriverUnlock(driver);
-
-    if (hostnuma)
-        virCapabilitiesHostNUMARef(hostnuma);
-
-    return hostnuma;
-}
-
-
 virCPUDefPtr
 virQEMUDriverGetHostCPU(virQEMUDriverPtr driver)
 {
@@ -1380,7 +1359,7 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)
                   "DOI \"%s\"", model, doi);
     }
 
-    caps->host.numa = virQEMUDriverGetHostNUMACaps(driver);
+    caps->host.numa = virCapabilitiesHostNUMANewHost();
     caps->host.cpu = virQEMUDriverGetHostCPU(driver);
     return g_steal_pointer(&caps);
 }

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 411d08db365b1137a4d8ff767b2b861095b7e447..7025b5222e8bd3aa883c489cda1d634efa74788f 100644 (file)
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -270,11 +270,6 @@ struct _virQEMUDriver {
      */
     virCapsPtr caps;
 
-    /* Lazy initialized on first use, immutable thereafter.
-     * Require lock to get the pointer & do optional initialization
-     */
-    virCapsHostNUMAPtr hostnuma;
-
     /* Lazy initialized on first use, immutable thereafter.
      * Require lock to get the pointer & do optional initialization
      */
@@ -337,7 +332,6 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg);
 
 virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver);
 
-virCapsHostNUMAPtr virQEMUDriverGetHostNUMACaps(virQEMUDriverPtr driver);
 virCPUDefPtr virQEMUDriverGetHostCPU(virQEMUDriverPtr driver);
 virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver);
 virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr driver,

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2275c83aa8516f938a7b0ac6f0b6fb5df91918c8..f14a15d3b4fad142ec5211027d29ab23c692c108 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2498,8 +2498,7 @@ qemuDomainObjPrivateXMLParseVcpu(xmlNodePtr node,
 
 static int
 qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt,
-                                               qemuDomainObjPrivatePtr priv,
-                                               virQEMUDriverPtr driver)
+                                               qemuDomainObjPrivatePtr priv)
 {
     g_autoptr(virCapsHostNUMA) caps = NULL;
     g_autofree char *nodeset = NULL;
@@ -2513,7 +2512,7 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt,
     if (!nodeset && !cpuset)
         return 0;
 
-    if (!(caps = virQEMUDriverGetHostNUMACaps(driver)))
+    if (!(caps = virCapabilitiesHostNUMANewHost()))
         return -1;
 
     /* Figure out how big the nodeset bitmap needs to be.
@@ -3114,7 +3113,7 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
     }
     VIR_FREE(nodes);
 
-    if (qemuDomainObjPrivateXMLParseAutomaticPlacement(ctxt, priv, driver) < 0)
+    if (qemuDomainObjPrivateXMLParseAutomaticPlacement(ctxt, priv) < 0)
         goto error;
 
     if ((tmp = virXPathString("string(./libDir/@path)", ctxt)))

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3051d2c7cae5b1efefb862f3861b4987a741c78b..d8478945d6273a061e80f1469946ece2edbc452f 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1136,7 +1136,6 @@ qemuStateCleanup(void)
     virObjectUnref(qemu_driver->qemuCapsCache);
     virObjectUnref(qemu_driver->xmlopt);
     virCPUDefFree(qemu_driver->hostcpu);
-    virCapabilitiesHostNUMAUnref(qemu_driver->hostnuma);
     virObjectUnref(qemu_driver->caps);
     ebtablesContextFree(qemu_driver->ebtables);
     VIR_FREE(qemu_driver->qemuImgBinary);

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index cbd29d867b126d715c3f2d7e21fe5f6316c7c081..e0b42ee2c92109552d9ab36d6149f412151516c8 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6197,8 +6197,7 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
 
 
 static int
-qemuProcessPrepareDomainNUMAPlacement(virQEMUDriverPtr driver,
-                                      virDomainObjPtr vm)
+qemuProcessPrepareDomainNUMAPlacement(virDomainObjPtr vm)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     g_autofree char *nodeset = NULL;
@@ -6226,7 +6225,7 @@ qemuProcessPrepareDomainNUMAPlacement(virQEMUDriverPtr driver,
     if (virBitmapParse(nodeset, &numadNodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
         return -1;
 
-    if (!(caps = virQEMUDriverGetHostNUMACaps(driver)))
+    if (!(caps = virCapabilitiesHostNUMANewHost()))
         return -1;
 
     /* numad may return a nodeset that only contains cpus but cgroups don't play
@@ -6428,7 +6427,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
         }
         virDomainAuditSecurityLabel(vm, true);
 
-        if (qemuProcessPrepareDomainNUMAPlacement(driver, vm) < 0)
+        if (qemuProcessPrepareDomainNUMAPlacement(vm) < 0)
             return -1;
     }