From b9ff7393bca5df988e182b45ede34c636b0ae90d Mon Sep 17 00:00:00 2001
From: Erik Skultety <eskultet@redhat.com>
Date: Fri, 22 Aug 2014 16:05:37 +0200
Subject: [PATCH] numatune: setting --mode does not work well

When trying to set numatune mode directly using virsh numatune command,
correct error is raised, however numatune structure was not deallocated,
thus resulting in creating an empty numatune element in the guest XML,
if none was present before. Running the same command aftewards results
in a successful change with broken XML structure. Patch fixes the
deallocation problem as well as checking for invalid attribute
combination VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO + a nonempty nodeset.

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1129998
---
 src/conf/numatune_conf.c                      | 45 ++++++++++++-------
 ...d-auto-vcpu-static-numatune-no-nodeset.xml | 31 +++++++++++++
 tests/qemuxml2argvtest.c                      |  1 +
 3 files changed, 62 insertions(+), 15 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 48d1d04fe6..ff0f3eb34e 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -437,14 +437,27 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
                      int mode,
                      virBitmapPtr nodeset)
 {
-    bool create = !*numatunePtr;  /* Whether we are creating new struct */
+    bool created = false;
     int ret = -1;
-    virDomainNumatunePtr numatune = NULL;
+    virDomainNumatunePtr numatune;
 
     /* No need to do anything in this case */
     if (mode == -1 && placement == -1 && !nodeset)
         return 0;
 
+    if (!(*numatunePtr)) {
+        if (VIR_ALLOC(*numatunePtr) < 0)
+            goto cleanup;
+
+        created = true;
+        if (mode == -1)
+            mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
+        if (placement == -1)
+            placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_DEFAULT;
+    }
+
+    numatune = *numatunePtr;
+
     /* Range checks */
     if (mode != -1 &&
         (mode < 0 || mode >= VIR_DOMAIN_NUMATUNE_MEM_LAST)) {
@@ -453,6 +466,7 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
                        mode);
         goto cleanup;
     }
+
     if (placement != -1 &&
         (placement < 0 || placement >= VIR_DOMAIN_NUMATUNE_PLACEMENT_LAST)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -461,21 +475,9 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
         goto cleanup;
     }
 
-    if (create && VIR_ALLOC(*numatunePtr) < 0)
-        goto cleanup;
-    numatune = *numatunePtr;
-
-    if (create) {
-        /* Defaults for new struct */
-        if (mode == -1)
-            mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
-
-        if (placement == -1)
-            placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_DEFAULT;
-    }
-
     if (mode != -1)
         numatune->memory.mode = mode;
+
     if (nodeset) {
         virBitmapFree(numatune->memory.nodeset);
         numatune->memory.nodeset = virBitmapNewCopy(nodeset);
@@ -500,13 +502,26 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
         goto cleanup;
     }
 
+    /* setting nodeset when placement auto is invalid */
+    if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO &&
+        numatune->memory.nodeset) {
+        virBitmapFree(numatune->memory.nodeset);
+        numatune->memory.nodeset = NULL;
+    }
+
     if (placement != -1)
         numatune->memory.placement = placement;
 
     numatune->memory.specified = true;
 
     ret = 0;
+
  cleanup:
+    if (ret < 0 && created) {
+        virDomainNumatuneFree(*numatunePtr);
+        *numatunePtr = NULL;
+    }
+
     return ret;
 }
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml b/tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml
new file mode 100644
index 0000000000..b3cb5c5f42
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='auto'>2</vcpu>
+  <numatune>
+    <memory mode='interleave' placement='static'/>
+  </numatune>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu>
+    <topology sockets='2' cores='1' threads='1'/>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='ide' index='0'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 65dc9c708f..b289d0e843 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1223,6 +1223,7 @@ mymain(void)
     DO_TEST_PARSE_ERROR("numatune-memnodes-problematic", NONE);
     DO_TEST("numad", NONE);
     DO_TEST("numad-auto-vcpu-static-numatune", NONE);
+    DO_TEST_PARSE_ERROR("numad-auto-vcpu-static-numatune-no-nodeset", NONE);
     DO_TEST("numad-auto-memory-vcpu-cpuset", NONE);
     DO_TEST("numad-auto-memory-vcpu-no-cpuset-and-placement", NONE);
     DO_TEST("numad-static-memory-auto-vcpu", NONE);