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 BE31D1FF133 for ; Mon, 11 May 2026 18:46:15 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 215F521527; Mon, 11 May 2026 18:46:14 +0200 (CEST) Message-ID: Date: Mon, 11 May 2026 18:46:05 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 storage 07/15] iscsi: introduce helper to update discovery db To: Mira Limbeck , pve-devel@lists.proxmox.com References: <20260430173220.441001-1-m.limbeck@proxmox.com> <20260430173220.441001-8-m.limbeck@proxmox.com> Content-Language: en-US From: Samuel Rufinatscha In-Reply-To: <20260430173220.441001-8-m.limbeck@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.225 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [iscsiplugin.pm] Message-ID-Hash: SGA456YJLKRQCIULCRW7JH3AZT2SAO2F X-Message-ID-Hash: SGA456YJLKRQCIULCRW7JH3AZT2SAO2F X-MailFrom: s.rufinatscha@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 4/30/26 7:32 PM, Mira Limbeck wrote: > In the case of using mappings, running a discovery against the > configured portals could lead to lots of additional node entries, as can > be seen with the original iSCSI plugin way. > > Add a discovery db update helper that adds and removes node entries > based on what is configured via mappings. > For the non-mapping case a discovery is done. If no portal entries are > returned, only the portal from the storage config is kept for the > target. > > Also adds a helper to gather all node entries. > > Signed-off-by: Mira Limbeck > --- > src/PVE/Storage/ISCSIPlugin.pm | 164 +++++++++++++++++++++++++++++++++ > 1 file changed, 164 insertions(+) > > diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm > index 8f2bdb5..0f77f3f 100644 > --- a/src/PVE/Storage/ISCSIPlugin.pm > +++ b/src/PVE/Storage/ISCSIPlugin.pm > @@ -79,6 +79,36 @@ my $get_local_config = sub { > } > }; > > +sub iscsi_node_list { > + assert_iscsi_support(); > + > + my $cmd = [$ISCSIADM, '--mode', 'node']; > + > + my $res = {}; > + eval { > + run_command( > + $cmd, > + errmsg => 'iscsi node scan failed', > + outfunc => sub { > + my $line = shift; > + > + # example: 10.67.1.144:3260,4294967295 iqn.2003-01.org.linux-iscsi.iscsi.x8664:sn.81bb080df375 > + if ($line =~ m/^$ISCSI_TARGET_RE$/) { > + my ($portal, $target) = ($1, $2); > + > + push $res->{$target}->@*, $portal; > + } > + }, > + ); > + }; > + > + if (my $err = $@) { > + die $err if $err !~ m/: No records found$/i; > + } > + > + return $res; > +} > + > sub iscsi_session_list { > assert_iscsi_support(); > > @@ -368,6 +398,140 @@ sub iscsi_device_list { > return $res; > } > > +sub update_iscsi_discovery_db { > + my ($local_cfg, $cache) = @_; > + > + if ($local_cfg->{is_mapping}) { > + # do mapping specific discoverydb update > + > + my $host_targets = iscsi_node_list(); AFAICT this returns all local iscsiadm node entries > + my $mapped_targets = $local_cfg->{targets}; > + my $added = {}; > + my $removed = {}; > + > + for my $host_target (keys $host_targets->%*) { > + if (!defined($mapped_targets->{$host_target})) { Could this therefore mark entries from another iSCSI storage/mapping as stale and remove them below? > + # add all portals of that target to be removed > + $removed->{$host_target} = $host_targets->{$host_target}; > + } else { > + for my $host_target_portal ($host_targets->{$host_target}->@*) { > + if (!grep(/^\Q$host_target_portal\E$/, $mapped_targets->{$host_target}->@*)) { > + push $removed->{$host_target}->@*, $host_target_portal; > + } > + } > + } > + } > + > + for my $mapped_target (keys $mapped_targets->%*) { > + if (!defined($host_targets->{$mapped_target})) { > + # add all portals of that target to be added > + $added->{$mapped_target} = $mapped_targets->{$mapped_target}; > + } else { > + for my $mapped_target_portal ($mapped_targets->{$mapped_target}->@*) { > + if ( > + !grep(/^\Q$mapped_target_portal\E$/, > + $host_targets->{$mapped_target}->@*) > + ) { > + push $added->{$mapped_target}->@*, $mapped_target_portal; > + } > + } > + } > + } > + > + # remove stale entries > + for my $target (keys $removed->%*) { > + my $target_sessions = iscsi_session($cache, $target); > + my $sessions = {}; > + for my $target_session ($target_sessions->@*) { > + $sessions->{ $target_session->{portal} } = $target_session->{session_id}; > + } > + > + my $removed_sessions = []; > + for my $portal ($removed->{$target}->@*) { > + # log out of session before removing the stale node entry > + # iscsiadm returns an error otherwise > + print "removing stale iscsi session: $target via $portal\n"; > + if (defined($sessions->{$portal})) { > + my $cmd = [ > + $ISCSIADM, '--mode', 'session', '--sid', $sessions->{$portal}, > + '--logout', > + ]; > + eval { run_command($cmd); }; > + warn "failed to log out of stale session: $@\n" if $@; > + > + # remove logged out sessions > + # the list of still active sessions will be used to update cache later > + delete $sessions->{$portal}; > + } > + print "removing stale iscsi target entry: $target via $portal\n"; > + my $cmd = [ > + $ISCSIADM, > + '--mode', > + 'node', > + '--target', > + $target, > + '--portal', > + $portal, > + '-o', > + 'delete', > + ]; > + eval { run_command($cmd); }; > + warn "failed to remove stale node entry: $@\n" if $@; > + } > + # update cache with list of active sessions > + $cache->{iscsi_sessions}->{$target} = > + [map { { portal => $_, session_id => $sessions->{$_} } } keys $sessions->%*]; > + } > + > + # add new mapping entries > + for my $target (keys $added->%*) { > + for my $portal ($added->{$target}->@*) { > + print "adding new iscsi target entry: $target via $portal\n"; > + my $cmd = [ > + $ISCSIADM, > + '--mode', > + 'node', > + '--target', > + $target, > + '--portal', > + $portal, > + '-o', > + 'new', > + ]; > + eval { run_command($cmd); }; > + warn "failed to add new node entry: $@\n" if $@; > + } > + } > + > + } else { > + my $portals = undef; > + my $target = undef; > + for my $config_target (keys $local_cfg->{targets}->%*) { > + $target = $config_target; > + $portals = iscsi_portals($config_target, $local_cfg->{targets}->{$config_target}->[0]); > + last; > + } > + die "no target found for discovery\n" if !defined($target); > + die "no portals found for discovery\n" if !defined($portals); > + > + my $res = eval { iscsi_discovery($target, $portals, $cache); }; > + warn $@ if $@; > + > + if (defined($res->{$target})) { > + $local_cfg->{targets}->{$target} = [map { $_->{portal} } $res->{$target}->@*]; > + } else { > + # no portal is discovered, keep already configured node entries > + # in that case, otherwise we might remove sessions still in use > + # > + # cleanup in that case should be done manually after verifying that > + # active sessions are no longer needed > + $local_cfg->{targets}->{$target} = $portals; > + } > + } > + > + return 1; > +} > + > # Configuration > > sub type {