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 A3B391FF150 for ; Mon, 06 Jul 2026 16:02:25 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5B12C21457; Mon, 06 Jul 2026 16:02:23 +0200 (CEST) Message-ID: <5adf7aff-affc-4723-b0c4-1057aef267c2@proxmox.com> Date: Mon, 6 Jul 2026 16:01:47 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH qemu-server v2] remote migration: validate custom CPU configs To: Erik Fastermann , pve-devel@lists.proxmox.com References: <20260701135229.225599-1-e.fastermann@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20260701135229.225599-1-e.fastermann@proxmox.com> 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: 1783346501707 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.069 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) 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: 6WNVT5B3V2IZVSBRT4J4HIXMEX6XLNTQ X-Message-ID-Hash: 6WNVT5B3V2IZVSBRT4J4HIXMEX6XLNTQ 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: comments inline On 7/1/26 3:52 PM, Erik Fastermann wrote: > Previously, a remote migration would fail with a cryptic error message nit: an example of a such an error message would be much appreciated > if a custom CPU model was selected that did not exist on the target > server. Furthermore, no validation was performed to ensure that the > custom CPU definitions matched between the source and target. > > Fix this by comparing the CPU configurations before initiating the > migration and aborting early if they do not match. > > Reported-by: Walter Hoos > Suggested-by: Fiona Ebner > Tested-by: Arthur Bied-Charreton > Reviewed-by: Arthur Bied-Charreton > Signed-off-by: Erik Fastermann > --- > > changes since v1: > * fix noop JSONSchema::validate > * error handling with post-ifs > * more general error message for not found errors and similar > * move comparison to helper assert_custom_model_compatibility > * simplified sorted flags comparison > > src/PVE/API2/Qemu.pm | 25 +++++++++++++++++++++++++ > src/PVE/QemuServer/CPUConfig.pm | 21 +++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm > index 54883f1e..e2e87999 100644 > --- a/src/PVE/API2/Qemu.pm > +++ b/src/PVE/API2/Qemu.pm > @@ -5745,6 +5745,31 @@ __PACKAGE__->register_method({ > $param->{online} = 0; > } > > + if (defined($conf->{cpu})) { > + my $cpu = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $conf->{cpu}); > + my $cputype = $cpu->{cputype}; > + if (defined($cputype) && PVE::QemuServer::CPUConfig::is_custom_model($cputype)) { > + my $custom_cpu = PVE::QemuServer::CPUConfig::get_custom_model($cputype); > + > + my $remote_custom_cpu = eval { > + $api_client->get("/cluster/qemu/custom-cpu-models/" > + . URI::Escape::uri_escape_utf8($cputype)); > + }; > + die "could not validate custom CPU model compatibility: $@\n" if $@; > + > + my $cpu_schema = { > + type => 'object', > + properties => PVE::QemuServer::CPUConfig->options(), > + }; > + eval { PVE::JSONSchema::validate($remote_custom_cpu, $cpu_schema); }; > + die "could not validate custom CPU model compatibility: $@\n" if $@; wouldn't die'ing here mean that we cannot ever break this api when we want cross cluster migration with different major versions? wouldn't it be easier to send our (potentially old) version of the data to the remote server? then the newer version can decide what to do (and we could keep a compat version of the schema around for this?) though this has other implications (e.g. do we want to give the remote infos about our cpu models...) both directions can be argued ofc, but sending the old data to the newer instance can be detected in the code, the other way round sadly not... another 'simpler' way would be to document that the models have to be identical and leave it to the admin and simply error out when the model name does not exist on the target. (much less convenient though) > + > + PVE::QemuServer::CPUConfig::assert_custom_model_compatibility( > + $custom_cpu, $remote_custom_cpu, > + ); > + } > + } > + > my $storecfg = PVE::Storage::config(); > my $target_storage = extract_param($param, 'target-storage'); > my $storagemap = > diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm > index 9be145ef..df3e2b92 100644 > --- a/src/PVE/QemuServer/CPUConfig.pm > +++ b/src/PVE/QemuServer/CPUConfig.pm > @@ -647,6 +647,27 @@ sub get_custom_model($name, $noerr = undef, $conf = undef) { > return $model; > } > > +sub assert_custom_model_compatibility($local_cpu, $remote_cpu) { > + my $cputype = $local_cpu->{cputype}; > + > + my $local_cpu_flags = join ';', sort split /;/, ($local_cpu->{flags} // ''); > + my $remote_cpu_flags = join ';', sort split /;/, ($remote_cpu->{flags} // ''); personally i'd be for using parenthesis (and potentially splitting out the default assignment) here: my $local_flags = $local_cpu->{flags} // ''; join(';', sort(split(/;/, $local_flags))); makes it a bit more readable which part belongs to which part of the 3 calls in this line > + > + die "CPU $cputype config mismatch for flags: local=" > + . $local_cpu_flags > + . ",remote=" > + . $remote_cpu_flags . "\n" > + if $local_cpu_flags ne $remote_cpu_flags; > + > + for my $key (sort keys %$cpu_fmt) { > + next if $key eq 'flags'; > + my $v1 = $local_cpu->{$key} // ''; > + my $v2 = $remote_cpu->{$key} // ''; > + die "CPU $cputype config mismatch for $key: local=$v1,remote=$v2\n" > + if $v1 ne $v2; > + } more or less same as above. the code assumes the flags/formats will never change > +} > + > # Print a QEMU device node for a given VM configuration for hotplugging CPUs > sub print_cpu_device($conf, $arch, $id) { >