Re: [PATCH pve-manager v4 10/17] api: add CRUD handlers for custom CPU models

Fiona Ebner
2026-05-07T14:02:57Z

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:
>