public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH v3 storage 27/76] api: iscsi: add session logout with last-path protection
Date: Fri,  6 Nov 2026 00:31:02 +0100	[thread overview]
Message-ID: <20261105233336.2178064-28-t.lamprecht@proxmox.com> (raw)
In-Reply-To: <20261105233336.2178064-1-t.lamprecht@proxmox.com>

Session logout removes its LUN devices. Losing the last path makes
guests lose their disks or hang if the multipath map queues I/O.

Refuse logout if it would remove the last usable path of a LUN with
local users. Check users of a multipath map unless another path is
active, its checker found it ready or as an ALUA standby, and it
belongs to a logged-in iSCSI session or is a running Fibre Channel
path. For a plain disk, check the disk and its partitions. Always check
direct users of departing paths, excluding multipathd's path-checker
descriptors.

Claim devices losing their last path exclusively until logout returns.
Serialize logouts so two requests cannot each rely on a path the other
removes. Configured storage ownership alone does not block logout:
storage activation can log in again.

This exposes the logout endpoint from Dietmar Maurer's iSCSI API RFC
with these guards.

Link: https://lore.proxmox.com/pve-devel/20260731102156.3947857-1-dietmar@proxmox.com/
Co-authored-by: Dietmar Maurer <dietmar@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/API2/ISCSI.pm                    |  27 +++
 src/PVE/Diskmanage.pm                    |   9 +
 src/PVE/Storage/ISCSIPlugin.pm           | 190 +++++++++++++++-
 src/test/Makefile                        |   3 +-
 src/test/run_block_device_users_tests.pl |  37 +++-
 src/test/run_iscsi_logout_tests.pl       | 271 +++++++++++++++++++++++
 src/test/run_san_api_tests.pl            |   2 +-
 7 files changed, 525 insertions(+), 14 deletions(-)
 create mode 100755 src/test/run_iscsi_logout_tests.pl

