Re: [PATCH v2 storage 04/15] iscsi: introduce mapping support

[Mira Limbeck <m.limbeck@proxmox.com>]

On 7/13/26 11:19 AM, Dietmar Maurer wrote:
> This is a miss-understanding. The iscsi "portal" property is
> not really the iscsi "portal" - instead, this refers to the "discovery
> address"
It is actually both. A portal used for discovery, and in most of the
cases, also a portal used for a session.

> (we probably want to rename that in the future).
Yes, that would be good.

> 
> The iscsiadm tool has the same name confusion, using "portal" in the
> discovery
> command cli instead of discovery-address.
It looks like it doesn't make a distinction between a discovery portal
and a session portal. In the end it is just an IP/Hostname and an
optional port.
The distinction mostly comes from the context it is used in.

> 
> So I would leave "portal" a required property.
Then we would have to extend it to accept a list of portals.
Some SANs expose different portals to different clients, depending on
the controller the client is connected to.

This is one of the limitations encountered by users of the current iSCSI
plugin, and one of the reasons I introduced `discovery-portals` as part
of the mapping plugin, with a list of portals per mapping.

And we want to support multiple discovery portals in any case.

> 
> I also wonder what happens when we have both, a "target" and a "mapping"?
If you look at patch 5 [0], the helper that is used prefers mappings
when a mapping is found in the configuration. It ignores any configured
`target` then.

> 
> - Dietmar
> 
> On 4/30/26 7:31 PM, Mira Limbeck wrote:
>> `portal` and `target` should be optional then. There is no need for
>> those when a mapping is specified. In return the config needs to be
>> checked to make sure either portal and target are defined, or a mapping.
>>
>> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
>> ---
>>   src/PVE/Storage/ISCSIPlugin.pm | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/
>> ISCSIPlugin.pm
>> index 30f4178..88a35be 100644
>> --- a/src/PVE/Storage/ISCSIPlugin.pm
>> +++ b/src/PVE/Storage/ISCSIPlugin.pm
>> @@ -9,6 +9,7 @@ use IO::File;
>>     use PVE::JSONSchema qw(get_standard_option);
>>   use PVE::Storage::Plugin;
>> +use PVE::Storage::Mapping;
>>   use PVE::Tools
>>       qw(run_command file_read_firstline trim dir_glob_regex
>> dir_glob_foreach $IPV4RE $IPV6RE);
>>   @@ -354,8 +355,9 @@ sub properties {
>>     sub options {
>>       return {
>> -        portal => { fixed => 1 },
>> -        target => { fixed => 1 },
>> +        portal => { fixed => 1, optional => 1 },
>> +        target => { fixed => 1, optional => 1 },
>> +        mapping => { optional => 1 },
>>           nodes => { optional => 1 },
>>           disable => { optional => 1 },
>>           content => { optional => 1 },
>> @@ -703,4 +705,16 @@ sub volume_import {
>>       die "volume import is not possible on iscsi storage\n";
>>   }
>>   +sub check_config {
>> +    my ($class, $sectionId, $config, $create, $skipSchemaCheck) = @_;
>> +
>> +    my $checked = $class->SUPER::check_config($sectionId, $config,
>> $create, $skipSchemaCheck);
>> +
>> +    # check if either target or mapping is set
>> +    die "iscsi storage '$sectionId' has neither 'target' nor
>> 'mapping' defined\n"
>> +        if !defined($checked->{target}) && !defined($checked-
>> >{mapping});
>> +
>> +    return $checked;
>> +}
>> +
>>   1;


[0]
https://lore.proxmox.com/pve-devel/20260430173220.441001-6-m.limbeck@proxmox.com/