From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 42A711FF13F for ; Thu, 26 Feb 2026 15:00:56 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2850A31D1D; Thu, 26 Feb 2026 15:01:53 +0100 (CET) Message-ID: Date: Thu, 26 Feb 2026 15:01:47 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server v3 1/2] cpu config: Add 'arch' property to cpu_fmt To: Arthur Bied-Charreton , pve-devel@lists.proxmox.com References: <20260217141036.338747-1-a.bied-charreton@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260217141036.338747-1-a.bied-charreton@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1772114489456 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.074 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.618 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.734 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.78 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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: RN4XGTLFFXZA3UDXLHQR5FKXGSD3GLLC X-Message-ID-Hash: RN4XGTLFFXZA3UDXLHQR5FKXGSD3GLLC X-MailFrom: f.ebner@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: Please always send a cover-letter if there is more than one patch, see [0]. This helps with tooling like b4 (e.g. a R-b for the cover-letter will be picked up for the whole series). Am 17.02.26 um 3:11 PM schrieb Arthur Bied-Charreton: > Preparatory step for adding support for configuring custom CPU types in > the PVE UI. > > Add optional property 'arch' (x86_64|aarch64) to cpu_fmt to allow custom > models to indicate which architecture they belong to, default to x86_64 > for backwards compatibility. > > Add checks to get_cpu_options and validate_cpu_conf to deny illegal > configs early. > > Signed-off-by: Arthur Bied-Charreton I still gave a review below, since the comments are worthwhile for the future too, but I'm really not sure anymore if we actually want this. Since this was my idea, sorry about that! We'd at least need to also check in validate_vm_cpu_conf(), since $cpu_fmt is used there too. But again, I don't think it's worthwhile: The only use case for an explicit arch for custom models is when the reported-model cannot uniquely be associated to an arch. In all other cases, the option is just additional friction. Right now, the only affected models are 'host' and 'max' models. Supporting mixed arch clusters is not planned right now AFAIK, so actually just 'max'. And in practice, we can just associate the model to the arch of the VM its assigned to. If really needed, users can just define two custom models, one with flags for x86_64 and one with flags for aarch64. Even if at some point there is a model name conflict between archs for other models, it's gonna be very rare that users need both and if they do, they can just define two custom models. What do you think? If we really do see the need to add it (in the future), I think the default should rather be "arch of the reported-model and if that is not unique, arch of the VM it's assigned to". > --- > Changes since v1: > * add checks to validate_cpu_conf + get_cpu_options to fail early on > misconfigurations > * reuse the pve-qm-cpu-arch standard option for cpu_fmt's arch property > * do not use single quotes for hash keys if not needed > * do not use parentheses for post-ifs > > Changes since v2: > * use get_standard_option for getting the pve-qm-cpu-arch standard > option instead of accessing the arch_desc directly > > src/PVE/QemuServer/CPUConfig.pm | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm > index 32ec4954..a8e146d7 100644 > --- a/src/PVE/QemuServer/CPUConfig.pm > +++ b/src/PVE/QemuServer/CPUConfig.pm > @@ -374,6 +374,12 @@ my $cpu_fmt = { > . " note that doing so will break live migration to CPUs with other values.", > optional => 1, > }, > + arch => { > + %{ PVE::JSONSchema::get_standard_option('pve-qm-cpu-arch') }, > + default => 'x86_64', > + description => 'The architecture the CPU model belongs to.', > + optional => 1, > + }, Please note that get_standard_option() has an explicit way to specify a base via its second argument, which is a bit more explicit. > }; > > my $sev_fmt = { > @@ -475,6 +481,13 @@ sub validate_cpu_conf { > my ($cpu) = @_; > # required, but can't be forced in schema since it's encoded in section header for custom models > die "CPU is missing cputype\n" if !$cpu->{cputype}; > + > + if (my $reported_model = $cpu->{'reported-model'}) { > + my $arch = $cpu->{arch} // $cpu_fmt->{arch}->{default}; We use this logic three times and the $cpu_fmt name is a bit generic, which both suggest that having a helper function get_custom_cpu_arch() would be nice. > + die "reported model '$reported_model' is not valid for architecture '$arch'\n" > + if !$cpu_models_by_arch->{$arch}->{$reported_model}; > + } > + > return $cpu; > } > PVE::JSONSchema::register_format('pve-vm-cpu-conf', $cpu_fmt, \&validate_vm_cpu_conf); > @@ -612,6 +625,10 @@ sub get_cpu_models { > > my $conf = load_custom_model_conf(); > for my $custom_model (keys %{ $conf->{ids} }) { > + my $custom_model_arch = $conf->{ids}->{$custom_model}->{arch}; > + $custom_model_arch //= $cpu_fmt->{arch}->{default}; > + next if $custom_model_arch ne $arch; > + > my $reported_model = $conf->{ids}->{$custom_model}->{'reported-model'}; > $reported_model //= $cpu_fmt->{'reported-model'}->{default}; > my $vendor = $all_cpu_models->{$reported_model}; > @@ -854,6 +871,9 @@ sub get_cpu_options { > $builtin_cpu->{flags} = $model->{'flags'}; > } elsif (is_custom_model($cputype)) { > $custom_cpu = get_custom_model($cputype); > + my $custom_cpu_arch = $custom_cpu->{arch} // $cpu_fmt->{arch}->{default}; > + die "custom CPU model has architecture '$custom_cpu_arch', but VM has '$arch'\n" > + if $custom_cpu_arch ne $arch; > > $cputype = $custom_cpu->{'reported-model'} // $cpu_fmt->{'reported-model'}->{default}; > $kvm_off = $custom_cpu->{hidden} if defined($custom_cpu->{hidden}); [0]: https://lore.proxmox.com/all/176174696524.1776006.10760334798456181474.b4-ty@proxmox.com/