[pve-devel] [PATCH ha-manager 0/1] api: relocate/migrate resource: improve initialization of variables to avoid Perl warning

[pve-devel] [PATCH ha-manager 0/1] api: relocate/migrate resource: improve initialization of variables to avoid Perl warning

[Fiona Ebner]

As reported in the community forum [0], the resource relocation
endpoint would produce a warning:
> Can't use an undefined value as an ARRAY reference

In the get_resource_motion_info() function, properly initialize the
array references in the $blocking_resources_by_node hash to avoid
this. Note that the migration endpoint needs to be adapted to only
include 'blocking-resources' in the result if there is at least one
entry in the array to be compatible with this change. Align the
behavior of both migration and relocation endpoints, so that the
migration endpoint correctly handles the initialized, but empty array
too.

Alternatively, it could've been done the other way with the relocation
endpoint also checking for the array reference to be undefined if no
entries are to be added to the array, but since $comigrated_resources
is also initialized when empty, it seemed cleaner to go with the
approach here.

[0]: https://forum.proxmox.com/threads/173149/

pve-ha-manager:

Fiona Ebner (1):
  api: relocate/migrate resource: improve initialization of variables to
    avoid Perl warning

 src/PVE/API2/HA/Resources.pm | 2 +-
 src/PVE/HA/Config.pm         | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)


Summary over all repositories:
  2 files changed, 2 insertions(+), 1 deletions(-)

-- 
Generated by git-murpp 0.5.0


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH ha-manager 1/1] api: relocate/migrate resource: improve initialization of variables to avoid Perl warning

[Fiona Ebner]

As reported in the community forum [0], the resource relocation
endpoint would produce a warning:
> Can't use an undefined value as an ARRAY reference

In the get_resource_motion_info() function, properly initialize the
array references in the $blocking_resources_by_node hash to avoid
this. Note that the migration endpoint needs to be adapted to only
include 'blocking-resources' in the result if there is at least one
entry in the array to be compatible with this change. Align the
behavior of both migration and relocation endpoints, so that the
migration endpoint correctly handles the initialized, but empty array
too.

Alternatively, it could've been done the other way with the relocation
endpoint also checking for the array reference to be undefined if no
entries are to be added to the array, but since $comigrated_resources
is also initialized when empty, it seemed cleaner to go with the
approach here.

[0]: https://forum.proxmox.com/threads/173149/

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/HA/Resources.pm | 2 +-
 src/PVE/HA/Config.pm         | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
index 894fe90..aadddab 100644
--- a/src/PVE/API2/HA/Resources.pm
+++ b/src/PVE/API2/HA/Resources.pm
@@ -393,7 +393,7 @@ __PACKAGE__->register_method({
         my $blocking_resources = $blocking_resources_by_node->{$req_node};
 
         $result->{'comigrated-resources'} = $comigrated_resources if @$comigrated_resources;
-        $result->{'blocking-resources'} = $blocking_resources if $blocking_resources;
+        $result->{'blocking-resources'} = $blocking_resources if @$blocking_resources;
 
         return $result;
     },
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index 301c62f..b52465f 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -397,6 +397,7 @@ sub get_resource_motion_info {
         push @$dependent_resources, $_ for sort keys %$together;
 
         for my $node (keys %$ns) {
+            $blocking_resources_by_node->{$node} = [];
             next if $ns->{$node} ne 'online';
 
             for my $csid (sort keys %$separate) {
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH ha-manager 1/1] api: relocate/migrate resource: improve initialization of variables to avoid Perl warning

[Fiona Ebner]

The commit title should include a fix #6881

I didn't make it in time to send the patch and reply in the forum to
avoid the need for the user to create the bug report.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH ha-manager 1/1] api: relocate/migrate resource: improve initialization of variables to avoid Perl warning

[Daniel Kral]

On Wed Oct 1, 2025 at 4:02 PM CEST, Fiona Ebner wrote:
> diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
> index 301c62f..b52465f 100644
> --- a/src/PVE/HA/Config.pm
> +++ b/src/PVE/HA/Config.pm
> @@ -397,6 +397,7 @@ sub get_resource_motion_info {
>          push @$dependent_resources, $_ for sort keys %$together;
>  
>          for my $node (keys %$ns) {
> +            $blocking_resources_by_node->{$node} = [];

Unfortunately this breaks some callers of get_resource_motion_info(...),
which assume that a set $blocking_resources_by_node->{$node} means that
there was some blocking HA resource on that node, e.g.

$ ha-manager crm-command migrate vm:10000 node2
cannot migrate resource 'vm:10000' to node 'node2':

even though vm:10000 isn't in any HA rule. It might also break the
migrate precondition API in pve-container and qemu-server as it will not
add the node to 'allowed-nodes' there, which AFAICT we don't use in our
web interface but maybe some API users?

To be fair, these users should also check whether there are any
elements in the array to be safe, but it might be less churn to make it
a ref + array ref check for both API endpoints here?

>              next if $ns->{$node} ne 'online';
>  
>              for my $csid (sort keys %$separate) {



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH ha-manager 1/1] api: relocate/migrate resource: improve initialization of variables to avoid Perl warning

[Fiona Ebner]

Am 02.10.25 um 2:18 PM schrieb Daniel Kral:
> On Wed Oct 1, 2025 at 4:02 PM CEST, Fiona Ebner wrote:
>> diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
>> index 301c62f..b52465f 100644
>> --- a/src/PVE/HA/Config.pm
>> +++ b/src/PVE/HA/Config.pm
>> @@ -397,6 +397,7 @@ sub get_resource_motion_info {
>>          push @$dependent_resources, $_ for sort keys %$together;
>>  
>>          for my $node (keys %$ns) {
>> +            $blocking_resources_by_node->{$node} = [];
> 
> Unfortunately this breaks some callers of get_resource_motion_info(...),
> which assume that a set $blocking_resources_by_node->{$node} means that
> there was some blocking HA resource on that node, e.g.
> 
> $ ha-manager crm-command migrate vm:10000 node2
> cannot migrate resource 'vm:10000' to node 'node2':

Are you sure this is with the first part of my patch included? It works
for me.

> even though vm:10000 isn't in any HA rule. It might also break the
> migrate precondition API in pve-container and qemu-server as it will not
> add the node to 'allowed-nodes' there, which AFAICT we don't use in our
> web interface but maybe some API users?

Ah, good catch! I did not notice that there are callers outside the
ha-manager package. Okay, I'll go with the other approach then for v2.

> 
> To be fair, these users should also check whether there are any
> elements in the array to be safe, but it might be less churn to make it
> a ref + array ref check for both API endpoints here?
> 
>>              next if $ns->{$node} ne 'online';
>>  
>>              for my $csid (sort keys %$separate) {


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] superseded: [PATCH ha-manager 0/1] api: relocate/migrate resource: improve initialization of variables to avoid Perl warning

[Fiona Ebner]

superseded-by:
https://lore.proxmox.com/pve-devel/20251002135011.136664-1-f.ebner@proxmox.com/T/


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel