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 3AA8C1FF0BC for ; Thu, 29 Oct 2026 10:03:40 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 3EBD521244; Thu, 29 Oct 2026 10:03:36 +0100 (CET) Message-ID: <6bddcacb-5f5a-40b6-99cf-433c29b30d03@proxmox.com> Date: Thu, 29 Oct 2026 10:03:23 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH ha-manager v5 14/28] crm command: support JSON-style migrate command From: Fiona Ebner To: pve-devel@lists.proxmox.com References: <20261019144629.215541-1-f.ebner@proxmox.com> <20261019144629.215541-15-f.ebner@proxmox.com> Content-Language: en-US In-Reply-To: <20261019144629.215541-15-f.ebner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793264611259 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.332 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: EJORYKO2W67DWX66Y2YQEE5VEB57WTV5 X-Message-ID-Hash: EJORYKO2W67DWX66Y2YQEE5VEB57WTV5 X-MailFrom: f.ebner@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: Am 19.10.26 um 4:48 PM schrieb Fiona Ebner: > 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 > --- > > Changes in v5: > * Drop unused variables in handle_crm_motion_command(). > * Add handle_crm_motion_command() helper. > * Use elsif instead of if+next. > * Rename 'kind' to 'task' for consistency. > > src/PVE/HA/Manager.pm | 66 ++++++++++++++++++++++++++++++++++++------- > 1 file changed, 56 insertions(+), 10 deletions(-) > > diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm > index b8d0e3e..8d14ca7 100644 > --- a/src/PVE/HA/Manager.pm > +++ b/src/PVE/HA/Manager.pm > @@ -712,6 +712,56 @@ my sub check_resource_motion_valid { > return 1; > } > > +my sub handle_crm_motion_command { > + my ($self, $task, $sid, $node, $options) = @_; > + > + my $cmd = "$task $sid $node"; > + > + if (defined($options) && scalar(keys($options->%*))) { > + $cmd .= ' ' . JSON::to_json($options, { canonical => 1 }); > + } > + > + my ($haenv, $ss) = $self->@{qw(haenv ss)}; > + > + if (my $sd = $ss->{$sid}) { > + if (check_resource_motion_valid($self, $node, $sd, "ignore crm command '$cmd'")) { > + $self->queue_resource_motion($cmd, $task, $sid, $node, $options); > + } > + } else { > + $haenv->log('err', "crm command error - no such service: $cmd"); > + } > + > + return; > +} > + > +my sub handle_crm_json_command { > + my ($self, $cmd) = @_; > + > + my $haenv = $self->{haenv}; > + > + my $command_info = eval { JSON::decode_json($cmd) }; > + if (my $err = $@) { > + $haenv->log('err', "unable to decode JSON command: '$cmd' - $err"); > + next; As found by the automated review, this should be 'return' > + } > + > + my $task = $command_info->{task}; > + if (!defined($task)) { > + $haenv->log('err', "unable to handle JSON command without 'task': '$cmd'"); > + } elsif ($task eq 'migrate') { > + 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; Same here. I'll wait for other review before sending a v6. > + } > + handle_crm_motion_command($self, $task, $sid, $node, $options); > + } else { > + $haenv->log('err', "unable to handle unknown JSON command: '$cmd'"); > + } > + > + return; > +} > + > # read new crm commands and save them into crm master status > sub update_crm_commands { > my ($self) = @_; > @@ -723,16 +773,12 @@ 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 (check_resource_motion_valid($self, $node, $sd, "ignore crm command '$cmd'")) { > - $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. > + handle_crm_json_command($self, $cmd); > + } elsif ($cmd =~ m/^(migrate|relocate)\s+(\S+)\s+(\S+)$/) { > + handle_crm_motion_command($self, $1, $2, $3, {}); > } elsif ($cmd =~ m/^stop\s+(\S+)\s+(\S+)$/) { > my ($sid, $timeout) = ($1, $2); > if (my $sd = $ss->{$sid}) {