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 BBB781FF13F for ; Thu, 07 May 2026 16:02:57 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 66E421DF83; Thu, 7 May 2026 16:02:57 +0200 (CEST) Message-ID: <8aab9839-faed-4e9c-86dd-a10d8b314357@proxmox.com> Date: Thu, 7 May 2026 16:02:17 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-manager v4 10/17] api: add CRUD handlers for custom CPU models To: Arthur Bied-Charreton , pve-devel@lists.proxmox.com References: <20260430160109.565536-1-a.bied-charreton@proxmox.com> <20260430160109.565536-11-a.bied-charreton@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260430160109.565536-11-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: 1778162429965 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.041 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 PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far 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: SF4DHEFUTD7A7AUCPCNUU75VXY2RPRP6 X-Message-ID-Hash: SF4DHEFUTD7A7AUCPCNUU75VXY2RPRP6 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: Am 30.04.26 um 6:02 PM schrieb Arthur Bied-Charreton: > diff --git a/PVE/API2/Cluster/Qemu/CustomCPUModels.pm b/PVE/API2/Cluster/Qemu/CustomCPUModels.pm > new file mode 100644 > index 00000000..8c2bd724 > --- /dev/null > +++ b/PVE/API2/Cluster/Qemu/CustomCPUModels.pm > @@ -0,0 +1,211 @@ > +package PVE::API2::Cluster::Qemu::CustomCPUModels; > + > +use v5.36; > + > +use PVE::JSONSchema qw(get_standard_option); > +use PVE::RPCEnvironment; > +use PVE::RESTHandler; > +use PVE::Tools qw(extract_param); > + > +use PVE::QemuServer::CPUConfig; > + > +use base qw(PVE::RESTHandler); > + > +__PACKAGE__->register_method({ > + name => 'config', The endpoints for mappings have a child link for 'id'. We could add one for 'cputype'. > + path => '', > + method => 'GET', > + description => 'Read all custom CPU models definitions', > + permissions => { > + check => ['perm', '/nodes', ['Sys.Audit']], > + }, > + parameters => { > + additionalProperties => 0, > + }, > + returns => { > + type => 'array', > + items => { > + type => 'object', > + properties => get_standard_option('pve-qemu-cpu'), > + }, > + }, > + code => sub { > + my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf(); > + delete $conf->{order}; > + my $ids = []; Call it $res? It's not (just) the IDs. > + foreach my $id (keys %{ $conf->{ids} }) { Style nits: prefer 'for' and '->%*' > + delete $conf->{ids}->{$id}->{type}; > + push @$ids, $conf->{ids}->{$id}; > + } > + return $ids; > + }, > +}); > + > +__PACKAGE__->register_method({ > + name => 'create', > + path => '', > + method => 'POST', > + protected => 1, > + description => 'Add a custom CPU model definition', > + permissions => { > + check => ['perm', '/nodes', ['Sys.Console']], Should be Sys.Modify I guess? > + }, > + parameters => { > + additionalProperties => 0, > + properties => PVE::QemuServer::CPUConfig::add_cpu_json_properties({ > + digest => get_standard_option('pve-config-digest'), > + }), > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + my $digest = extract_param($param, 'digest'); > + > + (my $name = $param->{cputype}) =~ s/^custom-//; > + $param->{cputype} = "custom-$name"; > + As mentioned in the qemu-server patch already, there could/should be a check_config() call here. > + PVE::QemuServer::CPUConfig::lock_cpu_config(sub { > + my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf(); > + PVE::SectionConfig::assert_if_modified($conf, $digest); Do we care? If the ID is still available, we can still proceed even if there was another modification. We hold the lock. > + > + die "custom CPU model '$name' already exists\n" > + if defined($conf->{ids}->{$name}); > + $conf->{ids}->{$name} = $param; > + > + PVE::QemuServer::CPUConfig::write_custom_model_conf($conf); > + }); > + }, > +}); > + > +__PACKAGE__->register_method({ > + name => 'delete', > + path => '{cputype}', > + method => 'DELETE', > + protected => 1, > + description => 'Delete a custom CPU model definition', > + permissions => { > + check => ['perm', '/nodes', ['Sys.Console']], Sys.Modify? > + }, > + parameters => { > + additionalProperties => 0, > + properties => { > + digest => get_standard_option('pve-config-digest'), > + cputype => { > + type => 'string', > + description => "The custom model to delete.", > + }, > + }, > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + my $digest = extract_param($param, 'digest'); > + > + (my $name = $param->{cputype}) =~ s/^custom-//; > + > + PVE::QemuServer::CPUConfig::lock_cpu_config(sub { > + my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf(); > + PVE::SectionConfig::assert_if_modified($conf, $digest); Similar here. If the ID still exists, we can delete it without worrying about somebody having modified the config. I mean, you might want to check if the ID does not exist to give a better error (since there might have been a concurrent delete for the same ID). > + > + die "custom CPU model '$name' does not exist\n" > + if !defined($conf->{ids}->{$name}); > + delete $conf->{ids}->{$name}; > + > + PVE::QemuServer::CPUConfig::write_custom_model_conf($conf); > + }); > + }, > +}); > + > +__PACKAGE__->register_method({ > + name => 'update', > + path => '{cputype}', > + method => 'PUT', > + protected => 1, > + description => "Update a custom CPU model definition.", > + permissions => { > + check => ['perm', '/nodes', ['Sys.Console']], Sys.Modify? > + }, > + parameters => { > + additionalProperties => 0, > + properties => PVE::QemuServer::CPUConfig::add_cpu_json_properties({ > + digest => get_standard_option('pve-config-digest'), > + delete => { > + type => 'string', > + format => 'pve-configid-list', > + description => "A list of properties to delete.", > + optional => 1, > + }, > + }), > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + my $digest = extract_param($param, 'digest'); > + my $delete = extract_param($param, 'delete') // ''; > + my %delete_hash = map { $_ => 1 } PVE::Tools::split_list($delete); > + > + (my $name = $param->{cputype}) =~ s/^custom-//; > + $param->{cputype} = "custom-$name"; > + > + PVE::QemuServer::CPUConfig::lock_cpu_config(sub { > + my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf(); > + > + PVE::SectionConfig::assert_if_modified($conf, $digest); Of course, here it is very useful to abort :) > + > + my $model = $conf->{ids}->{$name}; > + die "custom CPU model '$name' does not exist\n" if !defined($model); > + As mentioned in the qemu-server patch already, there could/should be a check_config() call here. > + my $props = PVE::QemuServer::CPUConfig::add_cpu_json_properties({}); > + for my $p (keys %$props) { > + if ($delete_hash{$p}) { > + die "cannot delete 'cputype' property\n" if $p eq 'cputype'; > + die "cannot set and delete property '$p' at once\n" if $param->{$p}; > + delete $model->{$p}; Please use the PVE::SectionConfig::delete_from_config() function. > + } elsif (defined($param->{$p})) { > + $model->{$p} = $param->{$p}; > + } > + } > + > + PVE::QemuServer::CPUConfig::write_custom_model_conf($conf); > + }); > + }, > +}); > + > +__PACKAGE__->register_method({ > + name => 'info', > + path => '{cputype}', > + method => 'GET', > + description => 'Retrieve details about a specific custom CPU model', > + permissions => { > + check => ['perm', '/nodes', ['Sys.Audit']], > + }, > + parameters => { > + additionalProperties => 0, > + properties => { > + cputype => { > + type => 'string', > + description => "Name of the CPU model to query.", > + }, > + }, > + }, > + returns => { > + type => 'object', > + properties => PVE::QemuServer::CPUConfig::add_cpu_json_properties({ > + digest => get_standard_option('pve-config-digest'), > + }), > + }, > + code => sub { > + my ($param) = @_; > + (my $name = $param->{cputype}) =~ s/^custom-//; > + my $conf = PVE::QemuServer::CPUConfig::load_custom_model_conf(); > + my $digest = $conf->{digest}; Style nit: no need for an extra variable for the digest. > + my $retval = PVE::QemuServer::CPUConfig::get_custom_model($name); > + $retval->{digest} = $digest; > + return $retval; > + }, > +}); > + > +1; > diff --git a/PVE/API2/Cluster/Qemu/Makefile b/PVE/API2/Cluster/Qemu/Makefile > index 2ab3e0b1..76b661db 100644 > --- a/PVE/API2/Cluster/Qemu/Makefile > +++ b/PVE/API2/Cluster/Qemu/Makefile > @@ -2,8 +2,9 @@ include ../../../../defines.mk > > # for node independent, cluster-wide applicable, API endpoints > # ensure we do not conflict with files shipped by pve-cluster!! > -PERLSOURCE= \ > - CPUFlags.pm > +PERLSOURCE= \ > + CPUFlags.pm \ > + CustomCPUModels.pm > > all: >