[PATCH storage 5/7] iscsi: add iscsi-node-map property for per-node target and portals

[Dietmar Maurer <dietmar@proxmox.com>]

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 <dietmar@proxmox.com>
---
 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