Previously we were ignoring "nodeset" attribute for hugepage pages
if there was no guest NUMA topology configured in the domain XML.
Commit <fa6bdf6afa878b8d7c5ed71664ee72be8967cdc5> partially fixed
that issue but it introduced a somehow valid regression.
In case that there is no guest NUMA topology configured and the
"nodeset" attribute is set to "0" it was accepted and was working
properly even though it was not completely valid XML.
This patch introduces a workaround that it will ignore the nodeset="0"
only in case that there is no guest NUMA topology in order not to
hit the validation error.
After this commit the following XML configuration is valid:
<memoryBacking>
<hugepages>
<page size='2048' unit='KiB' nodeset='0'/>
</hugepages>
</memoryBacking>
but this configuration remains invalid:
<memoryBacking>
<hugepages>
<page size='2048' unit='KiB' nodeset='0'/>
<page size='1048576' unit='KiB'/>
</hugepages>
</memoryBacking>
The issue with the second configuration is that it was originally
working, however changing the order of the <page> elements resolved
into using different page size for the guest. The code is written
in a way that it expect only one page configured and always uses only
the first page in case that there is no guest NUMA topology configured.
See qemuBuildMemPathStr() function for details.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1591235
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
src/conf/domain_conf.c | 27 +++++++++++++++++
tests/qemuxml2argvdata/hugepages-pages10.args | 26 ++++++++++++++++
tests/qemuxml2argvtest.c | 2 +-
.../qemuxml2xmloutdata/hugepages-pages10.xml | 30 +++++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
5 files changed, 85 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/hugepages-pages10.args
create mode 100644 tests/qemuxml2xmloutdata/hugepages-pages10.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5249f59d1a..bf5000f7a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4085,6 +4085,31 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
}
+static void
+virDomainDefPostParseMemtune(virDomainDefPtr def)
+{
+ size_t i;
+
+ if (virDomainNumaGetNodeCount(def->numa) == 0) {
+ /* If guest NUMA is not configured and any hugepage page has nodemask
+ * set to "0" free and clear that nodemas, otherwise we would rise
+ * an error that there is no guest NUMA node configured. */
+ for (i = 0; i < def->mem.nhugepages; i++) {
+ ssize_t nextBit;
+
+ if (!def->mem.hugepages[i].nodemask)
+ continue;
+
+ nextBit = virBitmapNextSetBit(def->mem.hugepages[i].nodemask, 0);
+ if (nextBit < 0) {
+ virBitmapFree(def->mem.hugepages[i].nodemask);
+ def->mem.hugepages[i].nodemask = NULL;
+ }
+ }
+ }
+}
+
+
static int
virDomainDefAddConsoleCompat(virDomainDefPtr def)
{
@@ -5134,6 +5159,8 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
if (virDomainDefPostParseMemory(def, data->parseFlags) < 0)
return -1;
+ virDomainDefPostParseMemtune(def);
+
if (virDomainDefRejectDuplicateControllers(def) < 0)
return -1;
diff --git a/tests/qemuxml2argvdata/hugepages-pages10.args b/tests/qemuxml2argvdata/hugepages-pages10.args
new file mode 100644
index 0000000000..d094be1252
--- /dev/null
+++ b/tests/qemuxml2argvdata/hugepages-pages10.args
@@ -0,0 +1,26 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name SomeDummyHugepagesGuest \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 1024 \
+-mem-prealloc \
+-mem-path /dev/hugepages2M/libvirt/qemu/-1-SomeDummyHugepagesGu \
+-smp 2,sockets=2,cores=1,threads=1 \
+-uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,\
+path=/tmp/lib/domain--1-SomeDummyHugepagesGu/monitor.sock,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot c \
+-usb
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 15f9fb7b11..6a57c419d1 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -981,7 +981,7 @@ mymain(void)
QEMU_CAPS_OBJECT_MEMORY_FILE,
QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD);
DO_TEST_PARSE_ERROR("hugepages-pages9", NONE);
- DO_TEST_PARSE_ERROR("hugepages-pages10", NONE);
+ DO_TEST("hugepages-pages10", NONE);
DO_TEST("hugepages-memaccess", QEMU_CAPS_OBJECT_MEMORY_FILE,
QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_DEVICE_PC_DIMM,
QEMU_CAPS_NUMA);
diff --git a/tests/qemuxml2xmloutdata/hugepages-pages10.xml b/tests/qemuxml2xmloutdata/hugepages-pages10.xml
new file mode 100644
index 0000000000..ac219a7800
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/hugepages-pages10.xml
@@ -0,0 +1,30 @@
+<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'/>
+ </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>
+ <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='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index a70516ada1..052c3fe387 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -337,6 +337,7 @@ mymain(void)
DO_TEST("hugepages-pages5", NONE);
DO_TEST("hugepages-pages6", NONE);
DO_TEST("hugepages-pages7", NONE);
+ DO_TEST("hugepages-pages10", NONE);
DO_TEST("hugepages-shared", NONE);
DO_TEST("hugepages-memaccess", NONE);
DO_TEST("hugepages-memaccess2", NONE);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/11/2018 10:22 AM, Pavel Hrdina wrote:
> Previously we were ignoring "nodeset" attribute for hugepage pages
> if there was no guest NUMA topology configured in the domain XML.
> Commit <fa6bdf6afa878b8d7c5ed71664ee72be8967cdc5> partially fixed
> that issue but it introduced a somehow valid regression.
>
> In case that there is no guest NUMA topology configured and the
> "nodeset" attribute is set to "0" it was accepted and was working
> properly even though it was not completely valid XML.
>
> This patch introduces a workaround that it will ignore the nodeset="0"
> only in case that there is no guest NUMA topology in order not to
> hit the validation error.
>
> After this commit the following XML configuration is valid:
>
> <memoryBacking>
> <hugepages>
> <page size='2048' unit='KiB' nodeset='0'/>
> </hugepages>
> </memoryBacking>
>
> but this configuration remains invalid:
>
> <memoryBacking>
> <hugepages>
> <page size='2048' unit='KiB' nodeset='0'/>
> <page size='1048576' unit='KiB'/>
> </hugepages>
> </memoryBacking>
>
> The issue with the second configuration is that it was originally
> working, however changing the order of the <page> elements resolved
> into using different page size for the guest. The code is written
> in a way that it expect only one page configured and always uses only
> the first page in case that there is no guest NUMA topology configured.
> See qemuBuildMemPathStr() function for details.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1591235
>
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> src/conf/domain_conf.c | 27 +++++++++++++++++
> tests/qemuxml2argvdata/hugepages-pages10.args | 26 ++++++++++++++++
> tests/qemuxml2argvtest.c | 2 +-
> .../qemuxml2xmloutdata/hugepages-pages10.xml | 30 +++++++++++++++++++
> tests/qemuxml2xmltest.c | 1 +
> 5 files changed, 85 insertions(+), 1 deletion(-)
> create mode 100644 tests/qemuxml2argvdata/hugepages-pages10.args
> create mode 100644 tests/qemuxml2xmloutdata/hugepages-pages10.xml
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5249f59d1a..bf5000f7a2 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4085,6 +4085,31 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
> }
>
>
> +static void
> +virDomainDefPostParseMemtune(virDomainDefPtr def)
> +{
> + size_t i;
> +
> + if (virDomainNumaGetNodeCount(def->numa) == 0) {
> + /* If guest NUMA is not configured and any hugepage page has nodemask
> + * set to "0" free and clear that nodemas, otherwise we would rise
> + * an error that there is no guest NUMA node configured. */
> + for (i = 0; i < def->mem.nhugepages; i++) {
> + ssize_t nextBit;
> +
> + if (!def->mem.hugepages[i].nodemask)
> + continue;
> +
> + nextBit = virBitmapNextSetBit(def->mem.hugepages[i].nodemask, 0);
> + if (nextBit < 0) {
> + virBitmapFree(def->mem.hugepages[i].nodemask);
> + def->mem.hugepages[i].nodemask = NULL;
> + }
> + }
> + }
> +}
> +
> +
Problem is not that there is no guest NUMA node. The real problem is
that there is no guest NUMA node left for the default <page/>. Consider
the following example:
<memoryBacking>
<hugepages>
<page size='2048' unit='KiB' nodeset='0-1'/>
<page size='1048576' unit='KiB'/>
</hugepages>
</memoryBacking>
<cpu mode='host-passthrough' check='none'>
<topology sockets='1' cores='4' threads='2'/>
<numa>
<cell id='0' cpus='0-3' memory='4194304' unit='KiB'/>
<cell id='1' cpus='4-7' memory='4194304' unit='KiB'/>
</numa>
</cpu>
Do you consider this configuration valid?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 11, 2018 at 05:05:05PM +0200, Michal Privoznik wrote:
> On 07/11/2018 10:22 AM, Pavel Hrdina wrote:
> > Previously we were ignoring "nodeset" attribute for hugepage pages
> > if there was no guest NUMA topology configured in the domain XML.
> > Commit <fa6bdf6afa878b8d7c5ed71664ee72be8967cdc5> partially fixed
> > that issue but it introduced a somehow valid regression.
> >
> > In case that there is no guest NUMA topology configured and the
> > "nodeset" attribute is set to "0" it was accepted and was working
> > properly even though it was not completely valid XML.
> >
> > This patch introduces a workaround that it will ignore the nodeset="0"
> > only in case that there is no guest NUMA topology in order not to
> > hit the validation error.
> >
> > After this commit the following XML configuration is valid:
> >
> > <memoryBacking>
> > <hugepages>
> > <page size='2048' unit='KiB' nodeset='0'/>
> > </hugepages>
> > </memoryBacking>
> >
> > but this configuration remains invalid:
> >
> > <memoryBacking>
> > <hugepages>
> > <page size='2048' unit='KiB' nodeset='0'/>
> > <page size='1048576' unit='KiB'/>
> > </hugepages>
> > </memoryBacking>
> >
> > The issue with the second configuration is that it was originally
> > working, however changing the order of the <page> elements resolved
> > into using different page size for the guest. The code is written
> > in a way that it expect only one page configured and always uses only
> > the first page in case that there is no guest NUMA topology configured.
> > See qemuBuildMemPathStr() function for details.
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1591235
> >
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> > src/conf/domain_conf.c | 27 +++++++++++++++++
> > tests/qemuxml2argvdata/hugepages-pages10.args | 26 ++++++++++++++++
> > tests/qemuxml2argvtest.c | 2 +-
> > .../qemuxml2xmloutdata/hugepages-pages10.xml | 30 +++++++++++++++++++
> > tests/qemuxml2xmltest.c | 1 +
> > 5 files changed, 85 insertions(+), 1 deletion(-)
> > create mode 100644 tests/qemuxml2argvdata/hugepages-pages10.args
> > create mode 100644 tests/qemuxml2xmloutdata/hugepages-pages10.xml
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 5249f59d1a..bf5000f7a2 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -4085,6 +4085,31 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
> > }
> >
> >
> > +static void
> > +virDomainDefPostParseMemtune(virDomainDefPtr def)
> > +{
> > + size_t i;
> > +
> > + if (virDomainNumaGetNodeCount(def->numa) == 0) {
> > + /* If guest NUMA is not configured and any hugepage page has nodemask
> > + * set to "0" free and clear that nodemas, otherwise we would rise
> > + * an error that there is no guest NUMA node configured. */
> > + for (i = 0; i < def->mem.nhugepages; i++) {
> > + ssize_t nextBit;
> > +
> > + if (!def->mem.hugepages[i].nodemask)
> > + continue;
> > +
> > + nextBit = virBitmapNextSetBit(def->mem.hugepages[i].nodemask, 0);
> > + if (nextBit < 0) {
> > + virBitmapFree(def->mem.hugepages[i].nodemask);
> > + def->mem.hugepages[i].nodemask = NULL;
> > + }
> > + }
> > + }
> > +}
> > +
> > +
>
> Problem is not that there is no guest NUMA node. The real problem is
> that there is no guest NUMA node left for the default <page/>. Consider
> the following example:
It's not that simple. For non-NUMA guest
<memoryBacking>
<hugepages>
<page size='2048' unit='KiB' nodeset='0'/>
<page size='1048576' unit='KiB'/>
</hugepages>
</memoryBacking>
will start a guest with 2M pages but
<memoryBacking>
<hugepages>
<page size='1048576' unit='KiB'/>
<page size='2048' unit='KiB' nodeset='0'/>
</hugepages>
</memoryBacking>
will start a guest with 1G pages. That's wrong and it should not
depend on the order.
This patch fixes this XML to work again:
<memoryBacking>
<hugepages>
<page size='2048' unit='KiB' nodeset='0'/>
</hugepages>
</memoryBacking>
by changing the parsed data into:
<memoryBacking>
<hugepages>
<page size='2048' unit='KiB'/>
</hugepages>
</memoryBacking>
which will pass the validation.
As a side-effect it will also fix previous case by changing it into:
<memoryBacking>
<hugepages>
<page size='1048576' unit='KiB'/>
<page size='2048' unit='KiB'/>
</hugepages>
</memoryBacking>
Which will fail the validation as there are two default pages.
All of this applies only to non-NUMA guests.
> <memoryBacking>
> <hugepages>
> <page size='2048' unit='KiB' nodeset='0-1'/>
> <page size='1048576' unit='KiB'/>
> </hugepages>
> </memoryBacking>
>
> <cpu mode='host-passthrough' check='none'>
> <topology sockets='1' cores='4' threads='2'/>
> <numa>
> <cell id='0' cpus='0-3' memory='4194304' unit='KiB'/>
> <cell id='1' cpus='4-7' memory='4194304' unit='KiB'/>
> </numa>
> </cpu>
>
> Do you consider this configuration valid?
Completely different issue that this patch is not trying to solve
but we can handle that as well.
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.