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 96FE01FF0AF for ; Fri, 06 Nov 2026 01:54:00 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 56716219EA; Fri, 06 Nov 2026 01:51:08 +0100 (CET) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [PATCH v3 storage 40/76] iscsi: name plain LUNs after their identity link Date: Fri, 6 Nov 2026 00:31:15 +0100 Message-ID: <20261105233336.2178064-41-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: 1793926180161 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.320 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: 72X2ITTZHG2FDAJZEYWHBQWX2JLU2MY3 X-Message-ID-Hash: 72X2ITTZHG2FDAJZEYWHBQWX2JLU2MY3 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: Storages without the multipath option name each LUN after the last of its by-id links. udev can add aliases, such as SCSI name strings, that sort after scsi- but do not encode the LUN identity, so the volume name alone does not tell which LUN it denotes. Use the scsi- link for such LUNs instead, so operations on the volume can verify the LUN. The identities are only collected when a LUN has such aliases. Existing references to an alias name still resolve. Signed-off-by: Thomas Lamprecht --- src/PVE/Storage/ISCSIPlugin.pm | 15 ++++-- src/test/run_iscsi_device_list_tests.pl | 2 +- src/test/run_lun_identity_tests.pl | 70 +++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm index ad3afaf6..cadbda8a 100644 --- a/src/PVE/Storage/ISCSIPlugin.pm +++ b/src/PVE/Storage/ISCSIPlugin.pm @@ -26,6 +26,7 @@ use base qw(PVE::Storage::Plugin); my $ISCSIADM = '/usr/bin/iscsiadm'; our $INITIATOR_NAME_FILE = '/etc/iscsi/initiatorname.iscsi'; our $SYSFS = '/sys'; +our $BY_ID_DIR = '/dev/disk/by-id'; my $MULTIPATHD_EXECUTABLES = { '/usr/sbin/multipathd' => 1, '/sbin/multipathd' => 1 }; sub read_initiator_info { @@ -437,7 +438,7 @@ sub load_stable_scsi_paths { my $stable_paths = {}; - my $stabledir = "/dev/disk/by-id"; + my $stabledir = $BY_ID_DIR; if (my $dh = IO::Dir->new($stabledir)) { foreach my $tmp (sort $dh->read) { @@ -448,7 +449,7 @@ sub load_stable_scsi_paths { my $path = "$stabledir/$tmp"; my $bdevdest = readlink($path); if ($bdevdest && $bdevdest =~ m|^../../([^/]+)|) { - $stable_paths->{$1} = $tmp; + push $stable_paths->{$1}->@*, $tmp; } } } @@ -721,7 +722,8 @@ sub iscsi_device_list { $bdev = $multipathdev if $multipathdev; } - my $blockdev = $stable_paths->{$bdev}; + my $names = $stable_paths->{$bdev} // []; + my $blockdev = $names->[-1]; # scsi- can name a member before assembly; dm-uuid-mpath- only names the map. # Managed volumes keep the map name even while assembly is pending. if ($map_names) { @@ -730,6 +732,13 @@ sub iscsi_device_list { ? PVE::Multipath::Identity::path_wwid($ids, $path_dev) : undef; $blockdev = defined($wwid) ? "dm-uuid-mpath-$wwid" : undef; + } elsif ((grep { m/^scsi-/ } @$names) > 1) { + # udev can add aliases, such as SCSI name strings, that sort after scsi- but do + # not encode the LUN identity; prefer the name that does, so operations on the volume + # can verify which LUN it denotes. + $ids //= PVE::Multipath::Identity::collect(all => 1); + my $wwid = PVE::Multipath::Identity::path_wwid($ids, $path_dev); + $blockdev = "scsi-$wwid" if defined($wwid) && grep { $_ eq "scsi-$wwid" } @$names; } next if !$blockdev; diff --git a/src/test/run_iscsi_device_list_tests.pl b/src/test/run_iscsi_device_list_tests.pl index 6df3a416..00ce2684 100755 --- a/src/test/run_iscsi_device_list_tests.pl +++ b/src/test/run_iscsi_device_list_tests.pl @@ -55,7 +55,7 @@ session(4, 9, 'iqn.target-d'); my $plugin = Test::MockModule->new('PVE::Storage::ISCSIPlugin'); $plugin->mock( load_stable_scsi_paths => sub { - return { map { $_ => "scsi-3$_" } qw(sda sdb sdc sdd) }; + return { map { $_ => ["scsi-3$_"] } qw(sda sdb sdc sdd) }; }, ); diff --git a/src/test/run_lun_identity_tests.pl b/src/test/run_lun_identity_tests.pl index c5363e46..a49f27c4 100755 --- a/src/test/run_lun_identity_tests.pl +++ b/src/test/run_lun_identity_tests.pl @@ -597,6 +597,76 @@ is_deeply( 'volumes: every path of a resolved LUN named after its map, an unresolved LUN left out', ); +{ + # Without the option, LUNs are named after their by-id links, here in a scratch directory: + # { device => [lun, wwid, links] } + my $fixture = { + sdzzb => [0, '3W0', ['scsi-3W0', 'scsi-3W0-part1', 'scsi-8naa.W0']], + sdzzc => [1, '2E1', ['scsi-2E1', 'scsi-8eui.E1']], + sdzzd => [2, '3W2', ['scsi-3W2', 'scsi-SLIO-ORG_disk_W2']], + sdzze => [3, '3W3', ['scsi-3W3']], + sdzzf => [4, undef, ['scsi-3W4', 'scsi-8W4']], + sdzzg => [5, '3W5', ['scsi-8W5', 'scsi-SW5']], + }; + my ($disks, $lines) = ({}, {}); + for my $name (keys %$fixture) { + my ($lun, $wwid) = $fixture->{$name}->@*; + my $disk = { target => $T, lun => $lun, devt => "8:$lun", hctl => "9:0:0:$lun" }; + $disks->{$name} = $disk; + $lines->{$name} = { $disk->%{qw(devt hctl target)}, init => 'ok', wwid => $wwid } + if defined($wwid); + } + my $plain = { luns => PVE::Multipath::Identity::resolve($disks, $lines, $naa_ok) }; + my $by_id = tempdir(CLEANUP => 1); + my sub link_devices { + unlink(glob("$by_id/*")); + for my $name (@_) { + symlink("../../$name", "$by_id/$_") or die "$!\n" for $fixture->{$name}->[2]->@*; + } + } + local $PVE::Storage::ISCSIPlugin::BY_ID_DIR = $by_id; + my $collected = 0; + my $session_device_map = \&PVE::Storage::ISCSIPlugin::iscsi_session_device_map; + $identity->mock(collect => sub { $collected++; $plain }); + $iscsi->unmock('load_stable_scsi_paths'); + $iscsi->mock( + iscsi_session_device_map => sub { + return { + map { + $_ => { target => $T, channel => 0, id => 0, lun => $disks->{$_}->{lun} } + } + keys %$disks + }; + }, + ); + my $list = sub { [sort keys PVE::Storage::ISCSIPlugin::iscsi_device_list(0, $T)->{$T}->%*] }; + link_devices(keys %$fixture); + my $listed = $list->(); + is_deeply( + $listed, + [ + '0.0.0.scsi-3W0', + '0.0.1.scsi-2E1', + '0.0.2.scsi-3W2', + '0.0.3.scsi-3W3', + '0.0.4.scsi-8W4', + '0.0.5.scsi-SW5', + ], + 'plain volumes: the identity name over aliases, else the last name as before', + ); + is($collected, 1, 'plain volumes: identities collected once'); + link_devices('sdzze'); + $collected = 0; + is_deeply($list->(), ['0.0.3.scsi-3W3'], 'plain volumes: a LUN without aliases'); + is($collected, 0, 'plain volumes: needs no identities'); + + $iscsi->mock( + iscsi_session_device_map => $session_device_map, + load_stable_scsi_paths => sub { {} }, + ); + $identity->mock(collect => sub { $ids }); +} + my $collected = 0; $identity->mock(collect => sub { $collected++; return { luns => {} } }); my $derived = PVE::Storage::derived_multipath_wwids({ -- 2.47.3