From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id B05AD1FF143 for ; Sat, 23 May 2026 23:29:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7292C1B6A4; Sat, 23 May 2026 23:29:12 +0200 (CEST) From: Thomas Lamprecht To: Proxmox VE development discussion , Mira Limbeck Subject: Re: [PATCH v2 storage 07/15] iscsi: introduce helper to update discovery db Date: Sat, 23 May 2026 23:26:10 +0200 Message-ID: <20260523212856.2822353-5-t.lamprecht@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260430173220.441001-8-m.limbeck@proxmox.com> References: <20260430173220.441001-1-m.limbeck@proxmox.com> <20260430173220.441001-8-m.limbeck@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1779571725441 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.005 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 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: 7CZ5G5E7Z2RBWTBLE2PGOO654XVTUUIH X-Message-ID-Hash: 7CZ5G5E7Z2RBWTBLE2PGOO654XVTUUIH X-MailFrom: t.lamprecht@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: On Thu, 30 Apr 2026 19:27:05 +0200, Mira Limbeck wrote:=0D > diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin= .pm=0D > @@ -368,6 +398,140 @@ sub iscsi_device_list {=0D > + } else {=0D > + my $portals =3D undef;=0D > + my $target =3D undef;=0D > + for my $config_target (keys $local_cfg->{targets}->%*) {=0D > + $target =3D $config_target;=0D > + $portals =3D iscsi_portals($config_target, $local_cfg->{targ= ets}->{$config_target}->[0]);=0D > + last;=0D > + }=0D > + die "no target found for discovery\n" if !defined($target);=0D > + die "no portals found for discovery\n" if !defined($portals);=0D > +=0D > + my $res =3D eval { iscsi_discovery($target, $portals, $cache); }= ;=0D > + warn $@ if $@;=0D =0D With 08/15 calling update_iscsi_discovery_db() unconditionally at the top o= f=0D activate_storage, a plain (non-mapping) iSCSI storage now runs a persistent= =0D sendtargets discovery plus a per-portal tcp_ping every pvestatd cycle, so=0D every ~10s. But currently (without this series), discovery runs only when a= =0D login was actually needed. On a SAN exposing many targets/portals this is a= =0D recurring cost and a behavior change. Could be gated on need (no session /= =0D changed portal set), or use non-persistent discovery here too.=0D =0D > + if (defined($res->{$target})) {=0D > + $local_cfg->{targets}->{$target} =3D [map { $_->{portal} } $= res->{$target}->@*];=0D =0D Bisectability: this maps `$_->{portal}` over the discovery result, but=0D iscsi_discovery only returns hashrefs from 13/15 - through 07-12 it still p= ushes=0D plain portal strings. So once 08/15 calls this unconditionally, plain-iSCSI= =0D activate_storage dies ("Can't use a string as a HASH ref") every cycle unti= l=0D 13. Worth reordering so each commit builds and runs.=0D =0D > + } else {=0D > + # no portal is discovered, keep already configured node entr= ies=0D > + # in that case, otherwise we might remove sessions still in = use=0D > + #=0D > + # cleanup in that case should be done manually after verifyi= ng that=0D > + # active sessions are no longer needed=0D > + $local_cfg->{targets}->{$target} =3D $portals;=0D > + }=0D > + }=0D =0D The "keep configured entries, clean up manually" caution is good - but 09/1= 5=0D then adds a second pass that does not honor it; see reply on 09.=0D