Re: [PATCH qemu-server] remote migration: validate custom CPU configs

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

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 <a.bied-charreton@proxmox.com>

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 <w.hoos@proxmox.com>
> Suggested-by: Fiona Ebner <f.ebner@proxmox.com>
> Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
> ---
>  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
> 
> 
> 
>