From: "Daniel Kral" <d.kral@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Cc: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH ha-manager v3 4/8] rules: resource affinity: decouple get_resource_affinity helper from Usage class
Date: Mon, 03 Nov 2025 17:01:15 +0100	[thread overview]
Message-ID: <DDZ6ORD7HBTG.2LQL41TSFEAB1@proxmox.com> (raw)
In-Reply-To: <DDZ6GGB1XTNX.1XC2RTAJJ9K09@proxmox.com>
On Mon Nov 3, 2025 at 4:50 PM CET, Michael Köppl wrote:
> On Mon Oct 27, 2025 at 5:43 PM CET, Daniel Kral wrote:
>> @@ -543,14 +544,16 @@ sub get_resource_affinity : prototype($$$) {
>>              for my $csid (keys %{ $rule->{resources} }) {
>>                  next if $csid eq $sid;
>>  
>> -                my $nodes = $online_node_usage->get_service_nodes($csid);
>> -
>> -                next if !$nodes || !@$nodes; # skip unassigned nodes
>> +                my ($state, $node, $target) = $ss->{$csid}->@{qw(state node target)};
>
> During my testing of your other series [0], I noticed a problem with
> resources that are set to 'ignored'. $state is undefined if the
> resource is set to ignored because AFAICT its corresponding entry in $ss
> is removed in the loop that removes stale or ignored service in the
> manage function. This results in the following warning being printed
> repeatedly:
>
> Nov 03 16:37:07 pve-trixbuild02 pve-ha-crm[122707]: removing stale service 'ct:101' (ignored state requested)
> Nov 03 16:37:07 pve-trixbuild02 pve-ha-crm[122707]: Use of uninitialized value $state in string eq at /usr/share/perl5/PVE/HA/Usage.pm line 77.
> Nov 03 16:37:07 pve-trixbuild02 pve-ha-crm[122707]: Use of uninitialized value $state in string eq at /usr/share/perl5/PVE/HA/Usage.pm line 77.
Good catch, thanks!
Oh this makes sense, there's auto-vivification on $ss directly, because
of $ss->{$csid}->... in
	my ($state, $node, $target) = $ss->{$csid}->@{qw(state node target)};
and especially here, the loop iterates over $rule->{resources}->%*,
which is completely external and doesn't filter out the ignored
services. I'll fix that in a v4 and check for other possible
auto-vivification bugs in the patches, thanks!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply	other threads:[~2025-11-03 16:00 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-27 16:43 [pve-devel] [PATCH ha-manager/perl-rs/proxmox v3 00/11] Granular online_node_usage accounting Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH proxmox v3 1/1] resource-scheduling: change score_nodes_to_start_service signature Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH perl-rs v3 1/2] pve-rs: resource_scheduling: allow granular usage changes Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH perl-rs v3 2/2] test: resource_scheduling: use score_nodes helper to imitate HA Manager Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH ha-manager v3 1/8] manager: remove redundant recompute_online_node_usage from next_state_recovery Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH ha-manager v3 2/8] manager: remove redundant add_service_usage_to_node " Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH ha-manager v3 3/8] manager: remove redundant add_service_usage_to_node from next_state_started Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH ha-manager v3 4/8] rules: resource affinity: decouple get_resource_affinity helper from Usage class Daniel Kral
2025-11-03 15:50   ` Michael Köppl
2025-11-03 16:01     ` Daniel Kral [this message]
2025-10-27 16:43 ` [pve-devel] [PATCH ha-manager v3 5/8] manager: make recompute_online_node_usage use add_service_usage helper Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH ha-manager v3 6/8] usage: allow granular changes to Usage implementations Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH ha-manager v3 7/8] manager: make online node usage computation granular Daniel Kral
2025-10-27 16:43 ` [pve-devel] [PATCH ha-manager v3 8/8] implement static service stats cache Daniel Kral
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=DDZ6ORD7HBTG.2LQL41TSFEAB1@proxmox.com \
    --to=d.kral@proxmox.com \
    --cc=pve-devel-bounces@lists.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.