[RFC qemu-server 2/4] remote migrate: collect preconditions as structured findings

[Erik Fastermann <e.fastermann@proxmox.com>]

Extract the remote-migration precondition checks into a
validate_remote_migrate_preconditions helper that records each problem
as a {severity, code, message} finding and returns the derived
migration parameters. The endpoint fails on the first error finding,
preserving the previous abort behavior.

It also pulls the checks that previously lived only in the qm CLI into
the endpoint. Direct API callers such as the web UI now run them too,
so fewer migrations start only to fail partway. This prepares for a
precondition endpoint that returns the full findings list instead of
dying on the first error.

User-visible effect is minimal: successful migrations are unchanged.
Some precondition error messages are reworded and the "Establishing
API connection" info line is no longer printed.

Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
 src/PVE/API2/Qemu.pm | 304 ++++++++++++++++++++++++++++++-------------
 1 file changed, 211 insertions(+), 93 deletions(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 68f1800c..9b487457 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -1042,6 +1042,195 @@ sub assert_scsi_feature_compatibility {
     }
 }
 
+my sub validate_remote_migrate_preconditions {
+    my ($param, $findings) = @_;
+
+    my $add_error = sub {
+        push @$findings,
+            {
+                severity => 'error',
+                code => $_[0],
+                message => ("$_[1]" =~ s/\s+$//r),
+                @_[2 .. $#_],
+            };
+    };
+
+    my $add_warning = sub {
+        push @$findings,
+            {
+                severity => 'warning',
+                code => $_[0],
+                message => ("$_[1]" =~ s/\s+$//r),
+                @_[2 .. $#_],
+            };
+    };
+
+    my $source_vmid = extract_param($param, 'vmid');
+    my $target_endpoint = extract_param($param, 'target-endpoint');
+    my $remote = PVE::JSONSchema::parse_property_string('proxmox-remote', $target_endpoint);
+    my $target_vmid = extract_param($param, 'target-vmid') // $source_vmid;
+
+    my $target_storage = extract_param($param, 'target-storage');
+    my $storagemap = eval { PVE::JSONSchema::parse_idmap($target_storage, 'pve-storage-id') };
+    raise_param_exc({ 'target-storage' => "failed to parse storage map: $@" })
+        if $@;
+
+    my $target_bridge = extract_param($param, 'target-bridge');
+    my $bridgemap = eval { PVE::JSONSchema::parse_idmap($target_bridge, 'pve-bridge-id') };
+    raise_param_exc({ 'target-bridge' => "failed to parse bridge map: $@" })
+        if $@;
+
+    my $delete = extract_param($param, 'delete') // 0;
+
+    PVE::Cluster::check_cfs_quorum();
+
+    # test if VM exists
+    my $conf = eval { PVE::QemuConfig->load_config($source_vmid) };
+    if ($@) {
+        $add_error->('load-vm-config', $@);
+        return;
+    }
+
+    if (PVE::QemuConfig->has_lock($conf)) {
+        $add_error->('vm-locked', "VM is locked ($conf->{lock})");
+    }
+
+    if (PVE::HA::Config::service_is_configured("vm:$source_vmid")) {
+        $add_error->('vm-ha-configured', 'cannot remote-migrate VM that is configured for HA');
+    }
+
+    my $conn_args = {
+        protocol => 'https',
+        host => $remote->{host},
+        port => $remote->{port} // 8006,
+        apitoken => $remote->{apitoken},
+    };
+
+    if ($remote->{fingerprint}) {
+        $conn_args->{cached_fingerprints} = { uc($remote->{fingerprint}) => 1 };
+    }
+
+    my $api_client = PVE::APIClient::LWP->new(%$conn_args);
+
+    # check connection once at the start
+    eval { $api_client->get("/version") };
+    if ($@) {
+        $add_error->('remote-conn', $@);
+        return;
+    }
+
+    my $resources = $api_client->get("/cluster/resources", { type => 'vm' });
+    if (grep { defined($_->{vmid}) && $_->{vmid} eq $target_vmid } @$resources) {
+        $add_error->('target-vmid-exists', "remote: guest with ID '$target_vmid' already exists");
+    }
+
+    my $storages = $api_client->get("/nodes/localhost/storage", { enabled => 1 });
+
+    my $check_remote_storage = sub {
+        my ($storage) = @_;
+        my $found = [grep { $_->{storage} eq $storage } @$storages];
+
+        if (!@$found) {
+            $add_error->(
+                'storage-missing',
+                "remote: storage '$storage' does not exist (or missing permission)",
+                storage => $storage,
+            );
+            return;
+        }
+
+        $found = $found->[0];
+        my $content_types = [PVE::Tools::split_list($found->{content})];
+        $add_error->(
+            'storage-no-images',
+            "remote: storage '$storage' cannot store images",
+            storage => $storage,
+        ) if !grep { $_ eq 'images' } @$content_types;
+    };
+
+    foreach my $target_sid (values %{ $storagemap->{entries} }) {
+        $check_remote_storage->($target_sid);
+    }
+
+    $check_remote_storage->($storagemap->{default})
+        if $storagemap->{default};
+
+    my $repl_conf = PVE::ReplicationConfig->new();
+    my $is_replicated = $repl_conf->check_for_existing_jobs($source_vmid, 1);
+    $add_error->('vm-replicated', "cannot remote-migrate replicated VM")
+        if $is_replicated;
+
+    if (PVE::QemuServer::check_running($source_vmid)) {
+        $add_error->('offline-migration-vm-running', "cannot migrate running VM without --online")
+            if !$param->{online};
+    } else {
+        $add_warning->(
+            'online-migration-vm-not-running',
+            "VM isn't running, doing offline migration instead",
+        ) if $param->{online};
+        $param->{online} = 0;
+    }
+
+    my $check_custom_cpu = sub {
+        return if !defined($conf->{cpu});
+
+        my $cpu = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $conf->{cpu});
+        my $cputype = $cpu->{cputype};
+        return 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 ($@) {
+            $add_error->(
+                'custom-cpu-validation',
+                "could not validate custom CPU model compatibility: $@",
+            );
+            return;
+        }
+
+        my $cpu_schema = {
+            type => 'object',
+            properties => PVE::QemuServer::CPUConfig->options(),
+        };
+        eval { PVE::JSONSchema::validate($remote_custom_cpu, $cpu_schema); };
+
+        if ($@) {
+            $add_error->(
+                'custom-cpu-validation',
+                "could not validate custom CPU model compatibility: $@",
+            );
+            return;
+        }
+
+        eval {
+            PVE::QemuServer::CPUConfig::assert_custom_model_compatibility(
+                $custom_cpu, $remote_custom_cpu,
+            );
+        };
+
+        $add_error->('custom-cpu-mismatch', $@) if $@;
+    };
+    $check_custom_cpu->();
+
+    $add_error->('storage-mapping', "remote migration requires explicit storage mapping")
+        if $storagemap->{identity};
+
+    return {
+        conn_args => $conn_args,
+        api_client => $api_client,
+        source_vmid => $source_vmid,
+        target_vmid => $target_vmid,
+        storagemap => $storagemap,
+        bridgemap => $bridgemap,
+        delete => $delete,
+    };
+}
+
 __PACKAGE__->register_method({
     name => 'vmlist',
     path => '',
@@ -5713,104 +5902,26 @@ __PACKAGE__->register_method({
         my $rpcenv = PVE::RPCEnvironment::get();
         my $authuser = $rpcenv->get_user();
 
-        my $source_vmid = extract_param($param, 'vmid');
-        my $target_endpoint = extract_param($param, 'target-endpoint');
-        my $target_vmid = extract_param($param, 'target-vmid') // $source_vmid;
-
-        my $delete = extract_param($param, 'delete') // 0;
-
-        PVE::Cluster::check_cfs_quorum();
-
-        # test if VM exists
-        my $conf = PVE::QemuConfig->load_config($source_vmid);
-
-        PVE::QemuConfig->check_lock($conf);
-
-        raise_param_exc({ vmid => "cannot remote-migrate VM that is configured for HA" })
-            if PVE::HA::Config::service_is_configured("vm:$source_vmid");
-
-        my $remote = PVE::JSONSchema::parse_property_string('proxmox-remote', $target_endpoint);
-
-        # TODO: move this as helper somewhere appropriate?
-        my $conn_args = {
-            protocol => 'https',
-            host => $remote->{host},
-            port => $remote->{port} // 8006,
-            apitoken => $remote->{apitoken},
-        };
-
-        if ($remote->{fingerprint}) {
-            $conn_args->{cached_fingerprints} = { uc($remote->{fingerprint}) => 1 };
-        }
-
-        print "Establishing API connection with remote at '$remote->{host}'\n";
-
-        my $api_client = PVE::APIClient::LWP->new(%$conn_args);
-
-        my $repl_conf = PVE::ReplicationConfig->new();
-        my $is_replicated = $repl_conf->check_for_existing_jobs($source_vmid, 1);
-        die "cannot remote-migrate replicated VM\n" if $is_replicated;
+        my $findings = [];
+        my $plan = validate_remote_migrate_preconditions($param, $findings);
 
-        if (PVE::QemuServer::check_running($source_vmid)) {
-            die "can't migrate running VM without --online\n" if !$param->{online};
-
-        } else {
-            warn "VM isn't running. Doing offline migration instead.\n" if $param->{online};
-            $param->{online} = 0;
+        foreach my $finding (@$findings) {
+            warn "$finding->{message}\n" if $finding->{severity} eq 'warning';
+            die "$finding->{message}\n" if $finding->{severity} eq 'error';
         }
 
-        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 =
-            eval { PVE::JSONSchema::parse_idmap($target_storage, 'pve-storage-id') };
-        raise_param_exc({ 'target-storage' => "failed to parse storage map: $@" })
-            if $@;
-
-        my $target_bridge = extract_param($param, 'target-bridge');
-        my $bridgemap = eval { PVE::JSONSchema::parse_idmap($target_bridge, 'pve-bridge-id') };
-        raise_param_exc({ 'target-bridge' => "failed to parse bridge map: $@" })
-            if $@;
-
-        die "remote migration requires explicit storage mapping!\n"
-            if $storagemap->{identity};
-
-        $param->{storagemap} = $storagemap;
-        $param->{bridgemap} = $bridgemap;
+        $param->{storagemap} = $plan->{storagemap};
+        $param->{bridgemap} = $plan->{bridgemap};
         $param->{remote} = {
-            conn => $conn_args, # re-use fingerprint for tunnel
-            client => $api_client,
-            vmid => $target_vmid,
+            conn => $plan->{conn_args}, # re-use fingerprint for tunnel
+            client => $plan->{api_client},
+            vmid => $plan->{target_vmid},
         };
         $param->{migration_type} = 'websocket';
         $param->{'with-local-disks'} = 1;
-        $param->{delete} = $delete if $delete;
+        $param->{delete} = $plan->{delete} if $plan->{delete};
 
-        my $cluster_status = $api_client->get("/cluster/status");
+        my $cluster_status = $plan->{api_client}->get("/cluster/status");
         my $target_node;
         foreach my $entry (@$cluster_status) {
             next if $entry->{type} ne 'node';
@@ -5824,14 +5935,21 @@ __PACKAGE__->register_method({
             if !defined($target_node);
 
         my $realcmd = sub {
-            PVE::QemuMigrate->migrate($target_node, $remote->{host}, $source_vmid, $param);
+            PVE::QemuMigrate->migrate(
+                $target_node,
+                $plan->{conn_args}->{host},
+                $plan->{source_vmid},
+                $param,
+            );
         };
 
         my $worker = sub {
-            return PVE::GuestHelpers::guest_migration_lock($source_vmid, 10, $realcmd);
+            return PVE::GuestHelpers::guest_migration_lock($plan->{source_vmid}, 10, $realcmd);
         };
 
-        return $rpcenv->fork_worker('qmigrate', $source_vmid, $authuser, $worker);
+        return $rpcenv->fork_worker(
+            'qmigrate', $plan->{source_vmid}, $authuser, $worker,
+        );
     },
 });
 
-- 
2.47.3