From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 0AE081FF15C for ; Fri, 17 Oct 2025 17:46:20 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5B45C763F; Fri, 17 Oct 2025 17:46:38 +0200 (CEST) Mime-Version: 1.0 Date: Fri, 17 Oct 2025 17:46:03 +0200 Message-Id: From: "Daniel Kral" To: "Fiona Ebner" , "Proxmox VE development discussion" X-Mailer: aerc 0.20.0 References: <20250930142021.366529-1-d.kral@proxmox.com> <20250930142021.366529-9-d.kral@proxmox.com> <2416d9fd-c47f-48c9-ac0b-c7f8e4d754ab@proxmox.com> In-Reply-To: <2416d9fd-c47f-48c9-ac0b-c7f8e4d754ab@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1760715959938 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.014 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH ha-manager 5/9] rules: resource affinity: decouple get_resource_affinity helper from Usage class X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On Fri Oct 17, 2025 at 1:14 PM CEST, Fiona Ebner wrote: > Am 30.09.25 um 4:21 PM schrieb Daniel Kral: >> I wanted to put this information in PVE::HA::Usage directly, but figured >> it would make Usage much more dependent on $ss / $sd.. We can still do >> that later on or move the helper elsewhere, e.g. making ServiceStatus >> its own class? > > I think having the get_used_service_nodes() helper in PVE::HA::Usage is > better than in PVE::HA::Tools, because to me, "tools" sounds sounds like > things that should not be concerned with concrete HA state logic. You > could adapt the signature to take e.g. > ($state, $online_nodes, $node, $target) > rather than $sd to avoid any need to know about its internal structure. Sounds great, I'll do that! > >> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm >> index 468e41eb..0226e427 100644 >> --- a/src/PVE/HA/Manager.pm >> +++ b/src/PVE/HA/Manager.pm >> @@ -121,12 +121,12 @@ sub flush_master_status { >> >> =head3 select_service_node(...) >> >> -=head3 select_service_node($rules, $online_node_usage, $sid, $service_conf, $sd, $node_preference) >> +=head3 select_service_node($rules, $online_node_usage, $sid, $service_conf, $ss, $node_preference) >> > > Pre-existing, but is this duplicate heading intentional? Yes, I picked it up from the Perl documentation RFC. This makes it easier to reference it in perlpod when one can use only the short heading instead of having to write out the heading with all of the parameters, while the latter will be used for the LSP AFAICT (which I actually don't use myself..). L<< selects the service's node|/select_service_node(...) >>> This one could be removed because currently it isn't used anywhere. > >> diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm >> index 71eb5d0b..7f718e25 100644 >> --- a/src/PVE/HA/Tools.pm >> +++ b/src/PVE/HA/Tools.pm >> @@ -188,6 +188,35 @@ sub count_fenced_services { >> return $count; >> } >> >> +sub get_used_service_nodes { >> + my ($sd, $online_nodes) = @_; >> + >> + my $result = {}; >> + >> + my ($state, $node, $target) = $sd->@{qw(state node target)}; >> + >> + return $result if $state eq 'stopped' || $state eq 'request_start'; >> + >> + if ( >> + $state eq 'started' >> + || $state eq 'request_stop' >> + || $state eq 'fence' >> + || $state eq 'freeze' >> + || $state eq 'error' >> + || $state eq 'recovery' >> + || $state eq 'migrate' >> + || $state eq 'relocate' >> + ) { >> + $result->{current} = $node if $online_nodes->{$node}; >> + } >> + >> + if ($state eq 'migrate' || $state eq 'relocate' || $state eq 'request_start_balance') { >> + $result->{target} = $target if defined($target) && $online_nodes->{$target}; >> + } >> + >> + return $result; > > Returning ($current, $target) seems to better match what callers need. Right, will do! _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel