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 367EF1FF0E6 for ; Fri, 07 Aug 2026 11:13:08 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 447D7215CA; Fri, 07 Aug 2026 11:12:44 +0200 (CEST) From: Erik Fastermann To: pve-devel@lists.proxmox.com Subject: [PATCH qemu-server v2 2/5] remote migration: collect preconditions as structured findings Date: Fri, 7 Aug 2026 11:12:24 +0200 Message-ID: <20260807091227.73614-3-e.fastermann@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260807091227.73614-1-e.fastermann@proxmox.com> References: <20260807091227.73614-1-e.fastermann@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -0.525 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) KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: LSXPENAPZZ6YGU7J6L7M6KYYL3DFXRB3 X-Message-ID-Hash: LSXPENAPZZ6YGU7J6L7M6KYYL3DFXRB3 X-MailFrom: efastermann@ruth.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: Erik Fastermann X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Extract the remote-migration precondition checks into a validate_remote_migrate_preconditions helper that records each problem as a {severity, type, message} finding and returns the derived migration parameters. This prepares for a precondition endpoint that returns the full findings list instead of dying on errors. User-visible effect is minimal and successful migrations are unchanged. Some precondition error messages are reworded and the "Establishing API connection" info line is no longer printed, as this would be noise in the precondition endpoint. Precondition warnings now use log_warn and the collected precondition errors are all included in the die message. The HA check no longer uses raise_param_exc, which unifies the interface. Signed-off-by: Erik Fastermann --- src/PVE/API2/Qemu.pm | 280 +++++++++++++++++++++++++++++-------------- 1 file changed, 189 insertions(+), 91 deletions(-) diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm index 56c5f08c..b16a1365 100644 --- a/src/PVE/API2/Qemu.pm +++ b/src/PVE/API2/Qemu.pm @@ -1063,6 +1063,157 @@ sub assert_scsi_feature_compatibility { } } +my sub validate_remote_migrate_preconditions { + my ($param, $findings) = @_; + + my $add_finding = sub { + my ($severity, $type, $message, %extra_info) = @_; + chomp($message); + + push @$findings, + { + severity => $severity, + type => $type, + message => $message, + %extra_info, + }; + }; + + my $add_error = sub { $add_finding->('error', @_); }; + my $add_warning = sub { $add_finding->('warning', @_); }; + + 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 (my $err = $@) { + $add_error->('load-vm-config', $err); + 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 (my $err = $@) { + $add_error->('remote-query', $err); + return; + } + + 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 option", + ) if !$param->{online}; + } else { + $add_warning->( + 'online-migration-vm-not-running', + "VM isn't running, migrating offline 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 (my $err = $@) { + $add_error->( + 'custom-cpu-validation', + "could not validate custom CPU model compatibility: $err", + ); + return; + } + + my $cpu_schema = { + type => 'object', + properties => PVE::QemuServer::CPUConfig->options(), + }; + eval { PVE::JSONSchema::validate($remote_custom_cpu, $cpu_schema); }; + + if (my $err = $@) { + $add_error->( + 'custom-cpu-validation', + "could not validate custom CPU model compatibility: $err", + ); + 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 => '', @@ -5725,106 +5876,36 @@ __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); + my $findings = []; + my $migration_info = validate_remote_migrate_preconditions($param, $findings); - 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}, - }; + my $errors = ''; + my $error_count = 0; - if ($remote->{fingerprint}) { - $conn_args->{cached_fingerprints} = { uc($remote->{fingerprint}) => 1 }; + for my $finding (@$findings) { + next if $finding->{severity} ne 'error'; + $errors .= "- $finding->{message}\n"; + $error_count++; } - 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; - - 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; - } - - 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, - ); - } + if ($errors) { + die "detected $error_count error(s) preventing remote migration:\n" . $errors; } - 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} = $migration_info->{storagemap}; + $param->{bridgemap} = $migration_info->{bridgemap}; $param->{remote} = { - conn => $conn_args, # re-use fingerprint for tunnel - client => $api_client, - vmid => $target_vmid, + conn => $migration_info->{conn_args}, # re-use fingerprint for tunnel + client => $migration_info->{api_client}, + vmid => $migration_info->{target_vmid}, }; $param->{migration_type} = 'websocket'; $param->{'with-local-disks'} = 1; - $param->{delete} = $delete if $delete; + $param->{delete} = $migration_info->{delete} if $migration_info->{delete}; - my $cluster_status = $api_client->get("/cluster/status"); + my $cluster_status = $migration_info->{api_client}->get("/cluster/status"); my $target_node; - foreach my $entry (@$cluster_status) { + for my $entry (@$cluster_status) { next if $entry->{type} ne 'node'; if ($entry->{local}) { $target_node = $entry->{name}; @@ -5836,14 +5917,31 @@ __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, + $migration_info->{conn_args}->{host}, + $migration_info->{source_vmid}, + $param, + ); }; my $worker = sub { - return PVE::GuestHelpers::guest_migration_lock($source_vmid, 10, $realcmd); + # warn only in the worker, so the messages reach the task log and + # the task's warning count matches what is logged + for my $finding (@$findings) { + log_warn($finding->{message}) if $finding->{severity} eq 'warning'; + } + + return PVE::GuestHelpers::guest_migration_lock( + $migration_info->{source_vmid}, + 10, + $realcmd, + ); }; - return $rpcenv->fork_worker('qmigrate', $source_vmid, $authuser, $worker); + return $rpcenv->fork_worker( + 'qmigrate', $migration_info->{source_vmid}, $authuser, $worker, + ); }, }); -- 2.47.3