Re: [PATCH qemu-server 2/2] memory: add default numa allocation policy

[Dominik Csapak <d.csapak@proxmox.com>]

On 3/12/26 3:39 PM, Maximiliano Sandoval wrote:
> Dominik Csapak <d.csapak@proxmox.com> writes:
> 
>> just for clarity: was this requested or did you just notice that it was missing?
> 
> I noticed a while ago it was missing and never went around to fix it.
> Clarification was also requested a couple of times in enterprise
> support.

ok

> 
>> On 3/12/26 11:50 AM, Maximiliano Sandoval wrote:
>>> This follows the host numa policy.
>>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>>> ---
>>>    src/PVE/QemuServer/Memory.pm | 7 ++++---
>>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>> diff --git a/src/PVE/QemuServer/Memory.pm b/src/PVE/QemuServer/Memory.pm
>>> index bcf6f9c5..9e65b2b0 100644
>>> --- a/src/PVE/QemuServer/Memory.pm
>>> +++ b/src/PVE/QemuServer/Memory.pm
>>> @@ -40,11 +40,12 @@ my $numa_fmt = {
>>>        },
>>>        policy => {
>>>            type => 'string',
>>> -        enum => [qw(preferred bind interleave)],
>>> +        enum => [qw(default preferred bind interleave)],
>>>            description => "NUMA allocation policy.",
>>>            verbose_description => <<EODESC,
>>>    NUMA allocation policy. Possible values are:
>>>    + - default: default host policy
>>>     - preferred: prefer the given host node list for allocation
>>>     - bind: restrict memory allocation to the given host node list
>>>     - interleave: interleave memory allocations across the given host node list
>>> @@ -53,6 +54,7 @@ The models are explained in more details at the kernel's documentation
>>>    https://docs.kernel.org/admin-guide/mm/numa_memory_policy.html#components-of-memory-policies.
>>>    EODESC
>>>            optional => 1,
>>> +        default => 'default',
>>>        },
>>>    };
>>>    PVE::JSONSchema::register_format('pve-qm-numanode', $numa_fmt);
>>> @@ -450,8 +452,7 @@ sub config {
>>>                    my $hostnodes = print_numa_hostnodes($hostnodelists);
>>>                      # policy
>>> -                my $policy = $numa->{policy};
>>> -                die "you need to define a policy for hostnode $hostnodes\n" if !$policy;
>>> +                my $policy = $numa->{policy} // 'default';
>>
>> while the property itself was already optional (since it was only required when
>> the hostnodes were given) when we add this
>> we should mark 'default' as the default in the schema.
> 
> This was set as a default a couple of lines above, is this what you
> meant?

ahh oops, i did not see that, then disregard these comments here ;)

> 
>>
>> otherwise we could keep this check here and not set a default.
>>
> 
> I will send a v2 where instead of `// 'default'` one checks for the
> default value in the schema.

no IMO it's fine as it is, just wanted to give an alternative to setting
the default, but since you did that already that's not necessary.

> 
>>>                    $mem_object .= ",host-nodes=$hostnodes,policy=$policy";
>>>                } else {
>>>                    die "numa hostnodes need to be defined to use hugepages" if $conf->{hugepages};
>