From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 9D4761FF153 for ; Mon, 22 Jun 2026 17:00:50 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C779EF8C9; Mon, 22 Jun 2026 17:00:48 +0200 (CEST) Date: Mon, 22 Jun 2026 17:00:42 +0200 From: Arthur Bied-Charreton To: Erik Fastermann Subject: Re: [PATCH qemu-server] remote migration: validate custom CPU configs Message-ID: <4uclywtw5prvek5qpfprrdeg2rjihffzh4wvj37nefitvm2ca7@2wxnwecha6ab> References: <20260618080811.20382-1-e.fastermann@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260618080811.20382-1-e.fastermann@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1782140433889 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.755 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 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: 4TCJC6GC3NATTCDVFQ7O7SA5PYZXB6SE X-Message-ID-Hash: 4TCJC6GC3NATTCDVFQ7O7SA5PYZXB6SE X-MailFrom: a.bied-charreton@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 CC: pve-devel@lists.proxmox.com 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 Thu, Jun 18, 2026 at 10:08:11AM +0200, Erik Fastermann wrote: Thanks for working on this, this kind of stuff improves the UX a lot imo. I applied the patch on a node in a cluster A managed by a PDM instance I had running. I then created a custom CPU model with some flags on A, assigned it to a VM and tried remote-migrating the VM to another cluster B. This failed right away due to the CPU model missing from B. I then created a custom CPU model on B with the same name but different flags and retried the remote migration, which also failed due to a config mismatch in the flags. I tested this with mismatches in each cpu model config field, always worked as advertised. The error messages show the values of the mismatching fields on both sides, which makes finding & fixing mismatches straightforward. Consider this: Tested-by: Arthur Bied-Charreton Left some comments inline. > Previously, a remote migration would fail with a cryptic error message > 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 > Signed-off-by: Erik Fastermann > --- > src/PVE/API2/Qemu.pm | 48 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm > index 54883f1e..3f1362c6 100644 > --- a/src/PVE/API2/Qemu.pm > +++ b/src/PVE/API2/Qemu.pm > @@ -5745,6 +5745,54 @@ __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)); > + }; > + if (my $err = $@) { nit: we usually use post-ifs for this kind of error handling (`die "... $@\n" if $@`) > + die "cpu $cputype config mismatch: $err\n"; if the API call fails due to a connectivity issue with the remote for example, the error message will still lead with "cpu ... config mismatch": ``` cpu custom-cpu config mismatch: 500 Can't connect to 10.50.0.50:8006 (No route to host) ``` maybe something more general like 'could not validate custom CPU model compatibility' would be better here? if the CPU model is not found the error returned by the API will say so anyway. > + } > + > + my $cpu_fmt = PVE::QemuServer::CPUConfig->options(); > + eval { PVE::JSONSchema::validate($remote_custom_cpu, $cpu_fmt); }; > + if (my $err = $@) { > + die "cpu $cputype config mismatch: $err\n"; not sure how often this would fail, but here too the error message might be a little misleading. > + } > + could make sense to move the deep comparison logic (i.e. the rest of the block starting from here) over to a helper in CPUConfig to make sure we have one source of truth for this. > + my @custom_cpu_flags = sort split /;/, ($custom_cpu->{flags} // ''); > + my @remote_custom_cpu_flags = sort split /;/, > + ($remote_custom_cpu->{flags} // ''); maybe the sorted flags could be joined back to a string instead of being kept as a list, then the following checks could just be one `if $a ne $b`. this would also avoid unconditionally constructing the $cpu_flags_mismatch_error string. > + > + my $cpu_flags_mismatch_error = > + "cpu $cputype config mismatch for flags: local=" > + . $custom_cpu->{flags} > + . ",remote=" > + . $remote_custom_cpu->{flags} . "\n"; > + > + die $cpu_flags_mismatch_error > + if @custom_cpu_flags != @remote_custom_cpu_flags; > + > + for my $i (0 .. $#custom_cpu_flags) { > + die $cpu_flags_mismatch_error > + if $custom_cpu_flags[$i] ne $remote_custom_cpu_flags[$i]; > + } > + > + for my $key (sort keys %$cpu_fmt) { > + next if $key eq 'flags'; > + my $v1 = $custom_cpu->{$key} // ''; > + my $v2 = $remote_custom_cpu->{$key} // ''; > + die "cpu $cputype config mismatch for $key: local=$v1,remote=$v2\n" > + if $v1 ne $v2; > + } > + } > + } > + > my $storecfg = PVE::Storage::config(); > my $target_storage = extract_param($param, 'target-storage'); > my $storagemap = > -- > 2.47.3 > > > >