Re: [PATCH ha-manager v2 27/40] usage: pass service data to add_service_usage

Daniel Kral
2026-03-30T07:41:16Z

On Mon Mar 30, 2026 at 9:34 AM CEST, Dominik Rusovac wrote:
> see comment inline, otherwise lgtm
>
> On Tue Mar 24, 2026 at 7:30 PM CET, Daniel Kral wrote:
>> The method is already dependent on three members of the service data and
>> in a following patch a fourth member is needed for adding more
>> information to the Usage implementations.
>>
>> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
>> ---
>> changes v1 -> v2:
>> - new!
>>
>> src/PVE/HA/Manager.pm | 11 +++++------
>> src/PVE/HA/Usage.pm | 6 +++---
>> 2 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
>> index 421c17da..d4b75ca9 100644
>> --- a/src/PVE/HA/Manager.pm
>> +++ b/src/PVE/HA/Manager.pm
>> @@ -281,17 +281,17 @@ sub recompute_online_node_usage {
>> foreach my $sid (sort keys %{ $self->{ss} }) {
>> my $sd = $self->{ss}->{$sid};
>>
>> - $online_node_usage->add_service_usage($sid, $sd->{state}, $sd->{node}, $sd->{target});
>> + $online_node_usage->add_service_usage($sid, $sd);
>> }
>>
>> # add remaining non-HA resources to online node usage
>> for my $sid (sort keys %$service_stats) {
>> next if $self->{ss}->{$sid};
>>
>> - my ($node, $state) = $service_stats->{$sid}->@{qw(node state)};
>> -
>> # the migration target is not known for non-HA resources
>> - $online_node_usage->add_service_usage($sid, $state, $node, undef);
>> + my $sd = { $service_stats->%{qw(node state)} };
>
> shouldn't this be:
>
> my $sd = { $service_stats->{$sid}->%{qw(node state)} };
>
> ?

Good catch, thanks!

I think this was left over because I wanted to

my $service_stats = $service_stats->{$sid};

but this would be quite confusing with the variable shadowing here.

>> +
>> + $online_node_usage->add_service_usage($sid, $sd);
>> }
>>
>
> [snip]
>
> Reviewed-by: Dominik Rusovac <d.rusovac@proxmox.com>