From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 7F3201FF138 for ; Tue, 21 Jul 2026 13:59:08 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 75EB821506; Tue, 21 Jul 2026 13:59:03 +0200 (CEST) From: Erik Fastermann To: pve-devel@lists.proxmox.com Subject: [RFC qemu-server 2/4] remote migrate: collect preconditions as structured findings Date: Tue, 21 Jul 2026 13:58:25 +0200 Message-ID: <20260721115827.163442-3-e.fastermann@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260721115827.163442-1-e.fastermann@proxmox.com> References: <20260721115827.163442-1-e.fastermann@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 2 AWL -0.057 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: K2LZLTGGUFP5CMKS3E57KFH5DWQ5QHQF X-Message-ID-Hash: K2LZLTGGUFP5CMKS3E57KFH5DWQ5QHQF 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, 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 --- 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