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 04D7E1FF0B7 for ; Fri, 16 Oct 2026 11:55:02 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id EB782214C9; Fri, 16 Oct 2026 11:54:57 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 16 Oct 2026 11:54:52 +0200 Message-Id: Subject: Re: [PATCH ha-manager v4 14/27] api: resources: migration: support additional migration options From: "Daniel Kral" To: "Fiona Ebner" , X-Mailer: aerc 0.22.0-10-g6373ac9d2179-dirty References: <20261015125112.174954-1-f.ebner@proxmox.com> <20261015125112.174954-15-f.ebner@proxmox.com> In-Reply-To: <20261015125112.174954-15-f.ebner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792144492205 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.768 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) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: U4HU3X2D5PDD6LMUEBI325OEWDXNX6QB X-Message-ID-Hash: U4HU3X2D5PDD6LMUEBI325OEWDXNX6QB X-MailFrom: d.kral@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 X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu Oct 15, 2026 at 2:50 PM CEST, Fiona Ebner wrote: > Additional migration options can now be passed from the guest API > endpoints to the HA stack. From the HA resource API endpoint, the > options are passed all the way to the resource plugins. > > Check that all nodes are recent enough to support the new-style JSON > CRM command, otherwise produce a warning and fall-back to the previous > behavior ignoring the additional migration options. > > The migration API endpoints for containers and VMs use different > parameters. For example, the endpoint for containers does not have > the 'with-conntrack-state' option. To keep the schema for the more > abstract endpoint for HA resources clean, separate by type using a > oneOf schema and let the resource plugins declare additional > parameters via a migrate_json_properties() method. > > Initially, this will be used to fix #7053 and pass along the > 'with-conntrack-state' migration option. > > Signed-off-by: Fiona Ebner > --- > > Dependency bump for pve-cluster needed! > > Changes in v4: > * Add cfs_update() call. > * Import raise_param_exc(). > * Ensure error messages are also visible on the CLI. > * Only use JSON-style command if required to avoid fallout in > scenarios with old manager that fails to report its version properly > * Document why continuing when the version-info for a node cannot be > parsed is better than aborting. > > src/PVE/API2/HA/Resources.pm | 110 ++++++++++++++++++++++++++++++----- > src/PVE/HA/Resources.pm | 1 + > 2 files changed, 97 insertions(+), 14 deletions(-) > > diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm > index b96572b..6a97b2f 100644 > --- a/src/PVE/API2/HA/Resources.pm > +++ b/src/PVE/API2/HA/Resources.pm > @@ -3,15 +3,20 @@ package PVE::API2::HA::Resources; > use strict; > use warnings; > =20 > +use JSON qw(); > + > +use PVE::Exception qw(raise_param_exc); > use PVE::SafeSyslog; > use PVE::Tools qw(extract_param); > use PVE::Cluster; > use PVE::HA::Config; > +use PVE::HA::Env::PVE2; Why depend on PVE2 here? > use PVE::HA::Resources; > use HTTP::Status qw(:constants); > use Storable qw(dclone); > use PVE::JSONSchema qw(get_standard_option); > use PVE::RPCEnvironment; > +use PVE::RESTEnvironment qw(log_warn); > =20 > use PVE::RESTHandler; > =20 > @@ -34,6 +39,49 @@ my $api_copy_config =3D sub { > return $scfg; > }; > =20 > +my sub migrate_json_properties { > + my ($type) =3D @_; > + my $plugin =3D PVE::HA::Resources->lookup($type); > + return $plugin->migrate_json_properties(); > +} > + > +my sub json_crm_command_is_supported { nit: should probably include _motion to specify that it only checks whether the migrate JSON command is supported > + PVE::Cluster::cfs_update(); > + my $version_info =3D PVE::Cluster::get_node_kv('version-info'); > + for my $node (sort keys $version_info->%*) { > + my $node_info =3D eval { JSON::decode_json($version_info->{$node= }); }; > + if (my $err =3D $@) { > + # An updated cluster is the usual situation. An error here s= hould not affect the usual > + # situation. The unusual situation, where the master is too = old and unable to report its > + # version properly is rare. Submitting a JSON CRM to such a = master will lead to warnings > + # and the command being ignored. Note that migration without= extra parameters also still > + # works. > + log_warn("failed check for migration options support - canno= t parse version info for" > + . " node '$node' - $err"); > + } elsif (!PVE::Cluster::pvecfg_min_version($node_info->{version}= , 9, 2, 22)) { > + return 0; > + } > + } > + > + return 1; > +} > + > +my sub json_crm_command_is_applicable { nit: should probably include _motion to specify that it only checks whether the migrate JSON command is applicable > + my ($sid, $req_node, $param) =3D @_; > + > + # FIXME: MAJOR VERSION: use JSON variant even if there are no extra = params. > + if (scalar(keys $param->%*) > 0) { > + if (json_crm_command_is_supported()) { > + return 1; > + } else { > + log_warn("$sid migration to $req_node: ignoring additional m= igration options - not" > + . " supported by CRM on all nodes"); > + } > + } > + > + return 0; > +} > + > sub check_service_state { > my ($sid, $req_state) =3D @_; > =20 > @@ -336,20 +384,32 @@ __PACKAGE__->register_method({ > check =3D> ['perm', '/', ['Sys.Console']], > }, > parameters =3D> { > - additionalProperties =3D> 0, > - properties =3D> { > - sid =3D> get_standard_option( > - 'pve-ha-resource-or-vm-id', > - { completion =3D> \&PVE::HA::Tools::complete_sid }, > - ), > - node =3D> get_standard_option( > - 'pve-node', > - { > - completion =3D> \&PVE::Cluster::complete_migration_t= arget, > - description =3D> "Target node.", > - }, > - ), > + 'type-property' =3D> 'resource-type', > + 'type-property-schema' =3D> { > + type =3D> 'string', > + description =3D> 'The resource type. Automatically determine= d from the service ID.', > + enum =3D> $resource_type_enum, > }, > + oneOf =3D> [ > + { > + 'instance-type' =3D> 'ct', > + additionalProperties =3D> 0, > + properties =3D> migrate_json_properties('ct'), > + }, > + { > + 'instance-type' =3D> 'vm', > + additionalProperties =3D> 0, > + properties =3D> migrate_json_properties('vm'), > + }, > + ], > + }, > + resolve_type =3D> sub { > + my ($param) =3D @_; > + my $type =3D (PVE::HA::Config::parse_sid($param->{sid}))[1]; > + if (!($type eq 'ct' || $type eq 'vm')) { > + die "Unknown service type '$type' for 'migrate' endpoint sch= ema.\n"; > + } > + return $type; > }, > returns =3D> { > type =3D> 'object', > @@ -403,12 +463,34 @@ __PACKAGE__->register_method({ > =20 > my ($sid, $type, $name) =3D PVE::HA::Config::parse_sid(extract_p= aram($param, 'sid')); > my $req_node =3D extract_param($param, 'node'); > + my $resource_type =3D extract_param($param, 'resource-type'); > + if ($resource_type ne $type) { > + raise_param_exc({ > + 'resource-type' =3D> "value '$resource_type' does not ma= tch type of '$sid'" }); > + } > + > + # The rest of the parameters in $param are migration options pas= sed along to the plugin. > =20 > PVE::HA::Config::service_is_ha_managed($sid); > =20 > check_service_state($sid); > =20 > - PVE::HA::Config::queue_crm_commands("migrate $sid $req_node"); > + my $crm_command; > + if (json_crm_command_is_applicable($sid, $req_node, $param)) { > + $crm_command =3D JSON::to_json( > + { > + kind =3D> 'migrate', > + sid =3D> $sid, > + node =3D> $req_node, > + options =3D> $param, > + }, > + { canonical =3D> 1 }, > + ); > + } else { > + $crm_command =3D "migrate $sid $req_node"; > + } > + > + PVE::HA::Config::queue_crm_commands($crm_command); > $result->{sid} =3D $sid; > $result->{'requested-node'} =3D $req_node; > =20 > diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm > index 57fde00..ad603dc 100644 > --- a/src/PVE/HA/Resources.pm > +++ b/src/PVE/HA/Resources.pm > @@ -3,6 +3,7 @@ package PVE::HA::Resources; > use strict; > use warnings; > =20 > +use PVE::Cluster; Oh here it is :-) > use PVE::JSONSchema qw(get_standard_option); > use PVE::SectionConfig; > use PVE::HA::Tools;