[libvirt] [PATCH] qemu: Deny hugepages for non-existent NUMA nodes

Michal Privoznik posted 1 patch 5 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/a20b0c4c6110548e9c09dce950960b46495b669e.1526641458.git.mprivozn@redhat.com
Test syntax-check passed
src/qemu/qemu_command.c                     | 10 +++++++
tests/qemuxml2argvdata/hugepages-pages8.xml | 41 +++++++++++++++++++++++++++++
tests/qemuxml2argvtest.c                    |  3 +++
3 files changed, 54 insertions(+)
create mode 100644 tests/qemuxml2argvdata/hugepages-pages8.xml
[libvirt] [PATCH] qemu: Deny hugepages for non-existent NUMA nodes
Posted by Michal Privoznik 5 years, 11 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1534418

Just like ec982f6d929f3c23 denies hugepages for non-existent
guest NUMA nodes in case there are some nodes configured.
Unfortunately, when there are none, qemuBuildNumaArgStr() is not
called and thus we have to have check in qemuBuildMemPathStr()
too.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_command.c                     | 10 +++++++
 tests/qemuxml2argvdata/hugepages-pages8.xml | 41 +++++++++++++++++++++++++++++
 tests/qemuxml2argvtest.c                    |  3 +++
 3 files changed, 54 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/hugepages-pages8.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f3724a766b..af0af33bd1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7276,6 +7276,16 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
     if (!def->mem.nhugepages)
         return 0;
 
+    if (def->mem.hugepages[0].nodemask) {
+        ssize_t next_bit = virBitmapNextSetBit(def->mem.hugepages[0].nodemask, -1);
+        if (next_bit >= 0) {
+            virReportError(VIR_ERR_XML_DETAIL,
+                           _("hugepages: node %zd not found"),
+                           next_bit);
+            return -1;
+        }
+    }
+
     /* There is one special case: if user specified "huge"
      * pages of regular system pages size.
      * And there is nothing to do in this case.
diff --git a/tests/qemuxml2argvdata/hugepages-pages8.xml b/tests/qemuxml2argvdata/hugepages-pages8.xml
new file mode 100644
index 0000000000..4cf4c1a8ad
--- /dev/null
+++ b/tests/qemuxml2argvdata/hugepages-pages8.xml
@@ -0,0 +1,41 @@
+<domain type='qemu'>
+  <name>SomeDummyHugepagesGuest</name>
+  <uuid>ef1bdff4-27f3-4e85-a807-5fb4d58463cc</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <memoryBacking>
+    <hugepages>
+      <page size='2048' unit='KiB' nodeset='1'/>
+    </hugepages>
+  </memoryBacking>
+  <vcpu placement='static'>2</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 78454acb1a..19801953e5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -937,6 +937,9 @@ mymain(void)
     DO_TEST("hugepages-pages7",
             QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_OBJECT_MEMORY_FILE,
             QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD);
+    DO_TEST_FAILURE("hugepages-pages8",
+                    QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_OBJECT_MEMORY_FILE,
+                    QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD);
     DO_TEST("hugepages-memaccess", QEMU_CAPS_OBJECT_MEMORY_FILE,
             QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_DEVICE_PC_DIMM,
             QEMU_CAPS_NUMA);
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Deny hugepages for non-existent NUMA nodes
Posted by John Ferlan 5 years, 11 months ago

On 05/18/2018 07:04 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1534418
> 
> Just like ec982f6d929f3c23 denies hugepages for non-existent
> guest NUMA nodes in case there are some nodes configured.
> Unfortunately, when there are none, qemuBuildNumaArgStr() is not
> called and thus we have to have check in qemuBuildMemPathStr()
> too.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_command.c                     | 10 +++++++
>  tests/qemuxml2argvdata/hugepages-pages8.xml | 41 +++++++++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                    |  3 +++
>  3 files changed, 54 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/hugepages-pages8.xml
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

although I'll admit following the reasoning why when
virDomainNumaGetNodeCount is the gate for both qemuBuildNumaArgStr and
qemuBuildMemPathStr didn't make it "easy to understand" ;-).

May be a "/* NB: qemuBuildNumaArgStr was only called when
virDomainNumaGetNodeCount finds numa, so a similar check is not made. */

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