Re: [PATCH 10/14] qemu: Add @nodemaskRet argument to qemuBuildMemoryBackendProps()

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

 



On Wed, Mar 08, 2023 at 12:14:37PM +0100, Michal Privoznik wrote:
> -    /* Make sure the requested nodeset is sensible */
> -    if (nodemask && !virNumaNodesetIsAvailable(nodemask))
> -        return -1;
> -
> -    /* If mode is "restrictive", we should only use cgroups setting allowed memory
> -     * nodes, and skip passing the host-nodes and policy parameters to QEMU command
> -     * line which means we will use system default memory policy. */
> -    if (nodemask && mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
> -        if (virJSONValueObjectAdd(&props,
> -                                  "m:host-nodes", nodemask,
> -                                  "S:policy", qemuNumaPolicyTypeToString(mode),
> -                                  NULL) < 0)
> +    if (nodemask) {
> +        /* Make sure the requested nodeset is sensible */
> +        if (!virNumaNodesetIsAvailable(nodemask))
>              return -1;
> +
> +        /* If mode is "restrictive", we should only use cgroups setting allowed memory
> +         * nodes, and skip passing the host-nodes and policy parameters to QEMU command
> +         * line which means we will use system default memory policy. */
> +        if (mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
> +            if (virJSONValueObjectAdd(&props,
> +                                      "m:host-nodes", nodemask,
> +                                      "S:policy", qemuNumaPolicyTypeToString(mode),
> +                                      NULL) < 0)
> +                return -1;
> +
> +            if (nodemaskRet)
> +                *nodemaskRet = nodemask;
> +        }
>      }

I don't mind the additional nesting, but adding it at the same time
as you're making functional changes makes it harder to focus on the
latter.

Please separate the functional and non-functional parts of this
change into two commits.

-- 
Andrea Bolognani / Red Hat / Virtualization




[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