Re: [PATCH qemu-server-rs v3 1/3] cpu: add pve-qemu-server-cpu crate for vCPU binding

["Elias Huhsovitz" <e.huhsovitz@proxmox.com>]

On Wed Nov 4, 2026 at 11:25 AM CET, Elias Huhsovitz wrote:
> On Tue Nov 3, 2026 at 3:32 PM CET, Dominik Csapak wrote:
>>
>>
>> On 11/3/26 3:00 PM, Elias Huhsovitz wrote:
>>> On Fri Oct 30, 2026 at 2:46 PM CET, Dominik Csapak wrote:
>> [snip]
>>>>
>>>> couldn't this be also inferred from the config instead from the qmp
>>>> output? I guess it does not make much difference, but I'd like
>>>> to use the config as 'authority' since the actual vm should (must?)
>>>> follow the config anyway, otherwise we'd have a bug somewhere....
>>>>
>>> 
>>> I guess it can. But i think using the qmp output is more consistent. I
>>> had the following problem in mind:
>>> 
>>> If the user configures something like:
>>> numa0 => 'cpus=0-1,hostnodes=0,memory=2048,policy=bind',
>>> numa1 => 'cpus=2-3,hostnodes=1,memory=2048,policy=bind',
>>> 
>>> We can easily associate the vCPUs with the vNUMA nodes.
>>> 
>>> But i was under operating under the assumption that the following is
>>> also a valid config (omitting the cpus option):
>>> numa0 => 'hostnodes=0,memory=2048,policy=bind',
>>> numa1 => 'hostnodes=1,memory=2048,policy=bind',
>>> 
>>> Since, in theory we have all of the information we need to bind numa0:
>>> hostnode0, numa1:hostnode1 from the qmp "query-cpus-fast" command.
>>> 
>>> Otherwise the user must set the cpus here, which basically brings us
>>> back to implementing a "manual" mode.
>>
>> but i don't think thats true?
>>
>> in our configs currently, the only numa nodes generated are per socket,
>> so if we have a config with
>>
>> sockets: x
>> cores: y
>>
>> we know that
>>
>> 0..(y-1) => vnuma 0
>> y..(2*y-1) => vnuma 1
>>
>> and so on
>>
>> having the vcpus listed in the numaX config is just a bit more control
>> that's probably not necessary most of the time
>>
>> or do I miss something here in how qemu assigns the vnuma nodes?
>>
>>
>
> TL;DR: Yes, this is true. Thanks!
>
> I have just realized that omitting the `cpus` parameter is not a valid
> config in pve. e.g. this would be invalid:
>
> numa0: hostnodes=0,memory=4096,policy=bind
> numa1: hostnodes=0,memory=4096,policy=bind
>
> It prints this message:
> vm 901 - unable to parse value of 'numa0' - format error
> cpus: property is missing and it is not optional
> (The task status is still OK, which made me miss this)
>
> When omitting the numaX config entirely and just setting
> numa: 1
>
> We actually do generate numa nodes per socket. 
>
> In my testing I was passing the -numa values to qemu directly, which
> resulted in a non-sequential (perhaps deterministic?) config.
>
> I just missed the code in Memory.pm:462.
>
> Then the optimal approach is to:
> 1. use the cpus= parameter defined in the numaX config
> 2. Fallback to the socket based generation.
>

I fixed this in v4:
https://lore.proxmox.com/pve-devel/20261111152027.260117-1-e.huhsovitz@proxmox.com/

The cpu binding mappings are now purely based on the existing config
(numa, affinity, cpu-binding-mode) In the same fashion that the
original RFC intended:
https://lore.proxmox.com/pve-devel/20260217114813.2063770-1-d.csapak@proxmox.com/

>>> 
>>>> [snip]
>>>>
>>>>> diff --git a/pve-qemu-server-cpu/src/error.rs b/pve-qemu-server-cpu/src/error.rs
>>>>> new file mode 100644
>>>>> index 0000000..73a569e
>>>>> --- /dev/null
>>>>> +++ b/pve-qemu-server-cpu/src/error.rs
>>>>> @@ -0,0 +1,38 @@
>>>>> +use thiserror::Error;
>>>>> +
>>>>> +#[derive(Debug, Error)]
>>>>> +pub enum BindError {
>>>>> +    // Parse errors
>>>>> +    #[error("map entry '{0}' is missing a ':' separator")]
>>>>> +    MissingColon(String),
>>>>> +
>>>>> +    #[error("invalid syntax in binding map: '{0}'")]
>>>>> +    BadSyntax(String),
>>>>> +
>>>>> +    // Validation errors (pre-start)
>>>>> +    #[error("host CPU {0} does not exist in this topology")]
>>>>> +    UnknownHostCpu(u32),
>>>>> +
>>>>> +    #[error("vCPU {0} appears in more than one map entry")]
>>>>> +    VcpuReused(u32),
>>>>> +
>>>>> +    #[error("vCPU {vcpu} is out of range (VM has vCPUs 0..{max})")]
>>>>> +    VcpuOutOfRange { vcpu: u32, max: u32 },
>>>>> +
>>>>> +    // Runtime errors (post-start)
>>>>> +    #[error("no running thread found for vCPU {0}")]
>>>>> +    ThreadNotFound(u32),
>>>>> +
>>>>> +    #[error("invalid kernel TID {tid} for vCPU {vcpu}")]
>>>>> +    BadThreadId { vcpu: u32, tid: u32 },
>>>>> +
>>>>> +    #[error("failed to init host topology: '{0}'")]
>>>>> +    TopologyInit(String),
>>>>> +
>>>>> +    #[error("binding vCPU {vcpu} failed: {message}")]
>>>>> +    Bind {
>>>>> +        vcpu: u32,
>>>>> +        /// Human-readable error from libhwloc (errno-based).
>>>>> +        message: String,
>>>>> +    },
>>>>> +}
>>>>
>>>> i we'd reuse the numaX binding modes, most of these would not be
>>>> necessary (as well as the parsing etc.)
>>>>
>>> 
>>> I will change it to utilize the existing numa[n] settings.
>>> 
>>>> in general if we'd restrict ourselves (for the moment) on the basic
>>>> modes + using affinity to limit the set further.
>>>>
>>>> the amount of information we'd need from sysfs would
>>>> be a numa node -> vcpu list sysfs read
>>>>
>>>> everything else (no smt sibling, only 'big' cores, etc.) we could
>>>> do via the affinity and the vcpu -> hostnuma node manual pinning
>>>> we could do via numaX
>>> 
>>> 
>>> I will focus on the basic pinning for now, but i will try to make
>>> the code easily extensible if we should need stuff like manual-pinning,
>>> SMT sibling filtering, etc in the future.
>>> 
>>>>
>>>> the manual mode is not really necessary since a user can simply do
>>>> it themselves if they want to.
>>>