diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ea3558a3db..66bee49d6f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1032,13 +1032,6 @@ virQEMUCapsInit(virFileCachePtr cache) true, true)) == NULL) goto error; - /* Some machines have problematic NUMA topology causing - * unexpected failures. We don't want to break the QEMU - * driver in this scenario, so log errors & carry on - */ - if (!(caps->host.numa = virCapabilitiesHostNUMANewHost())) - goto error; - if (virCapabilitiesInitCaches(caps) < 0) VIR_WARN("Failed to get host CPU cache info"); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 053bcc7e02..ba3001ccd0 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1204,6 +1204,22 @@ virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver, } +virCapsHostNUMAPtr +virQEMUDriverGetHostNUMACaps(virQEMUDriverPtr driver) +{ + qemuDriverLock(driver); + + if (!driver->hostnuma) + driver->hostnuma = virCapabilitiesHostNUMANewHost(); + + qemuDriverUnlock(driver); + + virCapabilitiesHostNUMARef(driver->hostnuma); + + return driver->hostnuma; +} + + virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) { size_t i, j; @@ -1255,6 +1271,7 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) "DOI \"%s\"", model, doi); } + caps->host.numa = virQEMUDriverGetHostNUMACaps(driver); return g_steal_pointer(&caps); } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 8b6c2a95d4..2d03df9171 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -262,6 +262,11 @@ struct _virQEMUDriver { */ virCapsPtr caps; + /* Lazy initialized on first use, immutable thereafter. + * Require lock to get the pointer & do optional initialization + */ + virCapsHostNUMAPtr hostnuma; + /* Immutable value */ virArch hostarch; @@ -319,6 +324,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg); virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver); bool virQEMUDriverIsPrivileged(virQEMUDriverPtr driver); +virCapsHostNUMAPtr virQEMUDriverGetHostNUMACaps(virQEMUDriverPtr driver); virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver); virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr driver, bool refresh); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0aad086e7b..c741c2fcbd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2922,7 +2922,7 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt, qemuDomainObjPrivatePtr priv, virQEMUDriverPtr driver) { - virCapsPtr caps = NULL; + g_autoptr(virCapsHostNUMA) caps = NULL; char *nodeset; char *cpuset; int nodesetSize = 0; @@ -2935,15 +2935,15 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt, if (!nodeset && !cpuset) return 0; - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + if (!(caps = virQEMUDriverGetHostNUMACaps(driver))) goto cleanup; /* Figure out how big the nodeset bitmap needs to be. * This is necessary because NUMA node IDs are not guaranteed to * start from 0 or be densely allocated */ - for (i = 0; i < caps->host.numa->cells->len; i++) { + for (i = 0; i < caps->cells->len; i++) { virCapsHostNUMACellPtr cell = - g_ptr_array_index(caps->host.numa->cells, i); + g_ptr_array_index(caps->cells, i); nodesetSize = MAX(nodesetSize, cell->num + 1); } @@ -2957,7 +2957,7 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt, } else { /* autoNodeset is present in this case, since otherwise we wouldn't * reach this code */ - if (!(priv->autoCpuset = virCapabilitiesHostNUMAGetCpus(caps->host.numa, + if (!(priv->autoCpuset = virCapabilitiesHostNUMAGetCpus(caps, priv->autoNodeset))) goto cleanup; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 533c40d5ad..9b1abc39ff 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6129,14 +6129,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, static int -qemuProcessPrepareDomainNUMAPlacement(virDomainObjPtr vm, - virCapsPtr caps) +qemuProcessPrepareDomainNUMAPlacement(virQEMUDriverPtr driver, + virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - char *nodeset = NULL; - virBitmapPtr numadNodeset = NULL; - virBitmapPtr hostMemoryNodeset = NULL; - int ret = -1; + g_autofree char *nodeset = NULL; + g_autoptr(virBitmap) numadNodeset = NULL; + g_autoptr(virBitmap) hostMemoryNodeset = NULL; + g_autoptr(virCapsHostNUMA) caps = NULL; /* Get the advisory nodeset from numad if 'placement' of * either or is 'auto'. @@ -6148,33 +6148,30 @@ qemuProcessPrepareDomainNUMAPlacement(virDomainObjPtr vm, virDomainDefGetMemoryTotal(vm->def)); if (!nodeset) - goto cleanup; + return -1; if (!(hostMemoryNodeset = virNumaGetHostMemoryNodeset())) - goto cleanup; + return -1; VIR_DEBUG("Nodeset returned from numad: %s", nodeset); if (virBitmapParse(nodeset, &numadNodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto cleanup; + return -1; + + if (!(caps = virQEMUDriverGetHostNUMACaps(driver))) + return -1; /* numad may return a nodeset that only contains cpus but cgroups don't play * well with that. Set the autoCpuset from all cpus from that nodeset, but * assign autoNodeset only with nodes containing memory. */ - if (!(priv->autoCpuset = virCapabilitiesHostNUMAGetCpus(caps->host.numa, numadNodeset))) - goto cleanup; + if (!(priv->autoCpuset = virCapabilitiesHostNUMAGetCpus(caps, numadNodeset))) + return -1; virBitmapIntersect(numadNodeset, hostMemoryNodeset); priv->autoNodeset = g_steal_pointer(&numadNodeset); - ret = 0; - - cleanup: - VIR_FREE(nodeset); - virBitmapFree(numadNodeset); - virBitmapFree(hostMemoryNodeset); - return ret; + return 0; } @@ -6252,10 +6249,6 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, size_t i; qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - virCapsPtr caps; - - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; priv->machineName = qemuDomainGetMachineName(vm); if (!priv->machineName) @@ -6271,7 +6264,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, } virDomainAuditSecurityLabel(vm, true); - if (qemuProcessPrepareDomainNUMAPlacement(vm, caps) < 0) + if (qemuProcessPrepareDomainNUMAPlacement(driver, vm) < 0) goto cleanup; } @@ -6359,7 +6352,6 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, ret = 0; cleanup: - virObjectUnref(caps); virObjectUnref(cfg); return ret; } diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index f3009728a8..c01bbdd1a0 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -109,6 +109,18 @@ virFindFileInPath(const char *file) return NULL; } + +virCapsHostNUMAPtr +virCapabilitiesHostNUMANewHost(void) +{ + /* + * Build a NUMA topology with cell_id (NUMA node id + * being 3(0 + 3),4(1 + 3), 5 and 6 + */ + return virTestCapsBuildNUMATopology(3); +} + + static int testQemuAddGuest(virCapsPtr caps, virArch arch) @@ -201,11 +213,7 @@ virCapsPtr testQemuCapsInit(void) qemuTestSetHostCPU(caps, NULL); - /* - * Build a NUMA topology with cell_id (NUMA node id - * being 3(0 + 3),4(1 + 3), 5 and 6 - */ - if (!(caps->host.numa = virTestCapsBuildNUMATopology(3))) + if (!(caps->host.numa = virCapabilitiesHostNUMANewHost())) goto cleanup; for (i = 0; i < VIR_ARCH_LAST; i++) {