From 40a162f83e3f34ad9ef92f3b87e4077de7e105af Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 2 Dec 2020 09:26:30 +0100 Subject: [PATCH] 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 Reviewed-by: Daniel Henrique Barboza --- src/qemu/qemu_conf.c | 23 +---------------------- src/qemu/qemu_conf.h | 6 ------ src/qemu/qemu_domain.c | 7 +++---- src/qemu/qemu_driver.c | 1 - src/qemu/qemu_process.c | 7 +++---- 5 files changed, 7 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a33e9923b4..151bea01eb 100644 --- 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 411d08db36..7025b5222e 100644 --- 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 2275c83aa8..f14a15d3b4 100644 --- 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 3051d2c7ca..d8478945d6 100644 --- 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 cbd29d867b..e0b42ee2c9 100644 --- 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; }