all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal