4:53 a.m.
We already check that there's no <memnode mode='restrictive'/> when
<memory mode='restrictive'/> is set. But we are missing the opposite
check: there's <memory mode='restrictive'/> when there is <memnode
mode='restrictive'/>.
Michal Prívozník (3):
virDomainNumatuneNodeSpecified: Fix const correctness
numa_conf: Move memnode mode validation into
virDomainNumaDefValidate()
numa_conf: Deny other memory modes than 'restrictive' if a memnode is
'restrictive'
src/conf/numa_conf.c | 26 ++++++++----
src/conf/numa_conf.h | 2 +-
...strictive-mode-err-mixed.x86_64-latest.err | 1 +
...une-memnode-restrictive-mode-err-mixed.xml | 41 +++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
5 files changed, 61 insertions(+), 10 deletions(-)
create mode 100644
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.x86_64-latest.err
create mode 100644
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.xml
--
2.39.3
4:53 a.m.
New subject: [PATCH 1/3] virDomainNumatuneNodeSpecified: Fix const correctness
The virDomainNumatuneNodeSpecified() function does not write into
passed @numatune pointer, it just reads from it. Therefore, the
argument should be const, which allows this function to be called
from places where virDomainNuma is already const (e.g. domain
validation code).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/numa_conf.c | 2 +-
src/conf/numa_conf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index be0c4572c5..dd4997c759 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -116,7 +116,7 @@ struct _virDomainNuma {
bool
-virDomainNumatuneNodeSpecified(virDomainNuma *numatune,
+virDomainNumatuneNodeSpecified(const virDomainNuma *numatune,
int cellid)
{
if (numatune &&
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index bbb928abb2..f5a20315b6 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -210,7 +210,7 @@ int virDomainNumatuneSpecifiedMaxNode(virDomainNuma *numatune);
bool virDomainNumatuneNodesetIsAvailable(virDomainNuma *numatune,
virBitmap *auto_nodeset);
-bool virDomainNumatuneNodeSpecified(virDomainNuma *numatune,
+bool virDomainNumatuneNodeSpecified(const virDomainNuma *numatune,
int cellid);
int virDomainNumaDefParseXML(virDomainNuma *def, xmlXPathContextPtr ctxt);
--
2.39.3
4:53 a.m.
New subject: [PATCH 2/3] numa_conf: Move memnode mode validation into virDomainNumaDefValidate()
When parsing a <memnode/> we also check whether the @mode
argument fulfills some requirements wrt 'restrictive' mode. This
is not the right place though. There's virDomainNumaDefValidate()
which contains other checks.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/numa_conf.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index dd4997c759..6095139385 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -191,14 +191,6 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa,
VIR_DOMAIN_NUMATUNE_MEM_STRICT) < 0)
return -1;
- if (numa->memory.mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
- mem_node->mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("'restrictive' mode is required in memnode
element "
- "when mode is 'restrictive' in memory
element"));
- return -1;
- }
-
tmp = virXMLPropString(cur_node, "nodeset");
if (!tmp) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1083,6 +1075,14 @@ virDomainNumaDefValidate(const virDomainNuma *def)
const virDomainNumaNode *node = &def->mem_nodes[i];
g_autoptr(virBitmap) levelsSeen = virBitmapNew(0);
+ if (virDomainNumatuneNodeSpecified(def, i) &&
+ def->memory.mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
+ node->mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'restrictive' mode is required in memnode
element when mode is 'restrictive' in memory element"));
+ return -1;
+ }
+
for (j = 0; j < node->ncaches; j++) {
const virNumaCache *cache = &node->caches[j];
--
2.39.3
4:53 a.m.
New subject: [PATCH 3/3] numa_conf: Deny other memory modes than 'restrictive' if a memnode is 'restrictive'
We already do check that if there's <memory mode='restrictive'/>
then all <memnode/> have to be of 'restrictive' mode too. But
what we are missing the reverse: if there is <memnode/> with
'restrictive' mode, then the <memory/> has to be of the same mode
too.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2208946
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/numa_conf.c | 20 ++++++---
...strictive-mode-err-mixed.x86_64-latest.err | 1 +
...une-memnode-restrictive-mode-err-mixed.xml | 41 +++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
4 files changed, 57 insertions(+), 6 deletions(-)
create mode 100644
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.x86_64-latest.err
create mode 100644
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.xml
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 6095139385..a616521763 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -1075,12 +1075,20 @@ virDomainNumaDefValidate(const virDomainNuma *def)
const virDomainNumaNode *node = &def->mem_nodes[i];
g_autoptr(virBitmap) levelsSeen = virBitmapNew(0);
- if (virDomainNumatuneNodeSpecified(def, i) &&
- def->memory.mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
- node->mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("'restrictive' mode is required in memnode
element when mode is 'restrictive' in memory element"));
- return -1;
+ if (virDomainNumatuneNodeSpecified(def, i)) {
+ if (def->memory.mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
+ node->mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'restrictive' mode is required in memnode
element when mode is 'restrictive' in memory element"));
+ return -1;
+ }
+
+ if (node->mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
+ def->memory.mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'restrictive' mode is required in memory
element when mode is 'restrictive' in memnode element"));
+ return -1;
+ }
}
for (j = 0; j < node->ncaches; j++) {
diff --git
a/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.x86_64-latest.err
b/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.x86_64-latest.err
new file mode 100644
index 0000000000..88f02ee70e
--- /dev/null
+++
b/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.x86_64-latest.err
@@ -0,0 +1 @@
+XML error: 'restrictive' mode is required in memory element when mode is
'restrictive' in memnode element
diff --git a/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.xml
b/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.xml
new file mode 100644
index 0000000000..0304dec8ef
--- /dev/null
+++ b/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.xml
@@ -0,0 +1,41 @@
+<domain type='qemu'>
+ <name>QEMUGuest</name>
+ <uuid>9f4b6512-e73a-4a25-93e8-5307802821ce</uuid>
+ <memory unit='KiB'>24682468</memory>
+ <currentMemory unit='KiB'>24682468</currentMemory>
+ <vcpu placement='static'>32</vcpu>
+ <numatune>
+ <memory mode='interleave' nodeset='0-3'/>
+ <memnode cellid='0' mode='strict' nodeset='3'/>
+ <memnode cellid='2' mode='restrictive' nodeset='1-2'/>
+ </numatune>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu mode='custom' match='exact' check='none'>
+ <model fallback='forbid'>qemu64</model>
+ <numa>
+ <cell id='0' cpus='0' memory='20002'
unit='KiB'/>
+ <cell id='1' cpus='1-27,29' memory='660066'
unit='KiB'/>
+ <cell id='2' cpus='28,30-31' memory='24002400'
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-system-x86_64</emulator>
+ <controller type='usb' index='0' model='piix3-uhci'>
+ <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'/>
+ <audio id='1' type='none'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 93d5ae018f..2cc9bd074a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1868,6 +1868,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("numatune-memnode");
DO_TEST_PARSE_ERROR_NOCAPS("numatune-memnode-invalid-mode");
DO_TEST_CAPS_LATEST("numatune-memnode-restrictive-mode");
+
DO_TEST_CAPS_LATEST_PARSE_ERROR("numatune-memnode-restrictive-mode-err-mixed");
DO_TEST_CAPS_LATEST("numatune-system-memory");
DO_TEST_NOCAPS("numatune-memnode-no-memory");
--
2.39.3
5:28 a.m.
On Mon, May 22, 2023 at 11:53:45AM +0200, Michal Privoznik wrote:
We already check that there's no <memnode
mode='restrictive'/> when
<memory mode='restrictive'/> is set. But we are missing the opposite
check: there's <memory mode='restrictive'/> when there is <memnode
mode='restrictive'/>.
Even though the logic is inverted in here it is correct both in the
patch and in the commit messages, so for the series:
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
Michal Prívozník (3):
virDomainNumatuneNodeSpecified: Fix const correctness
numa_conf: Move memnode mode validation into
virDomainNumaDefValidate()
numa_conf: Deny other memory modes than 'restrictive' if a memnode is
'restrictive'
src/conf/numa_conf.c | 26 ++++++++----
src/conf/numa_conf.h | 2 +-
...strictive-mode-err-mixed.x86_64-latest.err | 1 +
...une-memnode-restrictive-mode-err-mixed.xml | 41 +++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
5 files changed, 61 insertions(+), 10 deletions(-)
create mode 100644
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.x86_64-latest.err
create mode 100644
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode-err-mixed.xml
--
2.39.3
550
days inactive
550
days old
4 comments
2 participants
participants (2)
-
Martin Kletzander -
Michal Privoznik