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