Re: [PATCH ha-manager v3 09/21] crm command: support JSON-style migrate command

["Daniel Kral" <d.kral@proxmox.com>]

On Fri Sep 18, 2026 at 6:08 PM CEST, Fiona Ebner wrote:
> To pass along opaque migration options from the resource-specific
> migration API endpoint to the resource plugin implementation, the
> options will be encoded as JSON. Instead of just adding an extra
> positional argument with those, support having the full CRM command be
> recorded as JSON in the queued commands file.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> New in v3.
>
>  src/PVE/HA/Manager.pm | 73 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 56 insertions(+), 17 deletions(-)
>
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index 555ea0d..25026d6 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -4,6 +4,7 @@ use strict;
>  use warnings;
>  
>  use Digest::MD5 qw(md5_base64);
> +use JSON qw();
>  
>  use PVE::Tools;
>  
> @@ -670,6 +671,36 @@ sub any_resource_motion_queued_or_running {
>      return 0;
>  }
>  
> +my sub crm_cmd_migrate_relocate {

Hm, queue_resource_motion() 'queues' the resource motion command to
$sd->{cmd}, which is handled in next_state_{started,stopped}().

Maybe we should call this handle_crm_motion_command() or some
permutation of that? Or assert_and_queue_resource_motion()?

The term 'command' is already quite overloaded in the HA stack, but at
least in the Manager we can discern between crm_*_command (crm_commands
stack) and resource_*_command ($sd->{cmd}), what do you think?

> +    my ($self, $task, $sid, $node, $options) = @_;
> +
> +    my $cmd = "$task $sid $node";
> +
> +    if (defined($options) && scalar(keys($options->%*))) {
> +        $cmd .= ' ' . JSON::encode_json($options);
> +    }
> +
> +    my ($haenv, $ms, $ns, $sc, $ss) = $self->@{qw(haenv ms ns sc ss)};
> +
> +    if (my $sd = $ss->{$sid}) {
> +        if (!$ns->node_is_online($node)) {
> +            $haenv->log('err', "crm command error - node not online: $cmd");
> +        } else {
> +            if ($node eq $sd->{node}) {
> +                $haenv->log(
> +                    'info', "ignore crm command - service already on target node: $cmd",
> +                );
> +            } else {
> +                $self->queue_resource_motion($cmd, $task, $sid, $node, $options);
> +            }
> +        }
> +    } else {
> +        $haenv->log('err', "crm command error - no such service: $cmd");
> +    }

If we factor out the logic to a assert_valid_resource_motion() helper as
suggested in patch #4, then we could use it here, but not a blocker for
this at all.

> +
> +    return;
> +}
> +
>  # read new crm commands and save them into crm master status
>  sub update_crm_commands {
>      my ($self) = @_;
> @@ -681,25 +712,33 @@ sub update_crm_commands {
>      foreach my $cmd (split(/\n/, $cmdlist)) {
>          chomp $cmd;
>  
> -        if ($cmd =~ m/^(migrate|relocate)\s+(\S+)\s+(\S+)$/) {
> -            my ($task, $sid, $node) = ($1, $2, $3);
> -            if (my $sd = $ss->{$sid}) {
> -                if (!$ns->node_is_online($node)) {
> -                    $haenv->log('err', "crm command error - node not online: $cmd");
> -                } else {
> -                    if ($node eq $sd->{node}) {
> -                        $haenv->log(
> -                            'info',
> -                            "ignore crm command - service already on target node: $cmd",
> -                        );
> -                    } else {
> -                        $self->queue_resource_motion($cmd, $task, $sid, $node, {});
> -                    }
> -                }
> -            } else {
> -                $haenv->log('err', "crm command error - no such service: $cmd");
> +        if ($cmd =~ m/^{/) {
> +            # New-style JSON-encoded command. Currently only 'migrate' is supported, allowing for
> +            # additional options.
> +
> +            my $command_info = eval { JSON::decode_json($cmd) };
> +            if (my $err = $@) {
> +                $haenv->log('err', "unable to decode command as JSON: '$cmd' - $err");

nit: unable to decode JSON command: '$cmd' - $err

> +                next;
>              }
>  
> +            my $kind = $command_info->{kind};
> +            if (defined($kind) && $kind eq 'migrate') {

nit: maybe move the defined($kind) check above this already, because
otherwise any other branch that checks for $kind eq 'something' would
need it as well.

> +                my ($sid, $node, $options) = $command_info->@{qw(sid node options)};
> +                if (!defined($sid) || !defined($node)) {
> +                    $haenv->log('err', "'migrate' JSON command without sid or node");
> +                    next;
> +                }
> +                crm_cmd_migrate_relocate($self, $kind, $sid, $node, $options);
> +            } else {
> +                $haenv->log('err', "unable to handle unknown JSON command: '$cmd'");
> +            }
> +
> +            next;
> +        }
> +
> +        if ($cmd =~ m/^(migrate|relocate)\s+(\S+)\s+(\S+)$/) {
> +            crm_cmd_migrate_relocate($self, $1, $2, $3, {});

Nice!

>          } elsif ($cmd =~ m/^stop\s+(\S+)\s+(\S+)$/) {
>              my ($sid, $timeout) = ($1, $2);
>              if (my $sd = $ss->{$sid}) {