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

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

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?


> 
>> [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.
>