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 4AB331FF0AF for ; Fri, 06 Nov 2026 01:58:36 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 3B44D21E21; Fri, 06 Nov 2026 01:51:21 +0100 (CET) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [PATCH v3 storage 33/76] multipath: publish per-node status Date: Fri, 6 Nov 2026 00:31:08 +0100 Message-ID: <20261105233336.2178064-34-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: 1793926179563 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.299 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: CJJE3KNZYOWCWIMQO22K7EKXLZTB4M2J X-Message-ID-Hash: CJJE3KNZYOWCWIMQO22K7EKXLZTB4M2J 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: Path health can differ between nodes. Publish each node's map health, path counts, tool and daemon availability, and configuration errors in the cluster key-value store. Include wanted LUNs even without maps, the number of unmanaged maps, and the count of unresolved LUN identities. For storage-derived LUNs, also report whether the target still presents them. Establish absence only after a session on every portal has finished scanning and every discovered LUN is identified. This distinguishes a withdrawn LUN from a map that has not assembled. Include the iSCSI initiator name for target access rules, omitting invalid names rather than truncating them. Resend unchanged reports once a minute so readers can detect stale reports. Nodes without wanted LUNs reuse a successful map listing for a minute. The store limits values to 32 KiB, enough for roughly 200 LUNs. Larger reports contain only counts per state, never a partial list that could make omitted LUNs look missing. Signed-off-by: Thomas Lamprecht --- src/PVE/Multipath/Generator.pm | 28 +++- src/PVE/Multipath/Identity.pm | 51 +++++- src/PVE/Multipath/Status.pm | 184 +++++++++++++++++++++ src/PVE/Storage.pm | 2 +- src/PVE/Storage/ISCSIPlugin.pm | 17 +- src/test/run_lun_identity_tests.pl | 237 +++++++++++++++++++++++++--- src/test/run_multipath_api_tests.pl | 151 ++++++++++++++++++ src/test/run_multipath_tests.pl | 114 ++++++++++++- 8 files changed, 742 insertions(+), 42 deletions(-) diff --git a/src/PVE/Multipath/Generator.pm b/src/PVE/Multipath/Generator.pm index b7aec227..5a5ad96c 100644 --- a/src/PVE/Multipath/Generator.pm +++ b/src/PVE/Multipath/Generator.pm @@ -93,20 +93,37 @@ sub wwid_records_differ($a, $b) { return $flat->($a) ne $flat->($b); } -# Storages can expose a LUN through different portal sets, so judge map health against the largest -# expected path count. +sub unresolved_lun_count($derived) { + my %luns = map { $_ => 1 } map { ($_->{unresolved} // [])->@* } values(($derived // {})->%*); + return scalar(keys %luns); +} + +# Omit 'derived' when no source finds the LUN and at least one could not scan: a failed scan cannot +# prove absence. Storages can expose a LUN through different portal sets, so judge map health +# against the largest expected path count. sub desired_wwid_info($desired, $derived) { my $wwids = {}; for my $wwid (keys %$desired) { my $info = {}; + my ($found, $unseen) = (0, 0); for my $source ($desired->{$wwid}->@*) { my ($storeid) = $source =~ m/^storage:(.+)$/ or next; push $info->{storages}->@*, $storeid; - my $luns = ($derived->{$storeid} // {})->{wwids} // {}; - my $expected = ($luns->{$wwid} // {})->{'paths-expected'}; + my $set = $derived->{$storeid} // {}; + my $lun = ($set->{wwids} // {})->{$wwid}; + if ($lun) { + $found = 1; + } elsif (!$set->{scanned}) { + $unseen = 1; + } + my $expected = ($lun // {})->{'paths-expected'}; $info->{'paths-expected'} = $expected if defined($expected) && $expected > ($info->{'paths-expected'} // 0); } + if ($info->{storages}) { + $info->{derived} = 1 if $found; + $info->{derived} = 0 if !$found && !$unseen; + } $wwids->{$wwid} = $info; } return $wwids; @@ -259,6 +276,8 @@ sub sync() { PVE::Storage::derived_multipath_wwids(PVE::Storage::config()); }; return { $state->%*, error => "reading the storage configuration failed - $@" } if $@; + my $unresolved = unresolved_lun_count($derived); + $state->{unresolved} = $unresolved if $unresolved; my (undef, undef, $desired) = plan_wwid_changes(\%explicit, $derived, {}, $record); @@ -287,6 +306,7 @@ sub sync() { return { wwids => $res->{wwids}, + $unresolved ? (unresolved => $unresolved) : (), defined($res->{error}) ? (error => $res->{error}) : (), }; } diff --git a/src/PVE/Multipath/Identity.pm b/src/PVE/Multipath/Identity.pm index 2a8d3d20..86141e98 100644 --- a/src/PVE/Multipath/Identity.pm +++ b/src/PVE/Multipath/Identity.pm @@ -96,18 +96,35 @@ sub resolve($disks, $daemon, $naa_ok) { return $luns; } -# The iSCSI sessions: { sid => { target, host } }. +# SCANNED means the initial LUN scan after login finished. my sub iscsi_sessions() { my $res = {}; dir_glob_foreach( "$SYSFS/class/iscsi_session", 'session(\d+)', sub($ent, $sid) { - my $target = sysfs_line("class/iscsi_session/$ent/targetname") // return; - my (undef, $host) = - dir_glob_regex("$SYSFS/class/iscsi_session/$ent/device", 'target(\d+):\d+:\d+'); - return if !defined($host); - $res->{$sid} = { target => $target, host => $host }; + my $dir = "class/iscsi_session/$ent"; + my $target = sysfs_line("$dir/targetname") // return; + # the SCSI host of the session, also when it has no target left, after its last LUN + # was removed + my $device = abs_path("$SYSFS/$dir/device") // return; + my ($host) = $device =~ m|/host(\d+)/session\d+$| or return; + my $session = { + target => $target, + host => $host, + state => sysfs_line("$dir/state") // '', + scan => sysfs_line("$dir/target_state") // '', + portal => '', + }; + my (undef, $conn) = dir_glob_regex("$SYSFS/$dir/device", '(connection\d+:\d+)'); + if (defined($conn)) { + my $cdir = "$dir/device/$conn/iscsi_connection/$conn"; + my $address = sysfs_line("$cdir/persistent_address") // ''; + my $port = sysfs_line("$cdir/persistent_port") // ''; + $address = "[$address]" if $address =~ m/:/; + $session->{portal} = "$address:$port"; + } + $res->{$sid} = $session; }, ); return $res; @@ -191,10 +208,32 @@ sub collect(%param) { return { luns => $luns, paths => $paths, + sessions => $sessions, + 'sessions-after' => iscsi_sessions(), defined($error) ? (error => $error) : (), }; } +# Whether every portal of $portals has a logged-in session to $target in $ids (see collect()) that +# had finished its LUN scan before the LUNs were collected and did not change until afterwards. +sub portals_covered($ids, $target, $portals) { + my ($before, $after) = $ids->@{qw(sessions sessions-after)}; + for my $portal (@$portals) { + my ($sid) = grep { + my $session = $before->{$_}; + $session->{target} eq $target + && $session->{portal} eq $portal + && $session->{state} eq 'LOGGED_IN' + && $session->{scan} eq 'SCANNED' + } sort keys %$before; + return 0 if !defined($sid) || !$after->{$sid}; + return 0 if grep { + $after->{$sid}->{$_} ne $before->{$sid}->{$_} + } keys $before->{$sid}->%*; + } + return scalar(@$portals) ? 1 : 0; +} + sub target_luns($ids, $target) { return [grep { $_->{target} eq $target } values $ids->{luns}->%*]; } diff --git a/src/PVE/Multipath/Status.pm b/src/PVE/Multipath/Status.pm index e3dccf08..cc8fe49d 100644 --- a/src/PVE/Multipath/Status.pm +++ b/src/PVE/Multipath/Status.pm @@ -6,7 +6,191 @@ use JSON; use PVE::Multipath; use PVE::Multipath::Config; +# Include wanted WWIDs without assembled maps so failed assembly remains visible. Expected path +# counts include paths the daemon no longer lists. +sub status_luns($wanted, $maps) { + my $luns = {}; + for my $wwid (keys %$wanted) { + my $info = $wanted->{$wwid}; + $luns->{$wwid} = { map { defined($info->{$_}) ? ($_ => $info->{$_}) : () } + qw(storages paths-expected derived kept) }; + } + + my $unmanaged = 0; + for my $map (($maps // [])->@*) { + my $wwid = $map->{wwid} // next; + my $lun = $luns->{$wwid}; + if (!$lun) { + $unmanaged++; + next; + } + + my ($active, $total) = ($map->{'paths-active'} // 0, $map->{'paths-total'} // 0); + my $expected = $lun->{'paths-expected'}; + $lun->{state} = + defined($expected) + ? PVE::Multipath::derive_health($active, $total, $expected) + : $map->{health}; + $lun->{'paths-active'} = $active; + $lun->{'paths-total'} = $total; + $lun->{transport} = $map->{transport} if defined($map->{transport}); + $lun->{size} = $map->{size} if defined($map->{size}); + } + return ($luns, $unmanaged); +} + +# The pmxcfs node KV store rejects values of 32 KiB and more. +my $KV_VALUE_LIMIT = 30 * 1024; +my $ERROR_LIMIT = 2048; my $KV_KEY = 'multipath'; +my $status_json = JSON->new->canonical->utf8; + +our $KV_REFRESH_SECONDS = 60; +our $KV_STALE_SECONDS = 3 * $KV_REFRESH_SECONDS; + +# Replace oversized details with a summary: a partial list is indistinguishable from missing LUNs. +sub encode_status($status, $limit = undef) { + $limit //= $KV_VALUE_LIMIT; + my $data = { $status->%* }; + for my $key (qw(apply-error report-error)) { + $data->{$key} = substr($data->{$key}, 0, $ERROR_LIMIT) . '...' + if defined($data->{$key}) && length($data->{$key}) > $ERROR_LIMIT; + } + my $encoded = $status_json->encode($data); + return $encoded if length($encoded) <= $limit; + + my $luns = delete($data->{luns}) // {}; + my $summary = { luns => scalar(keys %$luns) }; + $summary->{ $_->{state} // 'missing' }++ for values %$luns; + $data->{summary} = $summary; + return $status_json->encode($data); +} + +# Send on change, and unchanged once per refresh interval so readers can tell a current report from +# the last one of a node that stopped reporting. A failed send is retried on the next call. +my $last_kv; + +my sub update_node_kv($status) { + require PVE::Cluster; + + my $content = $status_json->encode($status); + my $now = time(); + return + if $last_kv + && $content eq $last_kv->{content} + && $now - $last_kv->{time} < $KV_REFRESH_SECONDS; + + eval { + # the cluster library only warns when the send fails + local $SIG{__WARN__} = sub($line) { die $line }; + PVE::Cluster::broadcast_node_kv($KV_KEY, encode_status({ $status->%*, time => $now })); + }; + if (my $err = $@) { + warn "multipath: broadcasting the status failed - $err"; + return; + } + $last_kv = { content => $content, time => $now }; +} + +# Unmanaged-map counts need no finer freshness than the broadcast refresh interval. +my $idle_maps; + +# Reject malformed or oversized initiator names rather than truncate identities used in target ACLs. +my sub valid_iscsi_name($name) { + return 0 if !defined($name) || length($name) > 223; + my $valid = $name =~ m/^(?: + iqn\.\d{4}-\d{2}\.[A-Za-z0-9][A-Za-z0-9._:-]* + | eui\.[0-9A-Fa-f]{16} + | naa\.[0-9A-Fa-f]{16}(?:[0-9A-Fa-f]{16})? + )$/x; + return $valid ? 1 : 0; +} + +# Publish even without tools: the report's presence also signals support for the storage option. +sub broadcast_status($apply) { + $apply //= {}; + my $wanted = $apply->{wwids} // {}; + my $status = { + supported => PVE::Multipath::is_supported() ? 1 : 0, + defined($apply->{error}) ? ('apply-error' => "$apply->{error}") : (), + $apply->{unresolved} ? ('unresolved-luns' => $apply->{unresolved}) : (), + }; + + require PVE::Storage::ISCSIPlugin; + my $initiator = eval { PVE::Storage::ISCSIPlugin::read_initiator_info() }; + $status->{'iscsi-initiator-name'} = $initiator->{name} + if $initiator && valid_iscsi_name($initiator->{name}); + + my $maps = []; + if ($status->{supported}) { + $status->{running} = PVE::Multipath::is_running() ? 1 : 0; + if ($status->{running}) { + if (!%$wanted && $idle_maps && time() - $idle_maps->{time} < $KV_REFRESH_SECONDS) { + $maps = $idle_maps->{maps}; + } else { + $maps = eval { PVE::Multipath::get_maps() }; + if (my $err = $@) { + $status->{'report-error'} = "querying multipathd failed - $err"; + $maps = []; + $idle_maps = undef; + } else { + $idle_maps = %$wanted ? undef : { time => time(), maps => $maps }; + } + } + } + } + + my ($luns, $unmanaged) = status_luns($wanted, $maps); + $status->{luns} = $luns; + $status->{unmanaged} = $unmanaged if $unmanaged; + + eval { update_node_kv($status) }; + warn "multipath: $@" if $@; +} + +sub read_status_broadcasts() { + require PVE::Cluster; + + my $now = time(); + my $members = PVE::Cluster::get_members() // {}; + my $raw = PVE::Cluster::get_node_kv($KV_KEY); + my $res = {}; + for my $node (keys %$raw) { + my $decoded = eval { decode_json($raw->{$node}) }; + next if ref($decoded) ne 'HASH'; + + my $entry = { + supported => $decoded->{supported} ? 1 : 0, + running => $decoded->{running} ? 1 : 0, + luns => ref($decoded->{luns}) eq 'HASH' ? $decoded->{luns} : undef, + time => $decoded->{time}, + }; + for my $key (qw(apply-error report-error unmanaged unresolved-luns summary)) { + $entry->{$key} = $decoded->{$key} if defined($decoded->{$key}); + } + # a report of an older release has none, a buggy peer may send anything + $entry->{'iscsi-initiator-name'} = $decoded->{'iscsi-initiator-name'} + if valid_iscsi_name($decoded->{'iscsi-initiator-name'}); + + my $reason; + if (%$members && (!$members->{$node} || !$members->{$node}->{online})) { + $reason = 'offline'; + } elsif (!defined($decoded->{time}) || $now - $decoded->{time} > $KV_STALE_SECONDS) { + $reason = 'stale'; + } elsif (!$entry->{supported}) { + $reason = 'unsupported'; + } elsif (!$entry->{running}) { + $reason = 'not-running'; + } elsif (defined($entry->{'report-error'})) { + $reason = 'query-error'; + } + $entry->{fresh} = defined($reason) ? 0 : 1; + $entry->{reason} = $reason if defined($reason); + + $res->{$node} = $entry; + } + return $res; +} # Older nodes can drop the unknown multipath option when rewriting storage.cfg; offline nodes # may also lack a report, so silence is not evidence of compatibility. diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 88134473..8cbe9308 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -300,7 +300,7 @@ sub storage_has_feature { } # Retain empty entries on failure: absence means the storage is no longer enabled here, not merely -# unreachable. +# unreachable. scanned requires every portal's completed scan and identified LUNs. sub derived_multipath_wwids { my ($cfg, $ids) = @_; diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm index 4831d2d4..ad3afaf6 100644 --- a/src/PVE/Storage/ISCSIPlugin.pm +++ b/src/PVE/Storage/ISCSIPlugin.pm @@ -751,7 +751,8 @@ sub iscsi_device_list { return $res; } -# Expect one path per node-record portal. +# Expect one path per node-record portal. scanned requires completed, stable sessions on every +# portal and all LUNs identified; an incomplete scan must not prove absence or remove owned entries. sub multipath_lun_wwids { my ($class, $scfg, $ids) = @_; @@ -760,13 +761,19 @@ sub multipath_lun_wwids { $ids //= PVE::Multipath::Identity::collect(targets => { $target => 1 }); my $luns = PVE::Multipath::Identity::target_luns($ids, $target); - return $res if !scalar(@$luns); - my $paths_expected = scalar(iscsi_portals($target, $scfg->{portal})->@*); + my $portals = iscsi_portals($target, $scfg->{portal}); + my @unresolved; for my $lun (@$luns) { - next if !defined($lun->{wwid}); - $res->{wwids}->{ $lun->{wwid} } = { 'paths-expected' => $paths_expected }; + if (!defined($lun->{wwid})) { + push @unresolved, "$target $lun->{lun}"; + next; + } + $res->{wwids}->{ $lun->{wwid} } = { 'paths-expected' => scalar(@$portals) }; } + $res->{unresolved} = [sort @unresolved] if @unresolved; + $res->{scanned} = 1 + if !@unresolved && PVE::Multipath::Identity::portals_covered($ids, $target, $portals); return $res; } diff --git a/src/test/run_lun_identity_tests.pl b/src/test/run_lun_identity_tests.pl index 33b92d7c..783fac18 100755 --- a/src/test/run_lun_identity_tests.pl +++ b/src/test/run_lun_identity_tests.pl @@ -260,18 +260,8 @@ my sub put { close($fh); } -# An iSCSI session as the kernel's transport class shows it: its SCSI target below the session -# device, which the session class links to. +# the SCSI targets of the sessions, see session_device() my %session_dirs; -my sub session { - my ($sid, $target, $scsi_target) = @_; - my ($host) = $scsi_target =~ m/^(\d+):/; - my $dir = "devices/platform/host$host/session$sid"; - make_path("$sys/$dir/target$scsi_target"); - put("class/iscsi_session/session$sid/targetname", "$target\n"); - symlink("$sys/$dir", "$sys/class/iscsi_session/session$sid/device") or die "$!\n"; - $session_dirs{$scsi_target} = "$dir/target$scsi_target"; -} # A SCSI device, below the target of its session if it has one, listed on the SCSI bus. my sub scsi_device { @@ -322,9 +312,27 @@ my sub wwids_of { }; } +# an iSCSI session on its SCSI host, logged in to $portal and done with its scan for LUNs +my sub session_device { + my ($sid, $host, $target, $portal, $id) = @_; + $id //= 0; + my $dir = "devices/platform/host$host/session$sid"; + make_path("$sys/$dir/target$host:0:$id"); + $session_dirs{"$host:0:$id"} = "$dir/target$host:0:$id"; + my $class = "class/iscsi_session/session$sid"; + put("$class/targetname", "$target\n"); + put("$class/state", "LOGGED_IN\n"); + put("$class/target_state", "SCANNED\n"); + my ($address, $port) = $portal =~ m/^(.+):(\d+)$/; + my $conn = "$dir/connection$sid:0/iscsi_connection/connection$sid:0"; + put("$conn/persistent_address", "$address\n"); + put("$conn/persistent_port", "$port\n"); + symlink("$sys/$dir", "$sys/$class/device") or die "$!\n"; +} + # session 1 to $T on host 7 with LUN 0 (a map) and LUN 1 (no map), session 2 to $O on host 8 -session(1, $T, '7:0:0'); -session(2, $O, '8:0:0'); +session_device(1, 7, $T, '192.0.2.10:3260'); +session_device(2, 8, $O, '192.0.2.20:3260'); disk_device('7:0:0:0', 'sdb', '8:16'); disk_device('7:0:0:1', 'sdc', '8:32'); disk_device('8:0:0:0', 'sdd', '8:48'); @@ -422,9 +430,9 @@ put('bus/scsi/devices/7:0:0:1/block/sdc/dev', "8:32\n"); # the only LUN of a third target: a path with a map, one that the daemon names differently my $P = 'iqn.2003-01.org.linux-iscsi.third:sn.3'; -session(3, $P, '9:0:0'); +session_device(3, 9, $P, '192.0.2.30:3260'); # a second session to it on the same SCSI host, as offload initiators run them -session(4, $P, '9:0:1'); +session_device(7, 9, $P, '192.0.2.31:3260', 1); disk_device('9:0:0:0', 'sde', '8:64'); disk_device('9:0:1:0', 'sdf', '8:80'); hold('9:0:0:0', 'sde', 'dm-2', '3W9'); @@ -445,12 +453,12 @@ is_deeply( # two targets through one SCSI host, as offload initiators run them: each LUN stays with the target # of its own session my ($Q, $R) = ('iqn.2003-01.org.linux-iscsi.fourth:sn.4', 'iqn.2003-01.org.linux-iscsi.fifth:sn.5'); -session(5, $Q, '10:0:0'); -session(6, $R, '10:0:1'); -disk_device('10:0:0:0', 'sdg', '8:96'); -disk_device('10:0:1:0', 'sdh', '8:112'); -hold('10:0:0:0', 'sdg', 'dm-3', '3WQ'); -hold('10:0:1:0', 'sdh', 'dm-4', '3WR'); +session_device(8, 11, $Q, '192.0.2.60:3260'); +session_device(9, 11, $R, '192.0.2.70:3260', 1); +disk_device('11:0:0:0', 'sdg', '8:96'); +disk_device('11:0:1:0', 'sdh', '8:112'); +hold('11:0:0:0', 'sdg', 'dm-3', '3WQ'); +hold('11:0:1:0', 'sdh', 'dm-4', '3WR'); for my $case ([$Q, '3WQ', 'sdg'], [$R, '3WR', 'sdh']) { my ($target, $wwid, $path) = @$case; @@ -503,8 +511,8 @@ my $iscsi = Test::MockModule->new('PVE::Storage::ISCSIPlugin'); $iscsi->mock(iscsi_portals => sub { ['192.0.2.10:3260', '192.0.2.11:3260'] }); is_deeply( PVE::Storage::ISCSIPlugin->multipath_lun_wwids({ target => $T, portal => '192.0.2.10' }, $ids), - { wwids => { '3W0' => { 'paths-expected' => 2 } } }, - 'derived: the resolved LUNs, with a path per portal expected', + { wwids => { '3W0' => { 'paths-expected' => 2 } }, unresolved => ["$T 1"] }, + 'derived: the resolved LUNs, with a path per portal expected, and the others', ); my $identity = Test::MockModule->new('PVE::Multipath::Identity'); @@ -555,4 +563,187 @@ my $derived = PVE::Storage::derived_multipath_wwids({ is_deeply([sort keys %$derived], ['a', 'b'], 'derived: every storage with the option'); is($collected, 1, 'derived: the identities are collected once for all of them'); +# --- whether a storage looked at all its LUNs --- + +my sub session { + my ($sid, $target, $portal, $scan, %extra) = @_; + return ( + $sid => { + target => $target, + host => $sid + 6, + state => $extra{state} // 'LOGGED_IN', + scan => $scan, + portal => $portal, + }, + ); +} +my $portals = ['192.0.2.10:3260', '192.0.2.11:3260']; +my $both = { session(1, $T, $portals->[0], 'SCANNED'), session(2, $T, $portals->[1], 'SCANNED') }; +my sub covered { + my ($before, $after) = @_; + return PVE::Multipath::Identity::portals_covered( + { sessions => $before, 'sessions-after' => $after // $before }, + $T, + $portals, + ); +} +ok(covered($both), 'covered: every portal with a session that finished its scan'); +ok(!covered({ session(1, $T, $portals->[0], 'SCANNED') }), + 'covered: not with a portal without one'); +ok( + !covered({ $both->%*, session(2, $T, $portals->[1], 'ALLOCATED') }), + 'covered: not while the scan of a session still runs', +); +ok( + !covered({ $both->%*, session(2, $T, $portals->[1], '') }), + 'covered: not without the scan state', +); +ok( + !covered({ $both->%*, session(2, $T, $portals->[1], 'SCANNED', state => 'FAILED') }), + 'covered: not with a session that is not logged in', +); +ok( + !covered( + $both, + { session(1, $T, $portals->[0], 'SCANNED'), session(3, $T, $portals->[1], 'SCANNED') }, + ), + 'covered: not if a session changed while the LUNs were collected', +); +ok( + !PVE::Multipath::Identity::portals_covered({ sessions => {}, 'sessions-after' => {} }, $T, []), + 'covered: not without any portal', +); + +$ids = { + luns => PVE::Multipath::Identity::resolve( + { disk(0, 1, map => '3W0'), disk(1, 2) }, + { line(2, 1, '3W1') }, + $naa_ok, + ), + sessions => $both, + 'sessions-after' => $both, +}; +$iscsi->mock(iscsi_portals => sub { $portals }); +my $scfg = { target => $T, portal => '192.0.2.10' }; +is_deeply( + PVE::Storage::ISCSIPlugin->multipath_lun_wwids($scfg, $ids), + { + wwids => { '3W0' => { 'paths-expected' => 2 }, '3W1' => { 'paths-expected' => 2 } }, + scanned => 1, + }, + 'derived: every LUN identified on every portal: looked at all of them', +); +$ids->{luns} = PVE::Multipath::Identity::resolve( + { disk(0, 1, map => '3W0'), disk(1, 2) }, + { line(2, 1, '3X1', init => 'udev-missing') }, + $naa_ok, +); +is_deeply( + PVE::Storage::ISCSIPlugin->multipath_lun_wwids($scfg, $ids), + { wwids => { '3W0' => { 'paths-expected' => 2 } }, unresolved => ["$T 1"] }, + 'derived: a LUN not identified: the others still, but not looked at all of them', +); +$ids->{luns} = {}; +is_deeply( + PVE::Storage::ISCSIPlugin->multipath_lun_wwids($scfg, $ids), + { wwids => {}, scanned => 1 }, + 'derived: no LUN left on a target that every portal reaches', +); +$ids->{sessions} = + { session(1, $T, $portals->[0], 'ALLOCATED'), session(2, $T, $portals->[1], 'SCANNED') }; +is_deeply( + PVE::Storage::ISCSIPlugin->multipath_lun_wwids($scfg, $ids), + { wwids => {} }, + 'derived: but not while a scan of the target runs', +); + +is( + PVE::Multipath::Generator::unresolved_lun_count({ + a => { unresolved => ["$T 1", "$T 2"] }, + b => { unresolved => ["$T 2"] }, + c => {}, + }), + 2, + 'unresolved: a LUN listed by two storages counts once', +); + +# --- whether a storage looked at all its LUNs, from sysfs --- + +{ + $iscsi->unmock_all(); + $identity->unmock_all(); + my $portal = '192.0.2.10:3260'; + $iscsi->mock(iscsi_portals => sub { [$portal] }); + my $scfg = { target => $T, portal => '192.0.2.10', multipath => 1 }; + $reply = { + sdc => { + devt => '8:32', + hctl => '7:0:0:1', + init => 'ok', + target => $T, + wwid => '3600a09803832355367242f747a716b7a', + }, + }; + my sub derived { PVE::Storage::ISCSIPlugin->multipath_lun_wwids($scfg) } + + ok(derived()->{scanned}, 'sysfs: every portal covered, every LUN identified'); + $iscsi->mock(iscsi_portals => sub { [$portal, '192.0.2.11:3260'] }); + ok(!derived()->{scanned}, 'sysfs: not with a portal without a session'); + $iscsi->mock(iscsi_portals => sub { [$portal] }); + put('class/iscsi_session/session1/target_state', "ALLOCATED\n"); + ok( + !derived()->{scanned} && !derived()->{scanned}, + 'sysfs: not while the scan still runs, again and again', + ); + put('class/iscsi_session/session1/target_state', "SCANNED\n"); + { + # the session goes through a recovery while the LUNs are collected + $multipath->mock( + query_path_identities => sub { + put('class/iscsi_session/session1/state', "FAILED\n"); + return $reply; + }, + ); + ok(!derived()->{scanned}, 'sysfs: not if the session changed during the collection'); + put('class/iscsi_session/session1/state', "LOGGED_IN\n"); + $multipath->mock(query_path_identities => + sub { $queries++; die $reply if !ref($reply); return $reply }); + } + + # removing the last LUN of a target also makes the kernel remove the session's SCSI target + my $empty = 'iqn.2003-01.org.linux-iscsi.empty:sn.4'; + session_device(4, 10, $empty, '192.0.2.40:3260'); + rmdir("$sys/devices/platform/host10/session4/target10:0:0") or die "$!\n"; + $iscsi->mock(iscsi_portals => sub { ['192.0.2.40:3260'] }); + is_deeply( + PVE::Storage::ISCSIPlugin->multipath_lun_wwids({ + target => $empty, + portal => '192.0.2.40', + }), + { wwids => {}, scanned => 1 }, + 'sysfs: a target without LUNs, all of them gone, looked at', + ); +} + +# --- the count of LUNs a node cannot identify, through its report --- + +{ + my $cluster = Test::MockModule->new('PVE::Cluster'); + my $sent; + $cluster->mock( + broadcast_node_kv => sub { $sent = $_[1] }, + get_node_kv => sub { { lab1 => $sent } }, + get_members => sub { { lab1 => { online => 1 } } }, + rrd_dump => sub { {} }, + ); + $multipath->mock(is_supported => sub { 0 }); + PVE::Multipath::Status::broadcast_status({ wwids => {}, unresolved => 2 }); + is( + PVE::Multipath::Status::read_status_broadcasts()->{lab1}->{'unresolved-luns'}, + 2, + 'report: the count of LUNs the node cannot identify', + ); + $multipath->unmock('is_supported'); +} + done_testing(); diff --git a/src/test/run_multipath_api_tests.pl b/src/test/run_multipath_api_tests.pl index fdd9ec39..3f0c8a92 100755 --- a/src/test/run_multipath_api_tests.pl +++ b/src/test/run_multipath_api_tests.pl @@ -22,6 +22,80 @@ use PVE::Storage; use PVE::Storage::ISCSIPlugin; use PVE::Multipath::Status; +subtest 'every node sends a bounded status report' => sub { + my $kv = {}; + my $cluster = Test::MockModule->new('PVE::Cluster'); + $cluster->mock( + get_node_kv => sub { + +{ map { $_ => $kv->{$_} } keys %$kv }; + }, + ); + $cluster->mock(get_members => sub { {} }); + my $node = 'n1'; + $cluster->mock( + broadcast_node_kv => sub { + die "oversized message\n" if length($_[1]) >= 32768; + $kv->{$node} = $_[1]; + }, + ); + my $mp = Test::MockModule->new('PVE::Multipath'); + my $supported = 0; + $mp->mock(is_supported => sub { $supported }); + $mp->mock(is_running => sub { 1 }); + my $maps = []; + $mp->mock(get_maps => sub { $maps }); + + PVE::Multipath::Status::broadcast_status({}); + my $reports = PVE::Multipath::Status::read_status_broadcasts(); + ok($reports->{n1}, 'a node without multipath-tools still reports'); + is($reports->{n1}->{reason}, 'unsupported', 'and says why it has no map states'); + + $node = 'n2'; + $supported = 1; + $maps = [{ wwid => 'w1', health => 'optimal', 'paths-active' => 2, 'paths-total' => 2 }]; + PVE::Multipath::Status::broadcast_status({ + wwids => { w1 => {}, w2 => { storages => ['san'] } } }); + $reports = PVE::Multipath::Status::read_status_broadcasts(); + ok($reports->{n2}->{fresh}, 'a node with a running daemon reports current states'); + is($reports->{n2}->{luns}->{w1}->{state}, 'optimal', 'with the state of an assembled map'); + is_deeply($reports->{n2}->{luns}->{w2}, { storages => ['san'] }, 'and the unassembled LUN'); + + $node = 'n3'; + $mp->mock(get_maps => sub { die "query failed\n" }); + PVE::Multipath::Status::broadcast_status({ wwids => { w1 => {} } }); + $reports = PVE::Multipath::Status::read_status_broadcasts(); + is( + $reports->{n3}->{reason}, + 'query-error', + 'a failing query is not evidence of a missing map', + ); + ok($reports->{n3}->{luns}->{w1}, 'and keeps the wanted LUNs listed'); + + $node = 'n5'; + my $queries = 0; + $mp->mock(get_maps => sub { $queries++; die "query failed\n" }); + PVE::Multipath::Status::broadcast_status({}); + $mp->mock(get_maps => sub { $queries++; [] }); + PVE::Multipath::Status::broadcast_status({}); + is($queries, 2, 'a failed query of an idle node is not reused'); + ok( + PVE::Multipath::Status::read_status_broadcasts()->{n5}->{fresh}, + 'and the next one counts', + ); + PVE::Multipath::Status::broadcast_status({}); + is($queries, 2, 'while a successful one is'); + + $node = 'n4'; + $mp->mock(get_maps => sub { [] }); + my $wanted = { map { (sprintf('3600%032x', $_) => { storages => ['san'] }) } 1 .. 1000 }; + PVE::Multipath::Status::broadcast_status({ wwids => $wanted, error => 'timeout ' x 10000 }); + $reports = PVE::Multipath::Status::read_status_broadcasts(); + ok($reports->{n4}->{fresh}, 'a node with more LUNs than fit still reports'); + ok(!defined($reports->{n4}->{luns}), 'without a partial list'); + is_deeply($reports->{n4}->{summary}, { luns => 1000, missing => 1000 }, 'but with counts'); + ok(length($reports->{n4}->{'apply-error'}) < 2100, 'and a bounded apply error'); +}; + subtest 'enabling the multipath option needs every node to know it' => sub { my $cluster = Test::MockModule->new('PVE::Cluster'); my $inotify = Test::MockModule->new('PVE::INotify'); @@ -136,6 +210,83 @@ subtest 'the iSCSI initiator identity reader' => sub { } }; +subtest 'the report carries the iSCSI initiator name of the node' => sub { + my $kv = {}; + my $cluster = Test::MockModule->new('PVE::Cluster'); + $cluster->mock(get_node_kv => sub { +{%$kv} }); + $cluster->mock(get_members => sub { {} }); + $cluster->mock(rrd_dump => sub { {} }); + $cluster->mock(broadcast_node_kv => sub { $kv->{n1} = $_[1] }); + my $mp = Test::MockModule->new('PVE::Multipath'); + my $supported = 0; + $mp->mock(is_supported => sub { $supported }); + $mp->mock(is_running => sub { 1 }); + $mp->mock(get_maps => sub { [] }); + my $iscsi = Test::MockModule->new('PVE::Storage::ISCSIPlugin'); + my $initiator; + $iscsi->mock(read_initiator_info => sub { $initiator }); + my $sent = sub { + my ($apply) = @_; + PVE::Multipath::Status::broadcast_status($apply // {}); + return PVE::Multipath::Status::read_status_broadcasts()->{n1}; + }; + + my $iqn = 'iqn.2026-10.test.lab:n1'; + $initiator = { name => $iqn }; + is($sent->()->{'iscsi-initiator-name'}, $iqn, 'a node without multipath-tools sends it'); + + $supported = 1; + $initiator = { name => 'iqn.2026-10.test.lab:renamed', alias => 'n1' }; + is( + $sent->()->{'iscsi-initiator-name'}, + 'iqn.2026-10.test.lab:renamed', + 'a changed name goes out with the next report', + ); + + $initiator = undef; + ok(!exists($sent->()->{'iscsi-initiator-name'}), 'a removed one is gone from it'); + + $iscsi->mock(read_initiator_info => sub { die "unreadable\n" }); + ok(!exists($sent->()->{'iscsi-initiator-name'}), 'an unreadable file only leaves it out'); + $iscsi->mock(read_initiator_info => sub { $initiator }); + + my $invalid = [ + ['iqn.2026-10.test.lab:' . ('x' x 210), 'a name too long to be an iSCSI name'], + ['iqn.bogus', 'an IQN without a date'], + ['iqn.2026-10.a b', 'one with a blank'], + ]; + for my $case ($invalid->@*) { + $initiator = { name => $case->[0] }; + ok(!exists($sent->()->{'iscsi-initiator-name'}), "is left out, not cut: $case->[1]"); + } + for my $name ('eui.02004567A425678D', 'naa.52004567BA64678D') { + $initiator = { name => $name }; + is($sent->()->{'iscsi-initiator-name'}, $name, "the other name formats: $name"); + } + + $initiator = { name => $iqn }; + my $wanted = { map { (sprintf('3600%032x', $_) => { storages => ['san'] }) } 1 .. 1000 }; + my $report = $sent->({ wwids => $wanted }); + ok($report->{summary}, 'a node with too many LUNs sends a summary'); + is($report->{'iscsi-initiator-name'}, $iqn, 'with the name'); + + $kv->{n1} = JSON::encode_json({ supported => 1, running => 1, luns => {}, time => time() }); + ok( + !exists( + PVE::Multipath::Status::read_status_broadcasts()->{n1}->{'iscsi-initiator-name'} + ), + 'a report of an older release reads without one', + ); + $kv->{n1} = + JSON::encode_json({ luns => {}, time => time(), 'iscsi-initiator-name' => ['x'] }); + ok( + !exists( + PVE::Multipath::Status::read_status_broadcasts()->{n1}->{'iscsi-initiator-name'} + ), + 'and a garbled one from a peer is dropped', + ); +}; + subtest 'an entry stays allow-listed while its map is in use' => sub { my $mp = Test::MockModule->new('PVE::Multipath'); my $map = { wwid => 'w1', sysfs => 'dm-999999', 'paths-active' => 2 }; diff --git a/src/test/run_multipath_tests.pl b/src/test/run_multipath_tests.pl index 992dd3dd..1c28ac86 100755 --- a/src/test/run_multipath_tests.pl +++ b/src/test/run_multipath_tests.pl @@ -3,6 +3,7 @@ use strict; use warnings; +use JSON; use Test::MockModule; use Test::More; @@ -367,13 +368,35 @@ is_deeply( PVE::Multipath::Generator::desired_wwid_info( { w1 => ['config', 'storage:a', 'storage:b'], w2 => ['config'] }, { - a => { wwids => { w1 => { 'paths-expected' => 2 } } }, - b => { wwids => { w1 => { 'paths-expected' => 4 } } }, + a => { wwids => { w1 => { 'paths-expected' => 2 } }, scanned => 1 }, + b => { wwids => { w1 => { 'paths-expected' => 4 } }, scanned => 1 }, }, ), - { w1 => { storages => ['a', 'b'], 'paths-expected' => 4 }, w2 => {} }, + { w1 => { storages => ['a', 'b'], 'paths-expected' => 4, derived => 1 }, w2 => {} }, 'every deriving storage is named, the largest path expectation wins', ); +is_deeply( + PVE::Multipath::Generator::desired_wwid_info( + { + gone => ['storage:a'], + unseen => ['storage:a', 'storage:b'], + once => ['storage:a', 'storage:b'], + failed => ['storage:c'], + }, + { + a => { wwids => { once => {} }, scanned => 1 }, + b => { wwids => {} }, + c => { wwids => {}, error => 'boom' }, + }, + ), + { + gone => { storages => ['a'], derived => 0 }, + unseen => { storages => ['a', 'b'] }, + once => { storages => ['a', 'b'], derived => 1 }, + failed => { storages => ['c'] }, + }, + 'derived: 0 only if every storage looked and none found it, unknown if one could not look', +); # --- WWID allow-list reconcile plan: only prune what PVE itself added, and only once every --- # --- reason it was added for is gone from the configuration --- @@ -632,4 +655,89 @@ is( ); } +# --- per-LUN status report --- +my $wanted_all = { map { $_->{wwid} => {} } $maps->@* }; +my ($luns, $unmanaged) = PVE::Multipath::Status::status_luns($wanted_all, $maps); +is_deeply([sort keys %$luns], [sort keys %$wanted_all], 'every wanted LUN is reported'); +is($unmanaged, 0, 'no unmanaged maps when everything is wanted'); +is($luns->{ $a->{wwid} }->{state}, 'optimal', 'optimal map reported as optimal'); +is($luns->{ $a->{wwid} }->{'paths-active'}, 2, 'active path count carried'); +is($luns->{ $a->{wwid} }->{'paths-total'}, 2, 'total path count carried'); +is($luns->{ $b->{wwid} }->{state}, 'degraded', 'degraded map reported as degraded'); +is($luns->{ $c->{wwid} }->{state}, 'failed', 'failed map reported as failed'); +ok(!exists $luns->{ $a->{wwid} }->{transport}, 'transport omitted when not derived'); + +($luns, $unmanaged) = PVE::Multipath::Status::status_luns({}, []); +is_deeply($luns, {}, 'nothing wanted and nothing assembled reports nothing'); + +($luns, $unmanaged) = PVE::Multipath::Status::status_luns( + { $a->{wwid} => { storages => ['san1'], 'paths-expected' => 4 }, w404 => {} }, $maps, +); +is_deeply([sort keys %$luns], [sort($a->{wwid}, 'w404')], 'only wanted LUNs are listed'); +is($unmanaged, 2, 'other maps are counted as unmanaged'); +is( + $luns->{ $a->{wwid} }->{state}, + 'degraded', + 'two active paths of four expected read as degraded, the missing ones are gone from the total', +); +is_deeply($luns->{ $a->{wwid} }->{storages}, ['san1'], 'the deriving storages are carried'); +is_deeply($luns->{w404}, {}, 'a wanted LUN without a map is listed without a state'); +($luns) = + PVE::Multipath::Status::status_luns({ w404 => { storages => ['san1'], derived => 0 } }, $maps); +is_deeply( + $luns->{w404}, + { storages => ['san1'], derived => 0 }, + 'whether the storage still derives the LUN is carried', +); +($luns) = PVE::Multipath::Status::status_luns({ $a->{wwid} => { 'paths-expected' => 2 } }, $maps); +is($luns->{ $a->{wwid} }->{state}, 'optimal', 'two of two expected stay optimal'); + +my $enriched = [{ + wwid => '3600x', + health => 'optimal', + 'paths-active' => 2, + 'paths-total' => 2, + transport => 'iscsi', + size => 34359738368, +}]; +($luns) = PVE::Multipath::Status::status_luns({ '3600x' => { kept => 'LVM' } }, $enriched); +is($luns->{'3600x'}->{transport}, 'iscsi', 'transport carried into the report'); +is($luns->{'3600x'}->{size}, 34359738368, 'size carried into the report'); +is($luns->{'3600x'}->{kept}, 'LVM', 'what keeps an unconfigured LUN is carried'); + +# --- encoding within the KV size limit --- +my $small = PVE::Multipath::Status::encode_status({ + supported => 1, + running => 1, + luns => { w1 => { state => 'optimal' } }, + 'apply-error' => 'x' x 5000, +}); +my $decoded = decode_json($small); +is_deeply($decoded->{luns}, { w1 => { state => 'optimal' } }, 'a small status keeps its LUNs'); +ok(length($decoded->{'apply-error'}) < 2100, 'an oversized apply error is cut down'); + +my $many = { + map { ( + sprintf('3600140500000000000000000000%04x', $_) => { + storages => ['san1'], + 'paths-expected' => 4, + state => $_ % 100 ? 'optimal' : 'degraded', + 'paths-active' => 4, + 'paths-total' => 4, + transport => 'iscsi', + size => 1099511627776, + }, + ) } 0 .. 999 +}; +$many->{wNone} = {}; +my $big = PVE::Multipath::Status::encode_status({ supported => 1, running => 1, luns => $many }); +ok(length($big) <= 30 * 1024, 'a large status stays within the KV limit'); +$decoded = decode_json($big); +ok(!exists $decoded->{luns}, 'a large status sends no partial LUN list'); +is_deeply( + $decoded->{summary}, + { luns => 1001, optimal => 990, degraded => 10, missing => 1 }, + 'but how many LUNs are in which state', +); + done_testing(); -- 2.47.3