Re: [PATCH v2 storage 05/15] iscsi: add helper to get local config

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On Thu, 30 Apr 2026 19:27:03 +0200, Mira Limbeck wrote:
> diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
> @@ -36,6 +36,49 @@ my sub assert_iscsi_support {
> +my $get_local_config = sub {
> +    my ($scfg) = @_;
> +
> +    die "neither 'target' nor 'mapping' defined\n"
> +        if !defined($scfg->{target}) && !defined($scfg->{mapping});
> +
> +    my $res = {};
> +    if ($scfg->{mapping}) {
> +        $res->{is_mapping} = 1;
> +
> +        my $local_mappings =
> +            PVE::Storage::Mapping::find_mapping_on_current_node($scfg->{mapping});
> +        die "no iSCSI per-node entries found for mapping '$scfg->{mapping}'\n"
> +            if !$local_mappings->@*;

The die when $local_mappings is empty fires for any node that legitimately
is not in the mapping's node set, and it propagates through status() and
check_connection() every pvestatd cycle. Treating that node like a
`nodes=`-restricted storage (reporting it as inactive) would be better;
worth defining how storage `nodes=` and per-node `map` coverage relate.

> +        for my $mapping ($local_mappings->@*) {
> +            $res->{targets}->{ $mapping->{target} } = []
> +                if !defined($res->{targets}->{ $mapping->{target} });
> +            my $portals = [PVE::Tools::split_list($mapping->{portals})];
> +
> +            my $add_port = sub {
> +                my ($val) = @_;
> +
> +                my ($ip, $port) = PVE::Tools::parse_host_and_port($val);
> +                if (defined($port)) {
> +                    return $val;
> +                } else {
> +                    # add default port
> +                    return $ip . ':3260';
> +                }
> +            };
> +            $portals->@* = map { $add_port->($_) } $portals->@*;
> +
> +            push $res->{targets}->{ $mapping->{target} }->@*, $portals->@*;
> +        }
> +        return $res;
> +    } else {
> +        $res->{is_mapping} = 0;
> +        $res->{targets}->{ $scfg->{target} } = iscsi_portals($scfg->{target}, $scfg->{portal});
> +        return $res;
> +    }
> +};

Further, portals get canonicalized to host:port only on the mapping branch
(add_port); the else branch keeps whatever iscsi_portals returns. This
asymmetry is what later needs the `(?::3260)?` and unanchored matches in
06/08. Normalizing both branches here once would let the comparisons be
exact everywhere.