Re: [PATCH v2 storage 07/15] iscsi: introduce helper to update discovery db

[Samuel Rufinatscha <s.rufinatscha@proxmox.com>]

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 <m.limbeck@proxmox.com>
> ---
>   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 {