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 B79991FF0E3 for ; Tue, 04 Aug 2026 11:09:16 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 362C821701; Tue, 04 Aug 2026 11:08:27 +0200 (CEST) From: Dietmar Maurer To: pve-devel@lists.proxmox.com Subject: [PATCH storage 5/7] iscsi: add iscsi-node-map property for per-node target and portals Date: Tue, 4 Aug 2026 11:08:17 +0200 Message-ID: <20260804090819.2136483-6-dietmar@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260804090819.2136483-1-dietmar@proxmox.com> References: <20260804090819.2136483-1-dietmar@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 2 AWL -0.104 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: IF54DEFKQVERFUGCO4NUHSHV2C7CR4JD X-Message-ID-Hash: IF54DEFKQVERFUGCO4NUHSHV2C7CR4JD X-MailFrom: dietmar@zilli.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 setups where every node reaches the same SAN through a different portal and target, a single cluster-wide portal/target pair is not enough. Allow configuring a list of static mappings, each optionally restricted to a set of nodes. Mapped entries bypass sendtargets discovery: node db records are created for exactly the listed portals and stale records are pruned, so this also works when the SAN filters or does not implement discovery. Entries explicitly listing a node replace the entries without a node list on that node, and portals of entries with the same target are merged. Entries without a target apply to the cluster-wide target, so per-node portals do not require repeating it. A node can resolve to multiple targets, which allows multipath across per-controller target names. Nodes without an applicable entry keep using the cluster-wide target and discovery address. Pruning logs out the session on a dropped portal first, because iscsiadm refuses to remove a record that a session still uses. So a portal removed from the configuration does not keep a session alive that nothing re-establishes. A stale session is kept as long as no configured portal has one, so replacing a portal cannot leave the target without a path; the next poll prunes it once the replacement is up. Resolving the configuration for the local node is a package sub, so code that needs to know which targets and how many portals a node should end up with can reuse it. Signed-off-by: Dietmar Maurer --- src/PVE/Storage/ISCSIPlugin.pm | 297 +++++++++++++++++++++++++++++---- 1 file changed, 264 insertions(+), 33 deletions(-) diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm index 85366ba..585b8c2 100644 --- a/src/PVE/Storage/ISCSIPlugin.pm +++ b/src/PVE/Storage/ISCSIPlugin.pm @@ -7,6 +7,7 @@ use File::stat; use IO::Dir; use IO::File; +use PVE::INotify; use PVE::JSONSchema qw(get_standard_option); use PVE::Storage::Plugin; use PVE::Tools @@ -125,6 +126,117 @@ sub iscsi_portals { } } +# normalize for comparison: iscsiadm reports portals with an explicit port +my sub normalize_portal { + my ($portal) = @_; + + my ($host, $port) = PVE::Tools::parse_host_and_port($portal); + return undef if !$host; + $port //= 3260; + return "$host:$port"; +} + +# make the node db records for $target match exactly the given portals +sub iscsi_sync_node_records { + my ($target, $portals, $cache) = @_; + + assert_iscsi_support(); + + my $wanted = {}; + for my $portal (@$portals) { + my $key = normalize_portal($portal); + $wanted->{$key} = $portal if defined($key); + } + + my $existing = {}; + eval { + run_command( + [$ISCSIADM, '--mode', 'node'], + outfunc => sub { + my $line = shift; + + if ($line =~ $ISCSI_TARGET_RE) { + my ($portal, $record_target) = ($1, $2); + if ($record_target eq $target) { + my $key = normalize_portal($portal); + $existing->{$key} = $portal if defined($key); + } + } + }, + ); + }; + + for my $key (sort keys %$wanted) { + next if $existing->{$key}; + my $cmd = [ + $ISCSIADM, + '--mode', + 'node', + '--op', + 'new', + '--targetname', + $target, + '--portal', + $wanted->{$key}, + ]; + eval { run_command($cmd); }; + warn $@ if $@; + } + + # prune stale records, so removed portals no longer get logged into + my $stale = [grep { !$wanted->{$_} } sort keys %$existing]; + return if !scalar(@$stale); + + # iscsiadm refuses to drop a record that a session still uses, so the + # session has to go first + my $sids = {}; + for my $session (@{ iscsi_session($cache, $target) // [] }) { + my $key = normalize_portal($session->{portal}); + $sids->{$key} = $session->{session_id} if defined($key); + } + + # A portal that replaced another one only gets a session from the login + # that follows, so tearing the old one down here would leave the target + # without a path. Keep it until a configured portal took over. + my $keep_last_path = + !grep { $sids->{$_} && iscsi_test_session($sids->{$_}) } keys %$wanted; + + for my $key (@$stale) { + my $portal = $existing->{$key}; + + if (defined(my $sid = $sids->{$key})) { + if ($keep_last_path) { + warn "keeping stale iscsi session $sid for $target via $portal:" + . " no configured portal has a session yet\n"; + next; + } + + print "logging out of stale iscsi session $sid: $target via $portal\n"; + eval { run_command([$ISCSIADM, '--mode', 'session', '--sid', $sid, '--logout']); }; + if (my $err = $@) { + warn $err; + next; # dropping the record would fail as well + } + # session state changed, force a re-query + delete $cache->{iscsi_sessions}; + } + + my $cmd = [ + $ISCSIADM, + '--mode', + 'node', + '--op', + 'delete', + '--targetname', + $target, + '--portal', + $portal, + ]; + eval { run_command($cmd); }; + warn $@ if $@; + } +} + # Discovery persists a node db record for every returned target unless # $nonpersistent is set. Callers that only inspect a portal should set it, # callers that log in afterwards rely on the records. @@ -167,12 +279,15 @@ sub iscsi_discovery { } sub iscsi_login { - my ($target, $portals, $cache) = @_; + my ($target, $portals, $cache, $static) = @_; assert_iscsi_support(); - eval { iscsi_discovery($target, $portals, $cache); }; - warn $@ if $@; + # static configurations have their node db records synced already + if (!$static) { + eval { iscsi_discovery($target, $portals, $cache); }; + warn $@ if $@; + } # Disable retries to avoid blocking pvestatd for too long, next iteration will retry anyway eval { @@ -367,6 +482,75 @@ sub plugindata { }; } +my $iscsi_node_map_fmt = { + nodes => get_standard_option( + 'pve-node-list', + { + description => "List of nodes for which this entry applies (all nodes if unset).", + optional => 1, + }, + ), + target => { + description => "iSCSI target used for this entry (defaults to the storage's target).", + type => 'string', + format => 'pve-storage-iscsi-target', + optional => 1, + }, + portals => { + description => "Static list of portals used for this target (bypasses discovery).", + type => 'string', + format => 'pve-storage-portal-dns-list', + }, +}; + +# Resolves the storage configuration for the local node into +# { static => 0|1, targets => [{ target, portals }, ...] }, where 'static' +# tells whether the portals come from an iscsi-node-map entry rather than +# from discovery. +# +# Entries explicitly listing the node replace the entries without a node +# list; portals of entries with the same target are merged. Falls back to +# the cluster-wide target and discovery address when no entry applies. +sub get_node_targets { + my ($scfg) = @_; + + my $nodename = PVE::INotify::nodename(); + + my $explicit = []; + my $default = []; + for my $item (@{ $scfg->{'iscsi-node-map'} // [] }) { + my $entry = PVE::JSONSchema::parse_property_string($iscsi_node_map_fmt, $item); + $entry->{target} //= $scfg->{target}; + if (!defined($entry->{nodes})) { + push @$default, $entry; + } elsif (grep { $_ eq $nodename } PVE::Tools::split_list($entry->{nodes})) { + push @$explicit, $entry; + } + } + + my $entries = scalar(@$explicit) ? $explicit : $default; + if (!scalar(@$entries)) { + return { + static => 0, + targets => [{ target => $scfg->{target}, portals => [$scfg->{portal}] }], + }; + } + + my $portals = {}; + my $targets = []; + for my $entry (@$entries) { + my $target = $entry->{target}; + push @$targets, $target if !$portals->{$target}; + $portals->{$target}->{$_} = 1 for PVE::Tools::split_list($entry->{portals}); + } + + return { + static => 1, + targets => + [map { { target => $_, portals => [sort keys %{ $portals->{$_} }] } } @$targets], + }; +} + sub properties { return { target => { @@ -380,6 +564,18 @@ sub properties { type => 'string', format => 'pve-storage-portal-dns', }, + 'iscsi-node-map' => { + description => "Static target and portal configuration, bypassing sendtargets" + . " discovery. Entries that explicitly list a node replace the entries" + . " without a node list on that node. Portals of entries with the same" + . " target are merged.", + type => 'array', + optional => 1, + items => { + type => 'string', + format => $iscsi_node_map_fmt, + }, + }, }; } @@ -387,6 +583,7 @@ sub options { return { portal => { fixed => 1 }, target => { fixed => 1 }, + 'iscsi-node-map' => { optional => 1 }, nodes => { optional => 1 }, disable => { optional => 1 }, content => { optional => 1 }, @@ -465,11 +662,14 @@ sub list_images { # we have no owner for iscsi devices - my $target = $scfg->{target}; - - if (my $dat = $cache->{iscsi_devices}->{$target}) { + my $seen = {}; + for my $node_target (@{ get_node_targets($scfg)->{targets} }) { + my $dat = $cache->{iscsi_devices}->{ $node_target->{target} }; + next if !$dat; foreach my $volname (keys %$dat) { + # a LUN reached via multiple targets shares its stable path + next if $seen->{$volname}++; my $volid = "$storeid:$volname"; @@ -500,8 +700,14 @@ sub iscsi_session { sub status { my ($class, $storeid, $scfg, $cache) = @_; - my $session = iscsi_session($cache, $scfg->{target}); - my $active = defined($session) ? 1 : 0; + # activate_storage keeps trying to log into targets without a session + my $active = 0; + for my $node_target (@{ get_node_targets($scfg)->{targets} }) { + if (defined(iscsi_session($cache, $node_target->{target}))) { + $active = 1; + last; + } + } return (0, 0, 0, $active); } @@ -511,28 +717,41 @@ sub activate_storage { return if !assert_iscsi_support(1); - my $sessions = iscsi_session($cache, $scfg->{target}); - my $portals = iscsi_portals($scfg->{target}, $scfg->{portal}); - my $do_login = !defined($sessions); + my $node_targets = get_node_targets($scfg); + my $static = $node_targets->{static}; - if (!$do_login) { - # We should check that sessions for all portals are available - my $session_portals = [map { $_->{portal} } (@$sessions)]; + for my $node_target (@{ $node_targets->{targets} }) { + my ($target, $portals) = ($node_target->{target}, $node_target->{portals}); - for my $portal (@$portals) { - if (!grep(/^\Q$portal\E$/, @$session_portals)) { - $do_login = 1; - last; + if ($static) { + iscsi_sync_node_records($target, $portals, $cache); + } else { + $portals = iscsi_portals($target, $portals->[0]); + } + + my $sessions = iscsi_session($cache, $target); + my $do_login = !defined($sessions); + + if (!$do_login) { + # We should check that sessions for all portals are available + my $session_portals = + { map { (normalize_portal($_->{portal}) // '') => 1 } (@$sessions) }; + + for my $portal (@$portals) { + if (!$session_portals->{ normalize_portal($portal) // '' }) { + $do_login = 1; + last; + } } } - } - if ($do_login) { - eval { iscsi_login($scfg->{target}, $portals, $cache); }; - warn $@ if $@; - } else { - # make sure we get all devices - iscsi_session_rescan($sessions); + if ($do_login) { + eval { iscsi_login($target, $portals, $cache, $static); }; + warn $@ if $@; + } else { + # make sure we get all devices + iscsi_session_rescan($sessions); + } } } @@ -541,8 +760,11 @@ sub deactivate_storage { return if !assert_iscsi_support(1); - if (defined(iscsi_session($cache, $scfg->{target}))) { - iscsi_logout($scfg->{target}); + for my $node_target (@{ get_node_targets($scfg)->{targets} }) { + my $target = $node_target->{target}; + if (defined(iscsi_session($cache, $target))) { + iscsi_logout($target); + } } } @@ -640,18 +862,27 @@ sub activate_volume { my $device_path = $udev_query_path->($real_path); my $resolved_paths = $resolve_virtual_devices->($device_path); - my $found = $check_devices_part_of_target->($resolved_paths, $scfg->{target}); - die "volume '$volname' not part of target '$scfg->{target}'\n" if !$found; + my $found = 0; + for my $node_target (@{ get_node_targets($scfg)->{targets} }) { + $found = $check_devices_part_of_target->($resolved_paths, $node_target->{target}); + last if $found; + } + die "volume '$volname' not part of any configured target\n" if !$found; } sub check_connection { my ($class, $storeid, $scfg) = @_; my $cache = {}; - my $portals = iscsi_portals($scfg->{target}, $scfg->{portal}); - for my $portal (@$portals) { - my $result = iscsi_test_portal($scfg->{target}, $portal, $cache); - return $result if $result; + my $node_targets = get_node_targets($scfg); + for my $node_target (@{ $node_targets->{targets} }) { + my ($target, $portals) = ($node_target->{target}, $node_target->{portals}); + $portals = iscsi_portals($target, $portals->[0]) if !$node_targets->{static}; + + for my $portal (@$portals) { + my $result = iscsi_test_portal($target, $portal, $cache); + return $result if $result; + } } return 0; -- 2.47.3