Remove the code from qemuProcessPrepareDomain so that it won't get even
more bloated.
---
 src/qemu/qemu_process.c | 61 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e6522a294..01fe33c92 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5410,6 +5410,44 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
 }


+static int
+qemuProcessPrepareDomainNUMAPlacement(virDomainObjPtr vm,
+                                      virCapsPtr caps)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    char *nodesetstr = NULL;
+    int ret = -1;
+
+    /* Get the advisory nodeset from numad if 'placement' of
+     * either <vcpu> or <numatune> is 'auto'.
+     */
+    if (!virDomainDefNeedsPlacementAdvice(vm->def))
+        return 0;
+
+    nodesetstr = virNumaGetAutoPlacementAdvice(virDomainDefGetVcpus(vm->def),
+                                               virDomainDefGetMemoryTotal(vm->def));
+
+    if (!nodesetstr)
+        goto cleanup;
+
+    VIR_DEBUG("Nodeset returned from numad: %s", nodesetstr);
+
+    if (virBitmapParse(nodesetstr, &priv->autoNodeset,
+                       VIR_DOMAIN_CPUMASK_LEN) < 0)
+        goto cleanup;
+
+    if (!(priv->autoCpuset = virCapabilitiesGetCpusForNodemask(caps,
+                                                               priv->autoNodeset)))
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(nodesetstr);
+    return ret;
+}
+
+
 /**
  * qemuProcessPrepareDomain
  *
@@ -5430,7 +5468,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
 {
     int ret = -1;
     size_t i;
-    char *nodeset = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     virCapsPtr caps;
@@ -5448,25 +5485,8 @@ qemuProcessPrepareDomain(virConnectPtr conn,
         }
         virDomainAuditSecurityLabel(vm, true);

-        /* Get the advisory nodeset from numad if 'placement' of
-         * either <vcpu> or <numatune> is 'auto'.
-         */
-        if (virDomainDefNeedsPlacementAdvice(vm->def)) {
-            nodeset = virNumaGetAutoPlacementAdvice(virDomainDefGetVcpus(vm->def),
-                                                    virDomainDefGetMemoryTotal(vm->def));
-            if (!nodeset)
-                goto cleanup;
-
-            VIR_DEBUG("Nodeset returned from numad: %s", nodeset);
-
-            if (virBitmapParse(nodeset, &priv->autoNodeset,
-                               VIR_DOMAIN_CPUMASK_LEN) < 0)
-                goto cleanup;
-
-            if (!(priv->autoCpuset = virCapabilitiesGetCpusForNodemask(caps,
-                                                                       priv->autoNodeset)))
-                goto cleanup;
-        }
+        if (qemuProcessPrepareDomainNUMAPlacement(vm, caps) < 0)
+            goto cleanup;
     }

     /* Whether we should use virtlogd as stdio handler for character
@@ -5537,7 +5557,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,

     ret = 0;
  cleanup:
-    VIR_FREE(nodeset);
     virObjectUnref(caps);
     virObjectUnref(cfg);
     return ret;
-- 
2.12.2

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

On Wed, 2017-07-12 at 15:44 +0200, Peter Krempa wrote:
> +static int
> +qemuProcessPrepareDomainNUMAPlacement(virDomainObjPtr vm,
> +                                      virCapsPtr caps)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    char *nodesetstr = NULL;
> +    int ret = -1;
> +
> +    /* Get the advisory nodeset from numad if 'placement' of
> +     * either <vcpu> or <numatune> is 'auto'.
> +     */
> +    if (!virDomainDefNeedsPlacementAdvice(vm->def))
> +        return 0;
> +
> +    nodesetstr = virNumaGetAutoPlacementAdvice(virDomainDefGetVcpus(vm->def),
> +                                               virDomainDefGetMemoryTotal(vm->def));
> +
> +    if (!nodesetstr)
> +        goto cleanup;
> +
> +    VIR_DEBUG("Nodeset returned from numad: %s", nodesetstr);
> +
> +    if (virBitmapParse(nodesetstr, &priv->autoNodeset,
> +                       VIR_DOMAIN_CPUMASK_LEN) < 0)

This call is not any longer than others before or after it,
so there's no reason IMHO to distribute it among two lines.

You could even rename 'nodesetstr' to 'nodeset', which is
the name you've used for the same purpose in other places,
and shorten it a bit further ;)


Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

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