On Fri, Apr 23, 2021 at 15:24:25 +0200, Michal Privoznik wrote:
> The aim of this function is to return whether domain definition
> and/or memory device that user intents to hotplug needs a private
> path inside cfg->memoryBackingDir. The rule for the memory device
> that's being hotplug includes checking whether corresponding
> guest NUMA node needs memoryBackingDir. Well, while the rationale
> behind makes sense it is not necessary to check for that really -
> just a few lines above every guest NUMA node was checked exactly
> for that.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_process.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 449e5f1547..a5f256154f 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3939,13 +3939,24 @@ qemuProcessNeedMemoryBackingPath(virDomainDef *def,
> return true;
> }
>
> - if (mem &&
> - mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM &&
> - (mem->access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT ||
> - (mem->targetNode >= 0 &&
> - virDomainNumaGetNodeMemoryAccessMode(def->numa, mem->targetNode)
> - != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)))
> - return true;
> + if (mem) {
> + switch (mem->model) {
> + case VIR_DOMAIN_MEMORY_MODEL_DIMM:
> + if (mem->access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) {
> + /* No need to check for access mode on the target node,
> + * it was checked for in the previous loop. */
> + return true;
> + }
missing 'break;' and fall-through doesn't make sense here.
> +
> + case VIR_DOMAIN_MEMORY_MODEL_NONE:
> + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
> + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
> + case VIR_DOMAIN_MEMORY_MODEL_LAST:
> + /* Backed by user provided path. Not stored in memory
> + * backing dir anyway. */
> + break;
> + }
> + }
With the above addressed:
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