diff --git a/src/PVE/API2/ISCSI.pm b/src/PVE/API2/ISCSI.pm
index 28c5a6e4..a3d55845 100644
--- a/src/PVE/API2/ISCSI.pm
+++ b/src/PVE/API2/ISCSI.pm
@@ -265,6 +265,7 @@ __PACKAGE__->register_method({
     code => sub {
         return [
             { name => 'rescan' },
+            { name => 'session-logout' },
             { name => 'session-params' },
             { name => 'status' },
             { name => 'target' },
@@ -387,6 +388,32 @@ __PACKAGE__->register_method({
     },
 });
 
+__PACKAGE__->register_method({
+    name => 'session_logout',
+    path => 'session-logout',
+    method => 'POST',
+    protected => 1,
+    proxyto => 'node',
+    description => "Log out of a single iSCSI session. Refused if a device being disconnected"
+        . " is used directly or a LUN in use would lose its last usable path.",
+    permissions => {
+        check => ['perm', '/', ['Sys.Modify']],
+    },
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+            'session-id' => $session_id_schema,
+        },
+    },
+    returns => { type => 'null' },
+    code => sub {
+        my ($param) = @_;
+        PVE::Storage::ISCSIPlugin::iscsi_logout_guarded($param->{'session-id'});
+        return undef;
+    },
+});
+
 __PACKAGE__->register_method({
     name => 'rescan',
     path => 'rescan',
diff --git a/src/PVE/Diskmanage.pm b/src/PVE/Diskmanage.pm
index 655cee37..0aa005d2 100644
--- a/src/PVE/Diskmanage.pm
+++ b/src/PVE/Diskmanage.pm
@@ -70,11 +70,15 @@ sub block_device_number {
     return PVE::Tools::dev_t_major($st[6]) . ':' . PVE::Tools::dev_t_minor($st[6]);
 }
 
+# Never read device data. Probe failures are not evidence of non-use. Excluding a holder must not
+# exclude direct users; multipathd can be exempted because its checkers keep member paths open.
 sub block_device_users {
     my ($devices, $options) = @_;
     $options //= {};
     my $sys = $options->{sysfs} // '/sys';
     my $proc = $options->{procfs} // '/proc';
+    my $ignore = $options->{'ignore-holders'} // {};
+    my $ignore_executables = $options->{'ignore-executables'} // {};
 
     my $entries = sub {
         my ($dir, $optional) = @_;
@@ -110,6 +114,7 @@ sub block_device_users {
             $visit->($part);
         }
         for my $holder ($entries->("$dir/holders")->@*) {
+            next if $ignore->{$holder};
             push $users->@*, "$name held by $holder";
             $visit->($holder);
         }
@@ -199,6 +204,10 @@ sub block_device_users {
                 die $err;
             }
             next if !defined($dev) || !defined($numbers->{$dev});
+            if (%$ignore_executables) {
+                my $exe = readlink("$proc/$pid/exe");
+                next if defined($exe) && $ignore_executables->{$exe};
+            }
             my $comm = file_read_firstline("$proc/$pid/comm") // '?';
             push $users->@*, "$comm (PID $pid) has $numbers->{$dev} open";
         }
diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
index e7d1ff3f..d10a5549 100644
--- a/src/PVE/Storage/ISCSIPlugin.pm
+++ b/src/PVE/Storage/ISCSIPlugin.pm
@@ -3,12 +3,15 @@ package PVE::Storage::ISCSIPlugin;
 use strict;
 use warnings;
 
+use Cwd qw(abs_path);
 use Digest::SHA qw(sha256_hex);
-use Fcntl qw(LOCK_EX LOCK_NB);
+use Fcntl qw(O_RDONLY O_EXCL O_NONBLOCK LOCK_EX LOCK_NB);
 use IO::Dir;
 use IO::File;
 
+use PVE::Diskmanage;
 use PVE::JSONSchema qw(get_standard_option);
+use PVE::Multipath;
 use PVE::Storage::Plugin;
 use PVE::Tools
     qw(run_command file_read_firstline trim dir_glob_regex dir_glob_foreach $IPV4RE $IPV6RE);
@@ -19,6 +22,7 @@ use base qw(PVE::Storage::Plugin);
 
 my $ISCSIADM = '/usr/bin/iscsiadm';
 our $SYSFS = '/sys';
+my $MULTIPATHD_EXECUTABLES = { '/usr/sbin/multipathd' => 1, '/sbin/multipathd' => 1 };
 
 my $found_iscsi_adm_exe;
 my sub assert_iscsi_support {
@@ -434,17 +438,32 @@ sub load_stable_scsi_paths {
     return $stable_paths;
 }
 
+# Strict callers die on unreadable sysfs directories instead of skipping them, so missing topology
+# cannot be mistaken for an absent device.
+my sub foreach_entry {
+    my ($strict, $dir, $regex, $code) = @_;
+
+    return dir_glob_foreach($dir, $regex, $code) if !$strict;
+    opendir(my $dh, $dir) or die "unable to inspect '$dir' - $!\n";
+    while (defined(my $entry = readdir($dh))) {
+        $code->($entry, $entry =~ m/^$regex$/) if $entry =~ m/^$regex$/;
+    }
+    closedir($dh) or die "unable to close '$dir' - $!\n";
+}
+
 # Offload initiators can run several sessions on one SCSI host, so the LUNs of a session are the
 # ones below its own target, not all LUNs of its host.
 my sub foreach_session_lun {
-    my ($sessiondir, $code) = @_;
+    my ($sessiondir, $code, $strict) = @_;
 
-    dir_glob_foreach(
+    foreach_entry(
+        $strict,
         $sessiondir,
         'target\d+:\d+:\d+',
         sub {
             my ($targetdir) = @_;
-            dir_glob_foreach(
+            foreach_entry(
+                $strict,
                 "$sessiondir/$targetdir",
                 '\d+:(\d+):(\d+):(\d+)',
                 sub {
@@ -456,27 +475,42 @@ my sub foreach_session_lun {
     );
 }
 
+# Destructive callers request strict probing so missing topology cannot be mistaken for a session
+# without devices.
 sub iscsi_session_device_map {
+    my ($strict) = @_;
 
     my $res = {};
 
     my $dirname = "$SYSFS/class/iscsi_session";
 
-    dir_glob_foreach(
+    foreach_entry(
+        $strict,
         $dirname,
         'session(\d+)',
         sub {
             my ($ent, $session) = @_;
 
             my $target = file_read_firstline("$dirname/$ent/targetname");
+            die "unable to identify iSCSI session $session - missing target\n"
+                if $strict && !$target;
             return if !$target;
 
+            my $device = abs_path("$dirname/$ent/device") // '';
+            my ($host) = $device =~ m{/host(\d+)/session\d+$};
+            die "unable to identify iSCSI session $session - missing host\n"
+                if $strict && !defined($host);
+            return if !defined($host);
+
             foreach_session_lun(
-                "$dirname/$ent/device",
+                $device,
                 sub {
                     my ($devdir, $channel, $id, $lun) = @_;
+                    my ($tmp) = $devdir =~ m{([^/]+)$};
 
                     my $type = file_read_firstline("$devdir/type");
+                    die "unable to identify SCSI device '$tmp' - missing type\n"
+                        if $strict && !defined($type);
                     return if !defined($type) || $type ne '0'; # list disks only
 
                     my $bdev;
@@ -485,23 +519,165 @@ sub iscsi_session_device_map {
                     } else {
                         (undef, $bdev) = dir_glob_regex($devdir, 'block:(\S+)');
                     }
+                    die "unable to identify SCSI device '$tmp' - missing block device\n"
+                        if $strict && !$bdev;
                     return if !$bdev;
 
                     $res->{$bdev} = {
                         target => $target,
+                        session => int($session),
+                        host => int($host),
                         channel => int($channel),
                         id => int($id),
                         lun => int($lun),
                     };
                 },
+                $strict,
             );
-
         },
     );
 
     return $res;
 }
 
+# block device names that end up in commands and device opens, untainted by their expected form
+my sub block_entries {
+    my ($dir) = @_;
+    opendir(my $dh, $dir) or die "unable to inspect '$dir' - $!\n";
+    my @entries = sort grep { $_ ne '.' && $_ ne '..' } readdir($dh);
+    closedir($dh) or die "unable to close '$dir' - $!\n";
+    return [map { m/^([\w.:+-]+)$/ ? $1 : die "unexpected entry '$_' in '$dir'\n" } @entries];
+}
+
+sub iscsi_claim_device {
+    my ($name, $number) = @_;
+
+    sysopen(my $fh, "/dev/$name", O_RDONLY | O_EXCL | O_NONBLOCK)
+        or die "unable to claim '/dev/$name' - $!\n";
+    die "device '/dev/$name' changed during logout\n"
+        if (PVE::Diskmanage::block_device_number($fh) // '') ne $number;
+    return $fh;
+}
+
+# Serialize path removals so concurrent logouts cannot each depend on the other's surviving path.
+# Storage ownership is not a user: an enabled storage can reconnect after a safe logout.
+sub iscsi_logout_guarded {
+    my ($sid) = @_;
+
+    my $result = PVE::Tools::lock_file(
+        '/run/lock/pve-iscsi-lifecycle.lock',
+        10,
+        sub {
+            my $session_dir = "$SYSFS/class/iscsi_session/session$sid";
+            my $session_ino = (CORE::stat($session_dir))[1]
+                // die "unable to identify iSCSI session $sid\n";
+            my $devices = iscsi_session_device_map(1);
+            my @departing = sort grep { $devices->{$_}->{session} == $sid } keys %$devices;
+            my %departing = map { $_ => 1 } @departing;
+            my (%claim, @users, @claims);
+            my $maps;
+            for my $name (@departing) {
+                my $dir = "$SYSFS/class/block/$name";
+                my @holders = block_entries("$dir/holders")->@*;
+                my @maps;
+                for my $holder (@holders) {
+                    next if $holder !~ m/^dm-\d+$/;
+                    my $uuid = file_read_firstline("$SYSFS/class/block/$holder/dm/uuid")
+                        // die "unable to identify holder '$holder'\n";
+                    push @maps, [$holder, $1] if $uuid =~ m/^mpath-(\S+)$/;
+                }
+                die "device '$name' belongs to multiple multipath maps\n" if @maps > 1;
+                my $ignore = {};
+                if (@maps) {
+                    my ($map_name, $wwid) = $maps[0]->@*;
+                    $ignore->{$map_name} = 1;
+                    $maps //= PVE::Multipath::get_maps();
+                    my ($map) =
+                        grep { ($_->{sysfs} // '') eq $map_name && $_->{wwid} eq $wwid } @$maps;
+                    die "unable to inspect multipath map '$map_name'\n" if !$map;
+                    my %slaves =
+                        map { $_ => 1 } block_entries("$SYSFS/class/block/$map_name/slaves")->@*;
+                    # Another path counts if device-mapper uses it and its checker last found it
+                    # usable; an unverified or failing checker state is no evidence of survival.
+                    my $survives = 0;
+                    for my $path (map { $_->{paths}->@* } $map->{'path-groups'}->@*) {
+                        my $other = $path->{dev};
+                        next
+                            if !$slaves{$other}
+                            || $departing{$other}
+                            || ($path->{'dm-state'} // '') ne 'active'
+                            || ($path->{'check-state'} // '') !~ m/^(?:ready|ghost)$/;
+                        if (my $session = $devices->{$other}) {
+                            my $state = file_read_firstline(
+                                "$SYSFS/class/iscsi_session/session$session->{session}/state");
+                            $survives = 1 if defined($state) && $state eq 'LOGGED_IN';
+                        } else {
+                            my $link = abs_path("$SYSFS/class/block/$other/device") // '';
+                            my $state =
+                                file_read_firstline("$SYSFS/class/block/$other/device/state") // '';
+                            $survives = 1 if $link =~ m{/rport-} && $state eq 'running';
+                        }
+                    }
+                    if (!$survives && !$claim{$map_name}) {
+                        my $open = '';
+                        run_command(
+                            [
+                                'dmsetup',
+                                'info',
+                                '-c',
+                                '--noheadings',
+                                '-o',
+                                'open',
+                                "/dev/$map_name",
+                            ],
+                            timeout => 10,
+                            outfunc => sub { $open .= "$_[0]\n" },
+                        );
+                        die "unable to inspect map '$map_name' - malformed open count\n"
+                            if $open !~ m/^\s*(\d+)\s*$/;
+                        $open = $1;
+                        push @users,
+                            "$map_name has $open open " . ($open == 1 ? 'reference' : 'references')
+                            if $open > 0;
+                        push @users, PVE::Diskmanage::block_device_users([$map_name])->@*;
+                        $claim{$map_name} = 1;
+                    }
+                } else {
+                    $claim{$name} = 1;
+                }
+                # multipathd's checkers keep every path of a map open, which does not count as use
+                my $daemon = @maps ? $MULTIPATHD_EXECUTABLES : {};
+                push @users,
+                    PVE::Diskmanage::block_device_users(
+                        [$name],
+                        { 'ignore-holders' => $ignore, 'ignore-executables' => $daemon },
+                    )->@*;
+            }
+            die
+                "cannot log out of iSCSI session '$sid' - a device is used directly or a LUN in use"
+                . " would lose its last usable path: "
+                . join(', ', @users) . "\n"
+                if @users;
+            for my $name (sort keys %claim) {
+                my $number = file_read_firstline("$SYSFS/class/block/$name/dev");
+                die "unable to identify device '$name'\n"
+                    if !defined($number) || $number !~ m/^\d+:\d+$/;
+                push @claims, iscsi_claim_device($name, $number);
+            }
+            die "iSCSI session $sid changed during logout\n"
+                if ((CORE::stat($session_dir))[1] // '') ne $session_ino;
+            run_command(
+                [$ISCSIADM, '--mode', 'session', '--sid', $sid, '--logout'],
+                errmsg => "logout of iSCSI session $sid failed",
+                outfunc => sub { },
+            );
+            return;
+        },
+    );
+    die $@ if $@;
+    return $result;
+}
+
 sub iscsi_device_list {
 
     my $res = {};
diff --git a/src/test/Makefile b/src/test/Makefile
index 9962bdfd..3517db6e 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -55,7 +55,8 @@ test_multipath: run_multipath_tests.pl
 test_san_api: run_san_api_tests.pl
 	./run_san_api_tests.pl
 
-test_iscsi: run_iscsi_discovery_tests.pl run_iscsi_rescan_tests.pl run_iscsi_device_list_tests.pl run_iscsi_session_tests.pl run_iscsi_volume_tests.pl
+test_iscsi: run_iscsi_logout_tests.pl run_iscsi_discovery_tests.pl run_iscsi_rescan_tests.pl run_iscsi_device_list_tests.pl run_iscsi_session_tests.pl run_iscsi_volume_tests.pl
+	./run_iscsi_logout_tests.pl
 	./run_iscsi_discovery_tests.pl
 	./run_iscsi_rescan_tests.pl
 	./run_iscsi_device_list_tests.pl
diff --git a/src/test/run_block_device_users_tests.pl b/src/test/run_block_device_users_tests.pl
index 5a27b503..1c040f60 100755
--- a/src/test/run_block_device_users_tests.pl
+++ b/src/test/run_block_device_users_tests.pl
@@ -72,22 +72,49 @@ $numbers{"$root/proc/999999/fd/3"} = '8:1';
 write_file('/proc/999999/comm', "qemu\n");
 is_deeply(
     PVE::Diskmanage::block_device_users(
-        ['sda'], $options,
+        ['sda'],
+        { %$options, 'ignore-holders' => { 'dm-0' => 1 } },
     ),
-    ['sda held by dm-0', 'qemu (PID 999999) has sda1 open'],
-    'holders and direct users of a member partition are both listed',
+    ['qemu (PID 999999) has sda1 open'],
+    'ignoring a map does not hide direct users of a member partition',
 );
 unlink("$root/proc/999999/fd/3") or die $!;
 symlink('/srv/raw-disk', "$root/proc/999999/fd/3") or die $!;
 is_deeply(
     PVE::Diskmanage::block_device_users(
-        ['sda'], $options,
+        ['sda'],
+        { %$options, 'ignore-holders' => { 'dm-0' => 1 } },
     ),
-    ['sda held by dm-0', 'qemu (PID 999999) has sda1 open'],
+    ['qemu (PID 999999) has sda1 open'],
     'device node outside /dev is not silently skipped',
 );
 unlink("$root/proc/999999/fd/3") or die $!;
 
+symlink('/dev/alias', "$root/proc/999999/fd/3") or die $!;
+symlink('/usr/sbin/multipathd', "$root/proc/999999/exe") or die $!;
+write_file('/proc/999999/comm', "multipathd\n");
+is_deeply(
+    PVE::Diskmanage::block_device_users(
+        ['sda'],
+        {
+            %$options,
+            'ignore-holders' => { 'dm-0' => 1 },
+            'ignore-executables' => { '/usr/sbin/multipathd' => 1 },
+        },
+    ),
+    [],
+    'an ignored executable holding a member open is no user',
+);
+is_deeply(
+    PVE::Diskmanage::block_device_users(
+        ['sda'],
+        { %$options, 'ignore-holders' => { 'dm-0' => 1 } },
+    ),
+    ['multipathd (PID 999999) has sda1 open'],
+    'other callers still see the executable',
+);
+unlink("$root/proc/999999/fd/3", "$root/proc/999999/exe") or die $!;
+
 write_file('/sys/class/block/loop0/loop/backing_file', "/dev/alias\n");
 write_file('/sys/kernel/config/nvmet/subsystems/test/namespaces/1/device_path', "/dev/alias\n");
 write_file('/sys/kernel/config/target/core/iblock_0/test/udev_path', "/dev/alias\n");
diff --git a/src/test/run_iscsi_logout_tests.pl b/src/test/run_iscsi_logout_tests.pl
new file mode 100755
index 00000000..b88e5740
--- /dev/null
+++ b/src/test/run_iscsi_logout_tests.pl
@@ -0,0 +1,271 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use File::Path qw(make_path);
+use File::Temp qw(tempdir);
+use Scalar::Util qw(weaken);
+use Test::MockModule;
+use Test::More;
+
+use lib ('.', '..');
+use PVE::Storage::ISCSIPlugin;
+
+my $root = tempdir(CLEANUP => 1);
+local $PVE::Storage::ISCSIPlugin::SYSFS = "$root/sys";
+my sub put {
+    my ($path, $value) = @_;
+    my ($dir) = "$root$path" =~ m{^(.*)/[^/]+$};
+    make_path($dir);
+    open(my $fh, '>', "$root$path") or die $!;
+    print $fh "$value\n";
+    close($fh) or die $!;
+}
+# The sysfs layout of the kernel's iSCSI transport class: the SCSI target and its LUNs are children
+# of the session device, and the SCSI bus lists the LUNs by H:C:T:L.
+my sub session {
+    my ($sid, $host, $target, $hctl, $dev, $number) = @_;
+    put("/sys/class/iscsi_session/session$sid/targetname", $target);
+    put("/sys/class/iscsi_session/session$sid/state", 'LOGGED_IN');
+    my $sessiondir = "/sys/devices/host$host/session$sid";
+    make_path("$root$sessiondir");
+    symlink("$root$sessiondir", "$root/sys/class/iscsi_session/session$sid/device") or die $!;
+    my ($tdir) = $hctl =~ m/^(\d+:\d+:\d+):\d+$/;
+    my $lundir = "$sessiondir/target$tdir/$hctl";
+    put("$lundir/type", '0');
+    make_path("$root$lundir/block/$dev", "$root/sys/bus/scsi/devices");
+    symlink("$root$lundir", "$root/sys/bus/scsi/devices/$hctl") or die $!;
+    put("/sys/class/block/$dev/dev", $number);
+    make_path("$root/sys/class/block/$dev/holders");
+}
+session(1, 1, 'iqn.same', '1:0:0:1', 'sda', '8:0');
+session(2, 2, 'iqn.same', '2:0:0:1', 'sdb', '8:16');
+put('/sys/class/block/dm-0/dev', '253:0');
+put('/sys/class/block/dm-0/dm/uuid', 'mpath-test');
+make_path("$root/sys/class/block/dm-0/slaves/sda", "$root/sys/class/block/dm-0/slaves/sdb");
+symlink('../../dm-0', "$root/sys/class/block/sda/holders/dm-0") or die $!;
+symlink('../../dm-0', "$root/sys/class/block/sdb/holders/dm-0") or die $!;
+my $devices = PVE::Storage::ISCSIPlugin::iscsi_session_device_map(1);
+is($devices->{sda}->{session}, 1, 'first session identified');
+is($devices->{sdb}->{session}, 2, 'same target on another session distinguished');
+is($devices->{sdb}->{host}, 2, 'host identified');
+
+my $plugin = Test::MockModule->new('PVE::Storage::ISCSIPlugin');
+my $disk = Test::MockModule->new('PVE::Diskmanage');
+my $mp = Test::MockModule->new('PVE::Multipath');
+my $tools = Test::MockModule->new('PVE::Tools');
+my (
+    $locked,
+    $dm_active,
+    $check_state,
+    $fc_state,
+    $opens,
+    $map_users,
+    $direct_users,
+    $fail_probe,
+    $fail_claim,
+);
+my (@checks, @logouts, %claims);
+$tools->mock(
+    lock_file => sub {
+        my ($path, $timeout, $code) = @_;
+        like($path, qr/iscsi-lifecycle/, 'node-wide lifecycle lock');
+        die "overlapping logout\n" if $locked;
+        $locked = 1;
+        my $res = eval { $code->() };
+        my $err = $@;
+        $locked = 0;
+        die $err if $err;
+        return $res;
+    },
+);
+$mp->mock(
+    get_maps => sub {
+        ok($locked, 'map topology under lock');
+        return [{
+            sysfs => 'dm-0',
+            wwid => 'test',
+            'path-groups' => [{
+                paths => [
+                    { dev => 'sda', 'dm-state' => 'active', 'check-state' => 'ready' },
+                    {
+                        dev => 'sdb',
+                        'dm-state' => $dm_active ? 'active' : 'failed',
+                        defined($check_state) ? ('check-state' => $check_state) : (),
+                    },
+                    defined($fc_state)
+                    ? ({ dev => 'sdf', 'dm-state' => 'active', 'check-state' => $fc_state })
+                    : (),
+                ],
+            }],
+        }];
+    },
+);
+$disk->mock(
+    block_device_users => sub {
+        my ($names, $options) = @_;
+        ok($locked, 'user check under lock');
+        my $name = $names->[0];
+        push @checks, $name;
+        die "user probe failed\n" if $fail_probe;
+        if ($name eq 'sda' && -e "$root/sys/class/block/sda/holders/dm-0") {
+            is_deeply($options->{'ignore-holders'}, { 'dm-0' => 1 },
+                'only map holder excluded');
+            ok(
+                $options->{'ignore-executables'}->{'/usr/sbin/multipathd'},
+                'multipathd is no user',
+            );
+        }
+        return $name eq 'dm-0' ? $map_users : $direct_users;
+    },
+);
+$plugin->mock(
+    run_command => sub {
+        my ($cmd, %opt) = @_;
+        ok($locked, 'commands under lock');
+        if ($cmd->[0] eq 'dmsetup') {
+            $opt{outfunc}->($opens);
+        } else {
+            push @logouts, $cmd;
+            ok(defined(fileno($claims{$_})), 'exclusive claim held during logout')
+                for keys %claims;
+        }
+    },
+    iscsi_claim_device => sub {
+        my ($name, $number) = @_;
+        ok($locked, 'claim under lock');
+        die "claim failed\n" if $fail_claim;
+        open(my $fh, '<', "$root/sys/class/block/$name/dev") or die $!;
+        $claims{$name} = $fh;
+        weaken($claims{$name});
+        return $fh;
+    },
+);
+my sub reset_case {
+    (
+        $dm_active, $check_state, $fc_state, $opens, $map_users, $direct_users, $fail_probe,
+        $fail_claim,
+    ) = (1, 'ready', undef, 0, [], [], 0, 0);
+    @checks = ();
+    @logouts = ();
+    %claims = ();
+    put('/sys/class/iscsi_session/session2/state', 'LOGGED_IN');
+}
+my sub logout {
+    my ($sid) = @_;
+    eval { PVE::Storage::ISCSIPlugin::iscsi_logout_guarded($sid // 1) };
+    return $@;
+}
+reset_case();
+$map_users = ['guest uses map'];
+is(logout(), '', 'in-use map retains another usable path');
+is_deeply(\@checks, ['sda'], 'departing member checked, not surviving map');
+is(scalar(@logouts), 1, 'logout executed');
+reset_case();
+($check_state, $map_users) = ('ghost', ['guest uses map']);
+is(logout(), '', 'an ALUA standby path is a usable alternate');
+
+# a running Fibre Channel path of the map is an alternate as well, by the same checker states
+my $rport = "$root/sys/devices/host5/rport-5:0-0/target5:0:0/5:0:0:1";
+make_path($rport, "$root/sys/class/block/sdf", "$root/sys/class/block/dm-0/slaves/sdf");
+put('/sys/devices/host5/rport-5:0-0/target5:0:0/5:0:0:1/state', 'running');
+symlink($rport, "$root/sys/class/block/sdf/device") or die $!;
+for my $case (['ready', 1], ['ghost', 1], ['faulty', 0], ['shaky', 0]) {
+    my ($state, $usable) = @$case;
+    reset_case();
+    ($dm_active, $fc_state, $map_users) = (0, $state, ['guest uses map']);
+    if ($usable) {
+        is(logout(), '', "a Fibre Channel alternate with checker state $state is usable");
+    } else {
+        like(logout(), qr/last usable path/, "a Fibre Channel alternate in state $state is not");
+        is(scalar(@logouts), 0, 'no logout on refusal');
+    }
+}
+rmdir("$root/sys/class/block/dm-0/slaves/sdf") or die $!;
+
+for my $case (
+    [
+        'FAILED alternate session',
+        sub {
+            put('/sys/class/iscsi_session/session2/state', 'FAILED');
+            $map_users = ['guest uses map'];
+        },
+    ],
+    ['failed dm path', sub { $dm_active = 0; $map_users = ['guest uses map'] }],
+    (
+        map {
+            my $state = $_;
+            [
+                'alternate path with checker state ' . ($state // 'missing'),
+                sub { ($check_state, $map_users) = ($state, ['guest uses map']) },
+            ]
+        } ('faulty', 'shaky', 'i/o pending', undef)
+    ),
+    ['dm opens', sub { $dm_active = 0; $opens = 1 }],
+    ['direct member user', sub { $direct_users = ['qemu has sda open'] }],
+    ['users probe failure', sub { $fail_probe = 1 }],
+    ['malformed open count', sub { $dm_active = 0; $opens = 'bad' }],
+    ['claim failure', sub { $dm_active = 0; $fail_claim = 1 }],
+) {
+    reset_case();
+    $case->[1]->();
+    ok(logout(), "$case->[0] refused");
+    is(scalar(@logouts), 0, 'no logout on refusal');
+}
+for my $count (1, 2) {
+    reset_case();
+    ($dm_active, $opens) = (0, $count);
+    my $reference = $count == 1 ? 'reference' : 'references';
+    like(logout(), qr/dm-0 has $count open $reference\n$/,
+        "open count $count uses correct grammar");
+    is(scalar(@logouts), 0, 'open references still prevent logout');
+}
+reset_case();
+$dm_active = 0;
+is(logout(), '', 'unused last-path map can be removed');
+is_deeply([sort keys %claims], ['dm-0'], 'map claimed');
+ok(!defined($claims{'dm-0'}), 'claim released after logout');
+
+unlink("$root/sys/class/block/sda/holders/dm-0") or die $!;
+reset_case();
+is(logout(), '', 'unused non-multipath disk can be removed');
+is_deeply([sort keys %claims], ['sda'], 'non-multipath disk claimed');
+reset_case();
+$direct_users = ['sda1 mounted'];
+like(logout(), qr/last usable path.*sda1 mounted/, 'partition user refuses non-multipath logout');
+
+# Offload initiators can run several sessions on one SCSI host; each LUN still belongs only to the
+# session whose target holds it.
+session(3, 9, 'iqn.other-a', '9:0:0:1', 'sdc', '8:32');
+session(4, 9, 'iqn.other-b', '9:0:1:1', 'sdd', '8:48');
+$devices = PVE::Storage::ISCSIPlugin::iscsi_session_device_map(1);
+is_deeply(
+    { map { $_ => [$devices->{$_}->@{qw(session target)}] } qw(sdc sdd) },
+    { sdc => [3, 'iqn.other-a'], sdd => [4, 'iqn.other-b'] },
+    'sessions sharing a SCSI host keep their own LUNs',
+);
+reset_case();
+$direct_users = ['qemu has sdc open'];
+like(logout(3), qr/last usable path.*qemu has sdc open/, 'user of a shared-host session refuses');
+is_deeply(\@checks, ['sdc'], 'the LUN of the shared-host session was checked');
+is(scalar(@logouts), 0, 'no logout of a shared-host session in use');
+
+reset_case();
+unlink("$root/sys/class/iscsi_session/session4/device") or die $!;
+like(logout(3), qr/session 4 - missing host/, 'a session without its device link fails closed');
+is(scalar(@logouts), 0, 'no logout with an unknown session topology');
+symlink("$root/sys/devices/host9/session4", "$root/sys/class/iscsi_session/session4/device")
+    or die $!;
+
+reset_case();
+rmdir("$root/sys/devices/host9/session4/target9:0:1/9:0:1:1/block/sdd") or die $!;
+like(logout(3), qr/missing block device/, 'incomplete topology of any session fails closed');
+make_path("$root/sys/devices/host9/session4/target9:0:1/9:0:1:1/block/sdd");
+
+unlink("$root/sys/class/iscsi_session/session1/targetname") or die $!;
+reset_case();
+like(logout(), qr/missing target/, 'missing identity fails closed');
+is(scalar(@logouts), 0, 'no logout without identity');
+
+done_testing();
diff --git a/src/test/run_san_api_tests.pl b/src/test/run_san_api_tests.pl
index 00d719a0..cf382a11 100755
--- a/src/test/run_san_api_tests.pl
+++ b/src/test/run_san_api_tests.pl
@@ -43,7 +43,7 @@ subtest 'node SAN read permissions' => sub {
         }
     }
     for my $case (
-        ['PVE::API2::ISCSI', qw(target_delete rescan)],
+        ['PVE::API2::ISCSI', qw(session_logout target_delete rescan)],
     ) {
         my ($class, @methods) = @$case;
         for my $name (@methods) {
-- 
2.47.3




  parent reply	other threads:[~2026-11-06  0:55 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-05 23:30 [PATCH v3 proxmox,proxmox-perl-rs,cluster,storage,docs,manager 00/76] SAN: cluster-wide multipath, SAN views and a guided storage setup Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 proxmox 01/76] add proxmox-multipath crate Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 proxmox-perl-rs 02/76] pve-rs: add PVE::RS::Multipath bindings for multipathd status parsing Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 cluster 03/76] cfs: observe the multipath configuration files Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 04/76] storage: distinguish status-only activation in plugins Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 05/76] lvm: find a shared volume group created by another node Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 06/76] storage: bound LVM status commands Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 07/76] iscsi: rate-limit session rescans per target Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 08/76] iscsi: list the LUNs of each session, not of its SCSI host Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 09/76] lvm: refuse to create a volume group on a device that holds data Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 10/76] lvm: refuse a base volume of a target that serves disk images Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 11/76] iscsi: refuse virtual devices other than multipath maps as volumes Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 12/76] diskmanage: collect disk transport type from lsblk Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 13/76] diskmanage: add helper to list multipath devices Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 14/76] diskmanage: qualify NVMe over fabrics transport Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 15/76] disks: list: add include-remote parameter Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 16/76] diskmanage: include iSCSI session devices in disk enumeration Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 17/76] diskmanage: link multipath member disks to their map device Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 18/76] iscsi: factor out session device map from device list Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 19/76] api: scan: add a SAN LUN inventory Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 20/76] pvesm: expose the " Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 21/76] disks: lvm: allow volume groups on SAN LUNs and multipath maps Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 22/76] multipath: add a library around multipathd Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 23/76] diskmanage: collect native NVMe multipath path state Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 24/76] api: scan: san-luns: report " Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 25/76] diskmanage: collect local block device users Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 26/76] api: add node-level iSCSI status and maintenance Thomas Lamprecht
2026-11-05 23:31 ` Thomas Lamprecht [this message]
2026-11-05 23:31 ` [PATCH v3 storage 28/76] multipath: add a cluster-wide configuration Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 29/76] iscsi: add an option to manage storage LUNs with multipath Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 30/76] lvm: activate a missing base volume before using the volume group Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 31/76] multipath: apply the configuration on every node Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 32/76] iscsi: share the initiator identity reader Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 33/76] multipath: publish per-node status Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 34/76] api: add endpoints for the cluster-wide multipath configuration Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 35/76] api: multipath: add the cluster-wide health status Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 36/76] multipath: collect identities of non-iSCSI SCSI paths Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 37/76] api: scan: san-luns: report the multipath state of each LUN Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 38/76] api: scan: flag duplicate physical volumes on distinct LUNs Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 39/76] api: disks: recheck local users and retain wipe device references Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 40/76] iscsi: name plain LUNs after their identity link Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 41/76] api: disks: guard SAN wipes with stable identities and storage checks Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 42/76] api: scan: refresh SAN usage with bounded probes Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 docs 43/76] storage: lvm: note the rescan after extending a shared volume group Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 docs 44/76] storage: document the cluster-wide multipath management Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 45/76] api: ceph: osd: keep iSCSI LUNs out of OSD creation Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 46/76] pvestatd: track pending storage status commands across re-exec Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 47/76] ui: storage: factor out the scan picker error mask Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 48/76] ui: storage: nfs: show scan errors and empty results Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 49/76] ui: storage: allow changing the node for NFS/CIFS scans Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 50/76] ui: storage: add guided remote storage wizard with NFS support Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 51/76] ui: storage wizard: add SMB/CIFS support Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 52/76] ui: storage wizard: preserve settings after partial setup Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 53/76] ui: storage wizard: add Fibre Channel shared LVM setup Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 54/76] ui: storage wizard: reconcile unanswered storage creation Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 55/76] ui: storage wizard: add ZFS over iSCSI support Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 56/76] ui: dc: storage: add remote storage wizard entry to the add menu Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 57/76] ui: node: add SAN LUNs panel Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 58/76] ui: san luns: show multipath path state Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 59/76] api: nodes: expose iSCSI status and maintenance Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 60/76] pvenode: add iscsi commands Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 61/76] ui: node: san: add iSCSI target and session management Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 62/76] pvestatd: apply the multipath configuration and report the node status Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 63/76] api: cluster: add the multipath endpoints Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 64/76] ui: dc: add a cluster-wide multipath view Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 65/76] ui: storage: iscsi: add the multipath option Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 66/76] ui: san luns: show multipath state and manage LUN entries Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 67/76] ui: storage wizard: add iSCSI support Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 68/76] ui: storage wizard: iscsi: show cluster initiator names Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 69/76] ui: storage wizard: iscsi: enable multipath for shared LVM Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 70/76] ui: node: san: wipe a LUN that no storage uses Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 71/76] ui: storage wizard: iscsi: reuse existing target connections Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 72/76] ui: storage wizard: fibre channel: enable multipath for a LUN Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 73/76] ui: storage wizard: let root@pam wipe a LUN with data Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 74/76] ui: storage wizard: do not offer LUNs in use for a new volume group Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 75/76] ui: san: show the local users of a LUN in its usage tooltip Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 76/76] ui: san: check the other nodes for users before wiping a LUN Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261105233336.2178064-28-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal