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

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

[Erik Fastermann]

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>
Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---

 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 $@;
+
+                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} // '');
+
+    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;
+    }
+}
+
 # Print a QEMU device node for a given VM configuration for hotplugging CPUs
 sub print_cpu_device($conf, $arch, $id) {
 
-- 
2.47.3

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

[Dominik Csapak]

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 <w.hoos@proxmox.com>
> Suggested-by: Fiona Ebner <f.ebner@proxmox.com>
> Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
> ---
> 
>   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) {
>