Re: [PATCH v3 03/14] qemu_process: Drop needless check in qemuProcessNeedMemoryBackingPath()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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>




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux