all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Daniel Kral" <d.kral@proxmox.com>
To: "Fiona Ebner" <f.ebner@proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH ha-manager v3 10/21] api: resources: migration: support additional migration options
Date: Mon, 12 Oct 2026 11:59:56 +0200	[thread overview]
Message-ID: <DM2RSZ53O1UN.2E9QBNRWZ47FP@proxmox.com> (raw)
In-Reply-To: <20260918160841.128088-11-f.ebner@proxmox.com>

On Fri Sep 18, 2026 at 6:08 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() function.
>
> Initially, this will be used to fix #7053 and pass along the
> 'with-conntrack-state' migration option.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> The node version might need to be adapted when applying!
>
> New in v3.
>
>  src/PVE/API2/HA/Resources.pm  | 99 ++++++++++++++++++++++++++++++-----
>  src/PVE/HA/Resources/PVECT.pm | 10 ++++
>  src/PVE/HA/Resources/PVEVM.pm | 10 ++++
>  3 files changed, 105 insertions(+), 14 deletions(-)
>
> diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
> index b96572b..6d808ca 100644
> --- a/src/PVE/API2/HA/Resources.pm
> +++ b/src/PVE/API2/HA/Resources.pm
> @@ -3,6 +3,8 @@ package PVE::API2::HA::Resources;
>  use strict;
>  use warnings;
>  
> +use JSON qw();
> +
>  use PVE::SafeSyslog;
>  use PVE::Tools qw(extract_param);
>  use PVE::Cluster;
> @@ -12,6 +14,7 @@ use HTTP::Status qw(:constants);
>  use Storable qw(dclone);
>  use PVE::JSONSchema qw(get_standard_option);
>  use PVE::RPCEnvironment;
> +use PVE::SafeSyslog;
>  
>  use PVE::RESTHandler;
>  
> @@ -34,6 +37,34 @@ my $api_copy_config = sub {
>      return $scfg;
>  };
>  
> +my $common_migrate_json_properties = {
> +    sid => get_standard_option(
> +        'pve-ha-resource-or-vm-id',
> +        { completion => \&PVE::HA::Tools::complete_sid },
> +    ),
> +    node => get_standard_option(
> +        'pve-node',
> +        {
> +            completion => \&PVE::Cluster::complete_migration_target,
> +            description => "Target node.",
> +        },
> +    ),
> +};
> +
> +my sub json_crm_command_is_supported {

nit: This assumes that cfs_update() is called before, which is called in
     PVE::RPCEnvironment::init_request() for API requests and
     PVE::RESTEnvironment::setup_default_cli_env() for CLI requests,
     that is used in PVE::CLI::ha_manager::setup_environment().

     Maybe we should add a comment about that, because otherwise the
     'version-info' KV will only contain the current node's JSON string,
     therefore checking only locally? Could be pedantic though.

> +    my $version_info = PVE::Cluster::get_node_kv('version-info');
> +    for my $node (sort keys $version_info->%*) {
> +        my $node_info = eval { JSON::decode_json($version_info->{$node}); };
> +        if (my $err = $@) { # warn, but continue
> +            syslog('warn', "cannot parse version info for $node as JSON - $err");

Shouldn't we fail if it's not possible to determine a node's version?

Any way, the syslog should probably have some context why parsing the
version info is needed here and should also be shown to stdout/stderr
for callers of ha-manager.

> +        } elsif (!PVE::Cluster::pvecfg_min_version($node_info->{version}, 9, 2, 21)) {
> +            return 0;
> +        }
> +    }
> +
> +    return 1;
> +}

IMO a generic version of this helper would be nice to have in
PVE::Cluster as well, as there already are users that need that
functionality (David's vNet pool series [0] and Lukas' notification
matcher series [1]), but no blocker for this.

[0] https://lore.proxmox.com/pve-devel/20260724142528.109453-4-d.riley@proxmox.com/
[1] https://lore.proxmox.com/pve-devel/20261009125646.284673-39-l.wagner@proxmox.com/

