<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:large"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 21, 2023 at 1:13 PM Michal Privoznik <<a href="mailto:mprivozn@redhat.com">mprivozn@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">When automatically adding a NUMA node (qemuDomainDefNumaAutoAdd()) the<br>
memory size of the node is computed as:<br>
<br>
total_memory - sum(memory devices)<br>
<br>
And we have a nice helper for that: virDomainDefGetMemoryInitial() so<br>
it looks logical to just call it. Except, this code runs in post parse<br>
callback, i.e. memory sizes were not validated and it may happen that<br>
the sum is greater than the total memory. This would be caught by<br>
virDomainDefPostParseMemory() but that runs only after driver specific<br>
callbacks (i.e. after qemuDomainDefNumaAutoAdd()) and because the<br>
domain config was changed and memory was increased to this huge<br>
number no error is caught.<br>
<br>
So let's do what virDomainDefGetMemoryInitial() would do, but<br>
with error checking.<br>
<br>
Resolves: <a href="https://bugzilla.redhat.com/show_bug.cgi?id=2216236" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=2216236</a><br>
Fixes: f5d4f5c8ee44e9f1939070afcc5381bdd5545e50<br>
Signed-off-by: Michal Privoznik <<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>><br>
---<br>
src/qemu/qemu_domain.c | 13 +++++++++++--<br>
1 file changed, 11 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c<br>
index 6eea8a9fa5..fdda001795 100644<br>
--- a/src/qemu/qemu_domain.c<br>
+++ b/src/qemu/qemu_domain.c<br>
@@ -4821,17 +4821,24 @@ qemuDomainDefNumaAutoAdd(virDomainDef *def,<br>
return 0;<br>
}<br>
<br>
- initialMem = virDomainDefGetMemoryInitial(def);<br>
+ initialMem = virDomainDefGetMemoryTotal(def);<br></blockquote><div><font size="2"><br></font></div><div><div style="font-size:large" class="gmail_default"><font size="2">I would prefer if this variable was renamed to totalMem / remainingMem / availableMem.</font><br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
if (!def->numa)<br>
def->numa = virDomainNumaNew();<br>
<br>
virDomainNumaSetNodeCount(def->numa, 1);<br>
- virDomainNumaSetNodeMemorySize(def->numa, 0, initialMem);<br>
<br>
for (i = 0; i < def->nmems; i++) {<br>
virDomainMemoryDef *mem = def->mems[i];<br>
<br>
+ if (mem->size > initialMem) {<br>
+ virReportError(VIR_ERR_XML_ERROR, "%s",<br>
+ _("Total size of memory devices exceeds the total memory size"));<br>
+ return -1;<br>
+ }<br>
+<br>
+ initialMem -= mem->size;<br>
+<br>
switch (mem->model) {<br>
case VIR_DOMAIN_MEMORY_MODEL_DIMM:<br>
case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:<br>
@@ -4848,6 +4855,8 @@ qemuDomainDefNumaAutoAdd(virDomainDef *def,<br>
}<br>
}<br>
<br>
+ virDomainNumaSetNodeMemorySize(def->numa, 0, initialMem);<br>
+<br>
return 0;<br>
}<br>
<br>
-- <br>
2.41.0<br>
<br></blockquote><div><br></div><div><div><div><div style="font-size:large" class="gmail_default"><font size="2">Reviewed-by: Kristina Hanicova <<a href="mailto:khanicov@redhat.com" target="_blank">khanicov@redhat.com</a>></font></div><font style="--darkreader-inline-color: #b2a696;" color="#888888"><font style="--darkreader-inline-color: #b2a696;" color="#888888"><font style="--darkreader-inline-color: #b2a696;" color="#888888"><font style="--darkreader-inline-color: #b2a696;" color="#888888"><font style="--darkreader-inline-color: #b2a696;" color="#888888"><div style="font-size:large" class="gmail_default"><font size="2">Kristina</font></div></font></font></font></font></font></div></div></div></div></div>