From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 4DF4B1FF0E5 for ; Wed, 15 Jul 2026 12:39:13 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id B09872143D; Wed, 15 Jul 2026 12:39:12 +0200 (CEST) Message-ID: <3c898c26-baf3-4f6f-b05d-09b37c8f0df7@proxmox.com> Date: Wed, 15 Jul 2026 12:39:08 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 storage 04/15] iscsi: introduce mapping support To: Dietmar Maurer , pve-devel@lists.proxmox.com References: <20260430173220.441001-1-m.limbeck@proxmox.com> <20260430173220.441001-5-m.limbeck@proxmox.com> <229b5f15-eb3f-405f-bf4f-f3b99ee084a4@proxmox.com> Content-Language: en-US From: Mira Limbeck In-Reply-To: <229b5f15-eb3f-405f-bf4f-f3b99ee084a4@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784111930346 X-SPAM-LEVEL: Spam detection results: 0 DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust 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: YYTGPQJTTU4KRVSFG67HVBZE4ES7PKFP X-Message-ID-Hash: YYTGPQJTTU4KRVSFG67HVBZE4ES7PKFP X-MailFrom: m.limbeck@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 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 >> --- >>   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/