Memory-less numa nodes could be specified by setting the memory size to
0. Allow ommiting of the parameter completely to specify the same.

Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1217144
---
 docs/schemas/cputypes.rng                          | 12 ++++----
 src/conf/numa_conf.c                               | 11 ++++---
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args | 22 ++++++++++++++
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml  | 25 ++++++++++++++++
 tests/qemuxml2argvtest.c                           |  1 +
 .../qemuxml2xmlout-cpu-numa4.xml                   | 34 ++++++++++++++++++++++
 tests/qemuxml2xmltest.c                            |  1 +
 7 files changed, 97 insertions(+), 9 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa4.xml

diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng
index 7cc9dd3d8..fdac7878f 100644
--- a/docs/schemas/cputypes.rng
+++ b/docs/schemas/cputypes.rng
@@ -103,13 +103,15 @@
       <attribute name="cpus">
         <ref name="cpuset"/>
       </attribute>
-      <attribute name="memory">
-        <ref name="memoryKB"/>
-      </attribute>
       <optional>
-        <attribute name="unit">
-          <ref name="unit"/>
+        <attribute name="memory">
+          <ref name="memoryKB"/>
         </attribute>
+        <optional>
+          <attribute name="unit">
+            <ref name="unit"/>
+          </attribute>
+        </optional>
       </optional>
       <optional>
         <attribute name="memAccess">
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index bfd37032a..ee55b421b 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -774,7 +774,7 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def,

         ctxt->node = nodes[i];
         if (virDomainParseMemory("./@memory", "./@unit", ctxt,
-                                 &def->mem_nodes[cur_cell].mem, true, false) < 0)
+                                 &def->mem_nodes[cur_cell].mem, false, false) < 0)
             goto cleanup;

         if ((tmp = virXMLPropString(nodes[i], "memAccess"))) {
@@ -807,6 +807,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf,
     virDomainMemoryAccess memAccess;
     char *cpustr;
     size_t ncells = virDomainNumaGetNodeCount(def);
+    unsigned long long cellmem;
     size_t i;

     if (ncells == 0)
@@ -823,9 +824,11 @@ virDomainNumaDefCPUFormat(virBufferPtr buf,
         virBufferAddLit(buf, "<cell");
         virBufferAsprintf(buf, " id='%zu'", i);
         virBufferAsprintf(buf, " cpus='%s'", cpustr);
-        virBufferAsprintf(buf, " memory='%llu'",
-                          virDomainNumaGetNodeMemorySize(def, i));
-        virBufferAddLit(buf, " unit='KiB'");
+        cellmem = virDomainNumaGetNodeMemorySize(def, i);
+        if (cellmem > 0) {
+            virBufferAsprintf(buf, " memory='%llu'", cellmem);
+            virBufferAddLit(buf, " unit='KiB'");
+        }
         if (memAccess)
             virBufferAsprintf(buf, " memAccess='%s'",
                               virDomainMemoryAccessTypeToString(memAccess));
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args
new file mode 100644
index 000000000..db53409a4
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args
@@ -0,0 +1,22 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 107 \
+-smp 1,maxcpus=16,sockets=2,cores=4,threads=2 \
+-numa node,nodeid=0,cpus=0-7,mem=107 \
+-numa node,nodeid=1,cpus=8-15,mem=0 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot n \
+-usb \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml
new file mode 100644
index 000000000..af30f785f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml
@@ -0,0 +1,25 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static' current='1'>16</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='network'/>
+  </os>
+  <cpu>
+    <topology sockets='2' cores='4' threads='2'/>
+    <numa>
+      <cell id='1' cpus='8-15'/>
+      <cell id='0' cpus='0-7' memory='109550' unit='KiB'/>
+    </numa>
+  </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>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f55b04b05..7ef555736 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1521,6 +1521,7 @@ mymain(void)
     DO_TEST("cpu-strict1", QEMU_CAPS_KVM);
     DO_TEST("cpu-numa1", NONE);
     DO_TEST("cpu-numa2", NONE);
+    DO_TEST("cpu-numa4", NONE);
     DO_TEST("cpu-numa-no-memory-element", NONE);
     DO_TEST_PARSE_ERROR("cpu-numa3", NONE);
     DO_TEST_FAILURE("cpu-numa-disjoint", NONE);
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa4.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa4.xml
new file mode 100644
index 000000000..a5161a158
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa4.xml
@@ -0,0 +1,34 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static' current='1'>16</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='network'/>
+  </os>
+  <cpu>
+    <topology sockets='2' cores='4' threads='2'/>
+    <numa>
+      <cell id='0' cpus='0-7' memory='109550' unit='KiB'/>
+      <cell id='1' cpus='8-15'/>
+    </numa>
+  </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>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0702f581e..ae55d1d7f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -962,6 +962,7 @@ mymain(void)

     DO_TEST("cpu-numa1", NONE);
     DO_TEST("cpu-numa2", NONE);
+    DO_TEST("cpu-numa4", NONE);
     DO_TEST("cpu-numa-no-memory-element", NONE);
     DO_TEST("cpu-numa-disordered", NONE);
     DO_TEST("cpu-numa-disjoint", NONE);
-- 
2.11.1

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

On Tue, Feb 21, 2017 at 04:22:05PM +0100, Peter Krempa wrote:
> Memory-less numa nodes could be specified by setting the memory size to
> 0. Allow ommiting of the parameter completely to specify the same.
> 
> Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1217144

Does this correctly plumb through to the guest OS ?

Last time we tried fixing this bug we found some problems - I can't remember
whether the problem was with 0 CPUs or 0 memory, but with one of them, QEMU
would totally mangle the NUMA topology, merging nodes together.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

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

On Tue, Feb 21, 2017 at 15:36:23 +0000, Daniel Berrange wrote:
> On Tue, Feb 21, 2017 at 04:22:05PM +0100, Peter Krempa wrote:
> > Memory-less numa nodes could be specified by setting the memory size to
> > 0. Allow ommiting of the parameter completely to specify the same.
> > 
> > Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1217144
> 
> Does this correctly plumb through to the guest OS ?
> 
> Last time we tried fixing this bug we found some problems - I can't remember
> whether the problem was with 0 CPUs or 0 memory, but with one of them, QEMU
> would totally mangle the NUMA topology, merging nodes together.

I tried it and indeed the NUMA topology for a memory-less node shows up
totally broken in the guest. From your comments on the linked bugzilla
I thought that it worked properly if you specify memory='0' explicitly.

Since cpu-less nodes are possible for a long time and with the new
cpu-hotplug API you can select individual vcpus to hotplug according to
the id I was under the impression that only the syntax-sugar stuff was
necessary to cover everyting.

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

On Wed, Feb 22, 2017 at 10:04:08AM +0100, Peter Krempa wrote:
> On Tue, Feb 21, 2017 at 15:36:23 +0000, Daniel Berrange wrote:
> > On Tue, Feb 21, 2017 at 04:22:05PM +0100, Peter Krempa wrote:
> > > Memory-less numa nodes could be specified by setting the memory size to
> > > 0. Allow ommiting of the parameter completely to specify the same.
> > > 
> > > Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1217144
> > 
> > Does this correctly plumb through to the guest OS ?
> > 
> > Last time we tried fixing this bug we found some problems - I can't remember
> > whether the problem was with 0 CPUs or 0 memory, but with one of them, QEMU
> > would totally mangle the NUMA topology, merging nodes together.
> 
> I tried it and indeed the NUMA topology for a memory-less node shows up
> totally broken in the guest. From your comments on the linked bugzilla
> I thought that it worked properly if you specify memory='0' explicitly.
> 
> Since cpu-less nodes are possible for a long time and with the new
> cpu-hotplug API you can select individual vcpus to hotplug according to
> the id I was under the impression that only the syntax-sugar stuff was
> necessary to cover everyting.

So, IIRC, there were bugs in both QEMU and Linux in this respect. eg QEMU
mangles the topology it builds but even if QEMU built it right, than
Linux mangled it again.

What I'm unclear on though is whether the Linux behaviour is just an
accidental bug because no one has tried this before, or whether it is a
fundamental limitation of x86 architecture in Linux.

Second, even if it is broken on x86 on Linux, this config might work on
non-x86 archs with NUMA. If we can get clarity on this from the kernel
side, then we can decide if we need to fix QEMU or not.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

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