[PATCH] qemu_domain: Partially validate memory amounts when auto-adding NUMA node

[Kristina Hanicova]

On Fri, Jul 21, 2023 at 1:13 PM Michal Privoznik <mprivozn at redhat.com>
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 <mprivozn at redhat.com>
> ---
>  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 <khanicov at redhat.com>
Kristina
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230725/482a2386/attachment.htm>