> +
>  sub check_service_state {
>      my ($sid, $req_state) = @_;
>  
> @@ -336,20 +367,34 @@ __PACKAGE__->register_method({
>          check => ['perm', '/', ['Sys.Console']],
>      },
>      parameters => {
> -        additionalProperties => 0,
> -        properties => {
> -            sid => get_standard_option(
> -                'pve-ha-resource-or-vm-id',
> -                { completion => \&PVE::HA::Tools::complete_sid },
> -            ),
> -            node => get_standard_option(
> -                'pve-node',
> -                {
> -                    completion => \&PVE::Cluster::complete_migration_target,
> -                    description => "Target node.",
> -                },
> -            ),
> +        'type-property' => 'resource-type',
> +        'type-property-schema' => {
> +            type => 'string',
> +            description => 'The resource type. Automatically determined from the service ID.',
> +            enum => ['ct', 'vm'],
>          },
> +        oneOf => [
> +            {
> +                'instance-type' => 'ct',
> +                additionalProperties => 0,
> +                properties => PVE::HA::Resources::PVECT::migrate_json_properties(
> +                    $common_migrate_json_properties),
> +            },
> +            {
> +                'instance-type' => 'vm',
> +                additionalProperties => 0,
> +                properties => PVE::HA::Resources::PVEVM::migrate_json_properties(
> +                    $common_migrate_json_properties),
> +            },
> +        ],
> +    },
> +    resolve_type => sub {
> +        my ($param) = @_;
> +        my $type = (PVE::HA::Config::parse_sid($param->{sid}))[1];
> +        if (!($type eq 'ct' || $type eq 'vm')) {
> +            die "Unknown service type '$type' for 'migrate' endpoint schema.\n";
> +        }
> +        return $type;
>      },
>      returns => {
>          type => 'object',
> @@ -403,12 +448,38 @@ __PACKAGE__->register_method({
>  
>          my ($sid, $type, $name) = PVE::HA::Config::parse_sid(extract_param($param, 'sid'));
>          my $req_node = extract_param($param, 'node');
> +        my $resource_type = extract_param($param, 'resource-type');
> +        if ($resource_type ne $type) {
> +            raise_param_exc({
> +                'resource-type' => "$resource_type does not match type of $sid" });
> +        }
> +
> +        # The rest of the parameters in $param are migration options passed along to the plugin.
>  
>          PVE::HA::Config::service_is_ha_managed($sid);
>  
>          check_service_state($sid);
>  
> -        PVE::HA::Config::queue_crm_commands("migrate $sid $req_node");
> +        my $crm_command;
> +        if (json_crm_command_is_supported()) {
> +            $crm_command = JSON::encode_json({
> +                kind => 'migrate',
> +                sid => $sid,
> +                node => $req_node,
> +                options => $param,
> +            });
> +        } else {
> +            if (scalar(keys $param->%*) > 0) {
> +                syslog(
> +                    'warn',
> +                    "$sid migration to $req_node: ignoring additional migration options"
> +                        . " - not supported by CRM on all nodes",
> +                );

nit: This should also be shown in stdout/stderr to direct callers of
     ha-manager.

> +            }
> +            $crm_command = "migrate $sid $req_node";
> +        }
> +
> +        PVE::HA::Config::queue_crm_commands($crm_command);

Nice!

>          $result->{sid} = $sid;
>          $result->{'requested-node'} = $req_node;
>  
> diff --git a/src/PVE/HA/Resources/PVECT.pm b/src/PVE/HA/Resources/PVECT.pm
> index 3d8a5b9..17dd697 100644
> --- a/src/PVE/HA/Resources/PVECT.pm
> +++ b/src/PVE/HA/Resources/PVECT.pm
> @@ -174,4 +174,14 @@ sub get_static_stats_from_config {
>      };
>  }
>  
> +# FIXME: the extra properties are missing from the docs, since the PVE::API::LXC module can't be
> +# included in that environment, because of cyclic dependencies..
> +sub migrate_json_properties {
> +    my ($props) = @_;
> +    my $extra_migrate_props = eval { PVE::API2::LXC::ha_migrate_json_properties() } // {};
> +    die $@ if $@ && !$ENV{PVE_GENERATING_DOCS};
> +    $props->{$_} = $extra_migrate_props->{$_} for keys $extra_migrate_props->%*;

This will write the specific migrate props in the external $props, i.e.,
$common_migrate_json_properties. For some reason, the one for VMs is run
first for me, which results in both VMs and CTs having
--with-conntrack-state in the end:


# ha-manager help migrate
USAGE: ha-manager crm-command migrate <sid> <node> --resource-type <string> [OPTIONS]

  Request resource migration (online) to another node.

  <sid>      <type>:<name>
             HA resource ID. This consists of a resource type followed by a
             resource specific name, separated with colon (example: vm:100
             / ct:100). For virtual machines and containers, you can simply
             use the VM or CT id as a shortcut (example: 100).

  <node>     <string>
             Target node.

  --resource-type <ct | vm>
             The resource type. Automatically determined from the service
             ID.

 Conditional options:

 [resource-type=ct]

  --skip-config-check <boolean>   (default=0)
             For intra-cluster migration. Skip checking if the
             configuration can be parsed successfully by the target. Using
             this flag can lead to dropped configuration options if the
             target is too old to parse them.

  --with-conntrack-state <boolean>   (default=0)
             Whether to migrate conntrack entries for running VMs.

 [resource-type=vm]

  --skip-config-check <boolean>   (default=0)
             For intra-cluster migration. Skip checking if the
             configuration can be parsed successfully by the target. Using
             this flag can lead to dropped configuration options if the
             target is too old to parse them.

  --with-conntrack-state <boolean>   (default=0)
             Whether to migrate conntrack entries for running VMs.


Therefore, this should only write in the new copy made by
ha_migrate_json_properties().

> +    return $props;
> +}
> +
>  1;
> diff --git a/src/PVE/HA/Resources/PVEVM.pm b/src/PVE/HA/Resources/PVEVM.pm
> index 964f374..52a48e4 100644
> --- a/src/PVE/HA/Resources/PVEVM.pm
> +++ b/src/PVE/HA/Resources/PVEVM.pm
> @@ -194,4 +194,14 @@ sub get_static_stats_from_config {
>      };
>  }
>  
> +# FIXME: the extra properties are missing from the docs, since the PVE::API::QEMU module can't be
> +# included in that environment, because of cyclic dependencies..
> +sub migrate_json_properties {
> +    my ($props) = @_;
> +    my $extra_migrate_props = eval { PVE::API2::Qemu::ha_migrate_json_properties() } // {};
> +    die $@ if $@ && !$ENV{PVE_GENERATING_DOCS};
> +    $props->{$_} = $extra_migrate_props->{$_} for keys $extra_migrate_props->%*;
> +    return $props;
> +}

Same here

> +
>  1;






  parent reply	other threads:[~2026-10-12 10:00 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 16:08 [PATCH-SERIES common/cluster/ha-manager/qemu-server/container v3 00/21] migration: strict config check for intra-cluster migration Fiona Ebner
2026-09-18 16:08 ` [PATCH common v3 01/21] tools: move version_cmp() helper from qemu-server Fiona Ebner
2026-09-18 16:08 ` [PATCH common v3 02/21] rest handler: handle: respect schema's 'type-property' when resolving type Fiona Ebner
2026-09-18 16:08 ` [PATCH cluster v3 03/21] cluster: move pvecfg node version helpers from qemu-server Fiona Ebner
2026-09-18 16:08 ` [PATCH ha-manager v3 04/21] next state {stopped,started}: factor out helper to handle motion command Fiona Ebner
2026-10-09 13:32   ` Daniel Kral
2026-09-18 16:08 ` [PATCH ha-manager v3 05/21] lrm: resource migration: support extra migration options Fiona Ebner
2026-10-09 13:34   ` Daniel Kral
2026-09-18 16:08 ` [PATCH ha-manager v3 06/21] change service state: support hash as a parameter value Fiona Ebner
2026-09-18 16:08 ` [PATCH ha-manager v3 07/21] next state: handle motion command: support migration options Fiona Ebner
2026-10-09 13:35   ` Daniel Kral
2026-09-18 16:08 ` [PATCH ha-manager v3 08/21] queue resource motion: " Fiona Ebner
2026-09-18 16:08 ` [PATCH ha-manager v3 09/21] crm command: support JSON-style migrate command Fiona Ebner
2026-10-09 13:38   ` Daniel Kral
2026-09-18 16:08 ` [PATCH ha-manager v3 10/21] api: resources: migration: support additional migration options Fiona Ebner
2026-09-18 16:21   ` Fiona Ebner
2026-10-12  9:59   ` Daniel Kral [this message]
2026-09-18 16:08 ` [PATCH qemu-server v3 11/21] helpers: move version_cmp() helper to pve-common Fiona Ebner
2026-10-08 14:46   ` Michael Köppl
2026-10-08 14:49     ` Fiona Ebner
2026-09-18 16:08 ` [PATCH qemu-server v3 12/21] helpers: move pvecfg node version helpers to pve-cluster Fiona Ebner
2026-09-18 16:08 ` [PATCH qemu-server v3 13/21] api: migrate: allow forwarding certain migration properties to HA Fiona Ebner
2026-09-18 16:08 ` [PATCH qemu-server v3 14/21] fix #7053: api: migrate: pass 'with-conntrack-state' flag to HA migration Fiona Ebner
2026-09-18 16:08 ` [PATCH qemu-server v3 15/21] qm: mtunnel: reply when a command is unknown Fiona Ebner
2026-09-18 16:08 ` [PATCH qemu-server v3 16/21] qm: mtunnel: add 'conf' command to do strict configuration parsing Fiona Ebner
2026-09-18 16:08 ` [PATCH qemu-server v3 17/21] migration: intra-cluster: check config can be parsed on target node Fiona Ebner
2026-09-18 16:08 ` [PATCH container v3 18/21] api: migrate: allow forwarding certain migration properties to HA Fiona Ebner
2026-09-18 16:08 ` [PATCH container v3 19/21] pct: introduce mtunnel command Fiona Ebner
2026-09-18 16:08 ` [PATCH container v3 20/21] d/control: bump versioned build dependency for libpve-common-perl to 9.0.12 Fiona Ebner
2026-09-18 16:08 ` [PATCH container v3 21/21] migration: intra-cluster: check config can be parsed on target node Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM2RSZ53O1UN.2E9QBNRWZ47FP@proxmox.com \
    --to=d.kral@proxmox.com \
    --cc=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal