[PATCH v2 storage 08/15] iscsi: rework to update discovery db and simplify login

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

Manually updating the discovery db, and running a discovery for the
non-mapping case, simplifies the tracking of sessions and node entries
to decide if a login is necessary to a target.

Logins are done on a node+target level, rather than on a portal
level. This makes a single iscsiadm call per target instead of per
portal.
One downside is that warnings are logged if some sessions already exist,
but not for all portals of a target.

Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
---
 src/PVE/Storage/ISCSIPlugin.pm | 48 ++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
index 0f77f3f..b3f4e60 100644
--- a/src/PVE/Storage/ISCSIPlugin.pm
+++ b/src/PVE/Storage/ISCSIPlugin.pm
@@ -74,7 +74,7 @@ my $get_local_config = sub {
         return $res;
     } else {
         $res->{is_mapping} = 0;
-        $res->{targets}->{ $scfg->{target} } = iscsi_portals($scfg->{target}, $scfg->{portal});
+        $res->{targets}->{ $scfg->{target} } = [$scfg->{portal}];
         return $res;
     }
 };
@@ -237,9 +237,6 @@ sub iscsi_login {
 
     assert_iscsi_support();
 
-    eval { iscsi_discovery($target, $portals, $cache); };
-    warn $@ if $@;
-
     # Disable retries to avoid blocking pvestatd for too long, next iteration will retry anyway
     eval {
         my $cmd = [
@@ -259,7 +256,14 @@ sub iscsi_login {
     };
     warn $@ if $@;
 
-    run_command([$ISCSIADM, '--mode', 'node', '--targetname', $target, '--login']);
+    # iscsiadm warns if some target/portals are already logged in
+    # a workaround would be to check each of them and only log in on those
+    # that have no session yet, rather than just doing a log in per node
+    my $cmd = [
+        $ISCSIADM, '--mode', 'node', '--targetname', $target, '--login',
+    ];
+    eval { run_command($cmd); };
+    warn $@ if $@;
 }
 
 sub iscsi_logout {
@@ -699,32 +703,30 @@ sub activate_storage {
     return if !assert_iscsi_support(1);
 
     my $local_cfg = $get_local_config->($scfg);
+
+    update_iscsi_discovery_db($local_cfg, $cache);
+
     my $targets = {};
     for my $target (keys $local_cfg->{targets}->%*) {
         my $sessions = iscsi_session($cache, $target);
         $targets->{$target}->{sessions} = $sessions;
         $targets->{$target}->{portals} = $local_cfg->{targets}->{$target};
     }
+
     my $do_login = 0;
+    my $login_targets = {};
     for my $target (keys $targets->%*) {
-        $do_login = 1 if !defined($targets->{$target}->{sessions});
-    }
-
-    if (!$do_login) {
-        # We should check that sessions for all portals are available
-        for my $target (keys $targets->%*) {
-            my $session_portals = [map { $_->{portal} } ($targets->{$target}->{sessions}->@*)];
-            for my $portal ($targets->{$target}->{portals}->@*) {
-                if (!grep(/^\Q$portal\E(?::3260)?$/, $session_portals->@*)) {
-                    $do_login = 1;
-                    last;
-                }
+        my $sessions = [map { $_->{portal} } ($targets->{$target}->{sessions}->@*)];
+        for my $portal ($targets->{$target}->{portals}->@*) {
+            if (!grep(/\Q$portal\E$/, $sessions->@*)) {
+                push $login_targets->{$target}->@*, $portal;
             }
         }
     }
+    $do_login = keys $login_targets->%*;
 
     if ($do_login) {
-        for my $target (keys $targets->%*) {
+        for my $target (keys $login_targets->%*) {
             eval { iscsi_login($target, $targets->{$target}->{portals}, $cache); };
             warn $@ if $@;
         }
@@ -857,9 +859,17 @@ sub check_connection {
 
     my $cache = {};
     my $local_cfg = $get_local_config->($scfg);
+    my $is_mapping = $local_cfg->{is_mapping};
 
     for my $target (keys $local_cfg->{targets}->%*) {
-        for my $portal ($local_cfg->{targets}->{$target}->@*) {
+        my $portals;
+        if ($is_mapping) {
+            $portals = $local_cfg->{targets}->{$target};
+        } else {
+            $portals = iscsi_portals($target, $local_cfg->{targets}->{$target}->[0]);
+        }
+
+        for my $portal ($portals->@*) {
             my $result = iscsi_test_portal($target, $portal, $cache);
             return $result if $result;
         }
-- 
2.47.3