From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 893B61FF0AF for ; Fri, 06 Nov 2026 01:55:40 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 35A0421B6C; Fri, 06 Nov 2026 01:51:13 +0100 (CET) From: Thomas Lamprecht 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 Message-ID: <20261105233336.2178064-28-t.lamprecht@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261105233336.2178064-1-t.lamprecht@proxmox.com> References: <20261105233336.2178064-1-t.lamprecht@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793926179028 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.311 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: QEUIXLQX6PYSQ7DZ23SLJR3OMN6RZ6HN X-Message-ID-Hash: QEUIXLQX6PYSQ7DZ23SLJR3OMN6RZ6HN X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 Signed-off-by: Thomas Lamprecht --- 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