From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 334B61FF0AA for ; Tue, 03 Nov 2026 15:32:40 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 31AF221891; Tue, 03 Nov 2026 15:32:37 +0100 (CET) Message-ID: Date: Tue, 3 Nov 2026 15:32:30 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH qemu-server-rs v3 1/3] cpu: add pve-qemu-server-cpu crate for vCPU binding To: Elias Huhsovitz , pve-devel@lists.proxmox.com References: <20261022092227.58871-1-e.huhsovitz@proxmox.com> <20261022092227.58871-2-e.huhsovitz@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793716351060 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.327 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: HXGYJXES36F7GEVG4PVHCBP6CEJNZRTH X-Message-ID-Hash: HXGYJXES36F7GEVG4PVHCBP6CEJNZRTH X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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. >