qemu: use NUMA capabilities object directly

Avoid grabbing the whole virCapsPtr object when we only need the
NUMA information.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

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");
 

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);
 }
 

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);

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;
     }

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 <vcpu> or <numatune> 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;
 }

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++) {