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 EFB761FF0AF for ; Fri, 06 Nov 2026 01:59:31 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id AD83121F0B; Fri, 06 Nov 2026 01:51:23 +0100 (CET) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [PATCH v3 storage 29/76] iscsi: add an option to manage storage LUNs with multipath Date: Fri, 6 Nov 2026 00:31:04 +0100 Message-ID: <20261105233336.2178064-30-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: 1793926179244 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.170 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) KAM_LOTSOFHASH 0.25 Emails with lots of hash-like gibberish 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: CHSVKY7XFB5FNTEZJ3PSFW3VGDWZCDHR X-Message-ID-Hash: CHSVKY7XFB5FNTEZJ3PSFW3VGDWZCDHR 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: Let iSCSI storages request multipath maps so LVM uses a stable LUN identity rather than an arbitrary path. Require new LVM base volumes to name the map and wait for it before initialization. Existing path-based base volumes also wait for their map. Refuse disabling multipath while LVM depends on one. Resolve identities from maps and daemon inventory, rejecting changed paths or conflicting evidence. Preserve unresolved identities so callers do not mistake an unchecked LUN for a device without a SAN identity. Older nodes drop unknown storage options when rewriting configuration. Require support reports from every cluster node and multipath tools on the storage nodes before accepting the option. Signed-off-by: Thomas Lamprecht --- src/PVE/Makefile | 2 + src/PVE/Multipath.pm | 45 ++ src/PVE/Multipath/Identity.pm | 221 ++++++++++ src/PVE/Multipath/Status.pm | 70 ++++ src/PVE/Storage.pm | 31 ++ src/PVE/Storage/ISCSIPlugin.pm | 147 ++++++- src/PVE/Storage/LVMPlugin.pm | 8 +- src/test/Makefile | 17 +- src/test/run_iscsi_activation_tests.pl | 171 ++++++++ src/test/run_lun_identity_tests.pl | 557 +++++++++++++++++++++++++ src/test/run_multipath_api_tests.pl | 92 ++++ 11 files changed, 1353 insertions(+), 8 deletions(-) create mode 100644 src/PVE/Multipath/Identity.pm create mode 100644 src/PVE/Multipath/Status.pm create mode 100755 src/test/run_iscsi_activation_tests.pl create mode 100755 src/test/run_lun_identity_tests.pl create mode 100755 src/test/run_multipath_api_tests.pl diff --git a/src/PVE/Makefile b/src/PVE/Makefile index b92059bd..29d0146d 100644 --- a/src/PVE/Makefile +++ b/src/PVE/Makefile @@ -7,6 +7,8 @@ install: install -D -m 0644 Multipath.pm ${DESTDIR}${PERLDIR}/PVE/Multipath.pm install -D -m 0644 Multipath/Config.pm ${DESTDIR}${PERLDIR}/PVE/Multipath/Config.pm install -D -m 0644 Multipath/ClusterConfig.pm ${DESTDIR}${PERLDIR}/PVE/Multipath/ClusterConfig.pm + install -D -m 0644 Multipath/Identity.pm ${DESTDIR}${PERLDIR}/PVE/Multipath/Identity.pm + install -D -m 0644 Multipath/Status.pm ${DESTDIR}${PERLDIR}/PVE/Multipath/Status.pm install -D -m 0644 CephConfig.pm ${DESTDIR}${PERLDIR}/PVE/CephConfig.pm install -D -m 0644 GuestImport.pm ${DESTDIR}${PERLDIR}/PVE/GuestImport.pm make -C Storage install diff --git a/src/PVE/Multipath.pm b/src/PVE/Multipath.pm index 69589ac3..e2f7ea40 100644 --- a/src/PVE/Multipath.pm +++ b/src/PVE/Multipath.pm @@ -2,6 +2,8 @@ package PVE::Multipath; use v5.36; +use JSON; + use PVE::Tools qw(run_command file_read_firstline); use PVE::RS::Multipath; @@ -14,6 +16,9 @@ my $QUERY_TIMEOUT = 10; my $TOOL_TIMEOUT = 30; my $RECONFIGURE_TIMEOUT = 120; +# Allow multipathd's default four-second socket timeout during startup or reconfiguration. +my $PATH_QUERY_TIMEOUT = 5; + my $supported; # Cached only while true: a long-lived daemon like pvestatd must notice a later multipath-tools @@ -64,6 +69,46 @@ sub parse_maps_json($json) { return PVE::RS::Multipath::parse_maps_json($json); } +my $PATH_FORMAT = '%d|%D|%i|%I|%n|%w'; + +# Daemon placeholders are not identity evidence; callers must validate them. A non-list reply +# means the daemon could not answer, unlike an empty list. +sub parse_path_identities($text) { + my @lines = grep { length } split(/\n/, $text // ''); + # the reply to a list without entries + return {} if scalar(@lines) == 1 && $lines[0] eq 'ok'; + + my ($res, $count) = ({}, {}); + for my $line (@lines) { + my ($dev, $devt, $hctl, $init, $target, $wwid) = split(/\|/, $line, 6); + return undef if !defined($wwid); + return undef + if $dev !~ m/^\S+$/ + || $devt !~ m/^(?:#:#|\d+:\d+)$/ + || $hctl !~ m/^(?:#:#:#:#|\d+:\d+:\d+:\d+)$/ + || $init !~ m/^[a-z-]+$/; + next if $dev eq '-'; + $count->{$dev}++; + $res->{$dev} = + { devt => $devt, hctl => $hctl, init => $init, target => $target, wwid => $wwid }; + } + delete $res->{$_} for grep { $count->{$_} > 1 } keys %$count; + return $res; +} + +sub query_path_identities() { + assert_supported(); + + my $output = ''; + run_command( + [$MULTIPATHD, 'show', 'paths', 'raw', 'format', $PATH_FORMAT], + timeout => $PATH_QUERY_TIMEOUT, + outfunc => sub($line) { $output .= "$line\n"; }, + errfunc => sub($line) { }, + ); + return parse_path_identities($output) // die "multipathd returned an invalid path list\n"; +} + # Expected paths include missing paths that the daemon no longer counts. sub derive_health($paths_active, $paths_total, $paths_expected = undef) { return PVE::RS::Multipath::derive_health($paths_active, $paths_total, $paths_expected); diff --git a/src/PVE/Multipath/Identity.pm b/src/PVE/Multipath/Identity.pm new file mode 100644 index 00000000..2a8d3d20 --- /dev/null +++ b/src/PVE/Multipath/Identity.pm @@ -0,0 +1,221 @@ +package PVE::Multipath::Identity; + +# Use multipathd's configured identity, not a guessed sysfs identifier. Maps pin their paths; +# unmapped names can be reused, so daemon entries must match dev_t, H:C:T:L and target, plus +# the cached VPD NAA designator when applicable. Only iSCSI has grouping evidence before resolution. + +use v5.36; + +use Cwd qw(abs_path); +use File::Basename qw(basename); + +use PVE::Multipath; +use PVE::Multipath::Config; +use PVE::Tools qw(dir_glob_foreach dir_glob_regex file_get_contents file_read_firstline); + +our $SYSFS = '/sys'; + +my sub sysfs_line($path) { + return file_read_firstline("$SYSFS/$path"); +} + +# The designators of a VPD page 0x83 as [code set, association, type, value], undef if the page is +# malformed. +sub vpd_designators($page) { + return undef if !defined($page) || length($page) < 4; + my ($code, $length) = unpack('x C n', $page); + my $end = 4 + $length; + return undef if $code != 0x83 || $end > length($page); + my ($pos, @designators) = (4); + while ($pos < $end) { + return undef if $pos + 4 > $end; + my ($set, $kind, $size) = unpack("x$pos C C x C", $page); + my $next = $pos + 4 + $size; + return undef if $next > $end; + push @designators, + [$set & 0x0f, ($kind & 0x30) >> 4, $kind & 0x0f, substr($page, $pos + 4, $size)]; + $pos = $next; + } + return \@designators; +} + +sub vpd_has_naa($page, $hex) { + my $designators = vpd_designators($page) // return 0; + # code set 1 (binary), association 0 (logical unit), designator type 3 (NAA) + my @match = grep { + my ($set, $association, $type, $value) = $_->@*; + $set == 1 && $association == 0 && $type == 3 && unpack('H*', $value) eq lc($hex) + } $designators->@*; + return @match ? 1 : 0; +} + +my sub line_identifies($disk, $line, $naa_ok) { + return 0 + if !$line || $line->{init} ne 'ok' || $line->{wwid} !~ $PVE::Multipath::Config::WWID_RE; + return 0 if $line->{devt} ne $disk->{devt} || $line->{hctl} ne $disk->{hctl}; + return 0 if defined($disk->{target}) && $line->{target} ne $disk->{target}; + my ($hex) = $line->{wwid} =~ m/^3([0-9a-fA-F]+)$/; + return defined($hex) ? $naa_ok->($disk, $hex) : 1; +} + +# An unlisted path alone does not make its LUN unresolved when the daemon answered and there is no +# other identity evidence. +sub resolve($disks, $daemon, $naa_ok) { + my $luns = {}; + for my $name (sort keys %$disks) { + my $disk = $disks->{$name}; + my $key = "$disk->{target}\0$disk->{lun}"; + $luns->{$key} //= { target => $disk->{target}, lun => $disk->{lun}, paths => [] }; + push $luns->{$key}->{paths}->@*, $name; + } + + for my $lun (values %$luns) { + my ($held, $listed, $unlisted) = ({}, {}, 0); + my $unresolved = !$daemon; + for my $name ($lun->{paths}->@*) { + my $disk = $disks->{$name}; + $held->{ $disk->{map} } = 1 if defined($disk->{map}); + my $line = $daemon ? $daemon->{$name} : undef; + if (line_identifies($disk, $line, $naa_ok)) { + $listed->{ $line->{wwid} } = 1; + } else { + $unlisted = 1; + $unresolved = 1 if $line; + } + } + my @held = keys %$held; + if (@held) { + # A missing daemon identity does not count against a trusted map identity. + $lun->{wwid} = $held[0] + if @held == 1 && !grep { $_ ne $held[0] } keys %$listed; + } elsif (scalar(keys %$listed) == 1 && !$unlisted) { + ($lun->{wwid}) = keys %$listed; + } + $lun->{unresolved} = 1 if !defined($lun->{wwid}) && ($unresolved || @held || %$listed); + } + return $luns; +} + +# The iSCSI sessions: { sid => { target, host } }. +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 }; + }, + ); + return $res; +} + +# The target of each LUN, from the session whose SCSI target holds it: offload initiators can run +# sessions to several targets on one SCSI host. +my sub lun_targets($sessions) { + my $res = {}; + for my $sid (keys %$sessions) { + my $dir = "$SYSFS/class/iscsi_session/session$sid/device"; + dir_glob_foreach( + $dir, + 'target\d+:\d+:\d+', + sub($targetdir) { + dir_glob_foreach( + "$dir/$targetdir", + '\d+:\d+:\d+:\d+', + sub($hctl) { $res->{$hctl} = $sessions->{$sid}->{target}; }, + ); + }, + ); + } + return $res; +} + +my sub session_disks($sessions, $targets) { + my $lun_target = lun_targets($sessions); + my $res = {}; + dir_glob_foreach( + "$SYSFS/bus/scsi/devices", + '\d+:\d+:\d+:(\d+)', + sub($hctl, $lun) { + my $target = $lun_target->{$hctl} // return; + return if $targets && !$targets->{$target}; + my $dir = "bus/scsi/devices/$hctl"; + return if (sysfs_line("$dir/type") // '') ne '0'; # disks only + my (undef, $name) = dir_glob_regex("$SYSFS/$dir/block", '([a-z]\S*)'); + return if !defined($name); + my $devt = sysfs_line("$dir/block/$name/dev") // return; + my $disk = + { target => $target, lun => int($lun), devt => $devt, hctl => $hctl, dir => $dir }; + # Read the holder through the SCSI device, so neither name can refer to another device + # meanwhile. + my (undef, $holder) = dir_glob_regex("$SYSFS/$dir/block/$name/holders", '(dm-\d+)'); + if (defined($holder)) { + my $uuid = sysfs_line("$dir/block/$name/holders/$holder/dm/uuid") // ''; + ($disk->{map}) = $uuid =~ m/^mpath-(\S+)$/; + } + $res->{$name} = $disk; + }, + ); + return $res; +} + +sub collect(%param) { + my $sessions = iscsi_sessions(); + my $disks = session_disks($sessions, $param{targets}); + + my ($daemon, $error); + if (grep { !defined($_->{map}) } values %$disks) { + $daemon = eval { PVE::Multipath::query_path_identities() }; + chomp($error = $@) if $@; + } + my $naa_ok = sub($disk, $hex) { + my $page = eval { file_get_contents("$SYSFS/$disk->{dir}/vpd_pg83") }; + return vpd_has_naa($page, $hex); + }; + my $luns = resolve($disks, $daemon, $naa_ok); + my $paths = {}; + for my $lun (values %$luns) { + for my $name ($lun->{paths}->@*) { + my $disk = $disks->{$name}; + $paths->{$name} = { + $disk->%{qw(devt hctl)}, + wwid => $lun->{wwid}, + $lun->{unresolved} ? (unresolved => 1) : (), + }; + } + } + return { + luns => $luns, + paths => $paths, + defined($error) ? (error => $error) : (), + }; +} + +sub target_luns($ids, $target) { + return [grep { $_->{target} eq $target } values $ids->{luns}->%*]; +} + +# The WWID of the LUN of the path $name in $ids, if it was resolved and the name still denotes the +# same device: the kernel may have given it to another one since the collection. +sub path_wwid($ids, $name) { + my $path = $ids->{paths}->{$name} // return undef; + return undef if !defined($path->{wwid}); + my $devt = sysfs_line("class/block/$name/dev"); + my $device = abs_path("$SYSFS/class/block/$name/device"); + if ( + !defined($devt) + || !defined($device) + || $devt ne $path->{devt} + || basename($device) ne $path->{hctl} + ) { + $path->{unresolved} = 1; + return undef; + } + return $path->{wwid}; +} + +1; diff --git a/src/PVE/Multipath/Status.pm b/src/PVE/Multipath/Status.pm new file mode 100644 index 00000000..e3dccf08 --- /dev/null +++ b/src/PVE/Multipath/Status.pm @@ -0,0 +1,70 @@ +package PVE::Multipath::Status; + +use v5.36; + +use JSON; +use PVE::Multipath; +use PVE::Multipath::Config; + +my $KV_KEY = 'multipath'; + +# 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. +sub nodes_without_status() { + require PVE::Cluster; + require PVE::INotify; + + my $local = PVE::INotify::nodename(); + my $raw = PVE::Cluster::get_node_kv($KV_KEY); + my $reported = sub($node) { + my $status = eval { decode_json($raw->{$node} // '') }; + return ref($status) eq 'HASH'; + }; + return [sort grep { $_ ne $local && !$reported->($_) } PVE::Cluster::get_nodelist()->@*]; +} + +# The nodes out of $nodes ({ node => 1 }, empty or undef for every node) whose status says that +# multipath-tools is not installed; nodes without a status are left to nodes_without_status(). +sub nodes_without_tools($nodes) { + require PVE::Cluster; + require PVE::INotify; + + my $local = PVE::INotify::nodename(); + my $raw = PVE::Cluster::get_node_kv($KV_KEY); + my $res = []; + for my $node (sort { $a cmp $b } PVE::Cluster::get_nodelist()->@*) { + next if $nodes && %$nodes && !$nodes->{$node}; + my $supported; + if ($node eq $local) { + $supported = PVE::Multipath::is_supported(); + } else { + next if !defined($raw->{$node}); + my $status = eval { decode_json($raw->{$node}) }; + $supported = ref($status) eq 'HASH' && $status->{supported}; + } + push $res->@*, $node if !$supported; + } + return $res; +} + +sub assert_multipath_tools($nodes) { + my $no_tools = nodes_without_tools($nodes); + die "multipath-tools is not installed on node(s) " + . join(', ', $no_tools->@*) + . ", which the storage is available on\n" + if scalar($no_tools->@*); + return; +} + +sub assert_cluster_supports_multipath($nodes) { + my $silent = nodes_without_status(); + die "node(s) " + . join(', ', $silent->@*) + . " did not report multipath support yet - upgrade" + . " every node and make sure it is online before enabling multipath on a storage\n" + if scalar($silent->@*); + assert_multipath_tools($nodes); + return; +} + +1; diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 2fc91a82..88134473 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -19,6 +19,7 @@ use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file); use PVE::DataCenterConfig; use PVE::Exception qw(raise_param_exc raise); use PVE::JSONSchema; +use PVE::Multipath::Identity; use PVE::INotify; use PVE::RPCEnvironment; use PVE::SSHInfo; @@ -298,6 +299,36 @@ sub storage_has_feature { return PVE::Storage::Plugin::storage_has_feature($scfg->{type}, $feature); } +# Retain empty entries on failure: absence means the storage is no longer enabled here, not merely +# unreachable. +sub derived_multipath_wwids { + my ($cfg, $ids) = @_; + + my @storeids; + for my $storeid (sort keys $cfg->{ids}->%*) { + my $scfg = $cfg->{ids}->{$storeid}; + next if !$scfg->{multipath} || $scfg->{type} ne 'iscsi'; + next if !storage_check_enabled($cfg, $storeid, undef, 1); + push @storeids, $storeid; + } + return {} if !@storeids; + $ids //= PVE::Multipath::Identity::collect( + targets => { map { $cfg->{ids}->{$_}->{target} => 1 } @storeids }, + ); + + my $derived = {}; + for my $storeid (@storeids) { + my $scfg = $cfg->{ids}->{$storeid}; + my $set = eval { PVE::Storage::ISCSIPlugin->multipath_lun_wwids($scfg, $ids) }; + if (my $err = $@) { + chomp $err; + $set = { wwids => {}, error => $err }; + } + $derived->{$storeid} = $set; + } + return $derived; +} + # storage_can_replicate: # return true if storage supports replication # (volumes allocated with vdisk_alloc() has replication feature) diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm index d10a5549..0e3b6df3 100644 --- a/src/PVE/Storage/ISCSIPlugin.pm +++ b/src/PVE/Storage/ISCSIPlugin.pm @@ -8,13 +8,16 @@ use Digest::SHA qw(sha256_hex); use Fcntl qw(O_RDONLY O_EXCL O_NONBLOCK LOCK_EX LOCK_NB); use IO::Dir; use IO::File; +use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC); use PVE::Diskmanage; use PVE::JSONSchema qw(get_standard_option); use PVE::Multipath; +use PVE::Multipath::Identity; use PVE::Storage::Plugin; use PVE::Tools qw(run_command file_read_firstline trim dir_glob_regex dir_glob_foreach $IPV4RE $IPV6RE); +use PVE::Multipath::Status; use base qw(PVE::Storage::Plugin); @@ -678,11 +681,15 @@ sub iscsi_logout_guarded { return $result; } +# The LUNs of the iSCSI sessions, { target => { volname => info } }. With $map_names a LUN is named +# after its multipath map, see PVE::Multipath::Identity, and only the LUNs of $target are listed. sub iscsi_device_list { + my ($map_names, $target) = @_; my $res = {}; my $stable_paths = load_stable_scsi_paths(); + my $ids = $map_names ? PVE::Multipath::Identity::collect(targets => { $target => 1 }) : undef; my $device_map = iscsi_session_device_map(); @@ -690,6 +697,8 @@ sub iscsi_device_list { my $info = $device_map->{$bdev}; my ($channel, $id, $lun) = $info->@{qw(channel id lun)}; + my $path_dev = $bdev; + #check multipath if (-d "$SYSFS/block/$bdev/holders") { my $multipathdev = dir_glob_regex("$SYSFS/block/$bdev/holders", '[A-Za-z]\S*'); @@ -697,6 +706,15 @@ sub iscsi_device_list { } my $blockdev = $stable_paths->{$bdev}; + # 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) { + my $wwid = + $info->{target} eq $target + ? PVE::Multipath::Identity::path_wwid($ids, $path_dev) + : undef; + $blockdev = defined($wwid) ? "dm-uuid-mpath-$wwid" : undef; + } next if !$blockdev; my $size = file_read_firstline("$SYSFS/block/$bdev/size"); @@ -717,6 +735,26 @@ sub iscsi_device_list { return $res; } +# Expect one path per node-record portal. +sub multipath_lun_wwids { + my ($class, $scfg, $ids) = @_; + + my $res = { wwids => {} }; + my $target = $scfg->{target}; + + $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})->@*); + for my $lun (@$luns) { + next if !defined($lun->{wwid}); + $res->{wwids}->{ $lun->{wwid} } = { 'paths-expected' => $paths_expected }; + } + + return $res; +} + # Configuration sub type { @@ -742,6 +780,11 @@ sub properties { type => 'string', format => 'pve-storage-portal-dns', }, + multipath => { + description => "Manage multipath access to the storage's LUNs and include them in" + . " the cluster-wide path-health monitoring.", + type => 'boolean', + }, }; } @@ -753,9 +796,62 @@ sub options { disable => { optional => 1 }, content => { optional => 1 }, bwlimit => { optional => 1 }, + multipath => { optional => 1 }, }; } +my sub base_volume_consumers { + my ($cfg, $storeid, $volname_re) = @_; + + my $consumers = []; + for my $sid (sort keys $cfg->{ids}->%*) { + my $base = $cfg->{ids}->{$sid}->{base} or next; + my ($baseid, $volname) = eval { PVE::Storage::Plugin::parse_volume_id($base) }; + next if !defined($baseid) || $baseid ne $storeid; + push $consumers->@*, $sid if $volname =~ $volname_re; + } + return $consumers; +} + +sub on_add_hook { + my ($class, $storeid, $scfg, %param) = @_; + + PVE::Multipath::Status::assert_cluster_supports_multipath($scfg->{nodes}) if $scfg->{multipath}; + + return $class->SUPER::on_add_hook($storeid, $scfg, %param); +} + +sub on_update_hook_full { + my ($class, $storeid, $scfg, $update, $delete, $sensitive) = @_; + + my $disables_multipath = (defined($update->{multipath}) && !$update->{multipath}) + || grep { $_ eq 'multipath' } ($delete // [])->@*; + + if ($scfg->{multipath} && $disables_multipath) { + my $cfg = PVE::Storage::config(); + my $consumers = base_volume_consumers($cfg, $storeid, qr/\.dm-uuid-mpath-\S+$/); + die "cannot disable multipath - storage '$consumers->[0]' uses a multipath map of this" + . " storage as its base volume\n" + if scalar($consumers->@*); + } elsif (!$scfg->{multipath} && $update->{multipath}) { + my $nodes = $update->{nodes} // $scfg->{nodes}; + $nodes = undef if grep { $_ eq 'nodes' } ($delete // [])->@*; + PVE::Multipath::Status::assert_cluster_supports_multipath($nodes); + } + + my $drops_restriction = grep { $_ eq 'nodes' } ($delete // [])->@*; + if ( + $scfg->{multipath} + && !$disables_multipath + && (defined($update->{nodes}) || $drops_restriction) + ) { + PVE::Multipath::Status::assert_multipath_tools( + $drops_restriction ? undef : $update->{nodes}); + } + + return $class->SUPER::on_update_hook_full($storeid, $scfg, $update, $delete, $sensitive); +} + # Storage implementation sub parse_volname { @@ -823,13 +919,14 @@ sub list_images { my $res = []; - $cache->{iscsi_devices} = iscsi_device_list() if !$cache->{iscsi_devices}; + my $cache_key = $scfg->{multipath} ? "iscsi_devices_mpath:$scfg->{target}" : 'iscsi_devices'; + $cache->{$cache_key} //= iscsi_device_list($scfg->{multipath}, $scfg->{target}); # we have no owner for iscsi devices my $target = $scfg->{target}; - if (my $dat = $cache->{iscsi_devices}->{$target}) { + if (my $dat = $cache->{$cache_key}->{$target}) { foreach my $volname (keys %$dat) { @@ -997,9 +1094,55 @@ $resolve_virtual_devices = sub { return $resolved; }; +# The polling budget excludes an in-flight identity or map query (bounded by five and ten +# seconds respectively). Legacy aliases need not be repointed at the map, so this only waits for +# an identified map; normal activation must resolve and validate the original alias afterwards. +sub wait_for_path_map { + my ($scfg, $path, $budget) = @_; + $budget //= 10; + my $deadline = clock_gettime(CLOCK_MONOTONIC) + $budget; + while (clock_gettime(CLOCK_MONOTONIC) < $deadline) { + my $real = Cwd::realpath($path) // ''; + if ($real =~ m{^/dev/(dm-\d+)$}) { + my $uuid = file_read_firstline("/sys/class/block/$1/dm/uuid") // ''; + return 1 if $uuid =~ m/^mpath-\S+$/ && -b $real; + } elsif ($real =~ m{^/dev/(sd[a-z]+)$}) { + my $name = $1; + my $wwid = eval { + my $ids = + PVE::Multipath::Identity::collect(targets => { $scfg->{target} => 1 }); + PVE::Multipath::Identity::path_wwid($ids, $name); + }; + last if clock_gettime(CLOCK_MONOTONIC) >= $deadline; + if (defined($wwid)) { + # One query per round leaves the outer monotonic clock in charge of polling. + return 1 if eval { PVE::Multipath::wait_for_map($wwid, 0) }; + } + } + my $remaining = $deadline - clock_gettime(CLOCK_MONOTONIC); + last if $remaining <= 0; + Time::HiRes::sleep($remaining < 1 ? $remaining : 1); + } + return 0; +} + sub activate_volume { my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_; + if ($scfg->{multipath}) { + if (my ($wwid) = $volname =~ m/\.dm-uuid-mpath-(\S+)$/) { + # Activation can race map assembly at login or boot; wait before resolving the link. + PVE::Multipath::wait_for_map($wwid) + or die "multipath map for WWID '$wwid' did not assemble in time" + . " - check the multipath status of this node\n"; + } else { + my $path = $class->filesystem_path($scfg, $volname, $snapname); + warn "volume '$volname' of storage '$storeid': timed out waiting to confirm a multipath" + . " map; continuing normal volume activation - check this node's multipath status\n" + if !wait_for_path_map($scfg, $path); + } + } + my $path = $class->filesystem_path($scfg, $volname, $snapname); my $real_path = Cwd::realpath($path); die "failed to get realpath for '$path': $!\n" if !$real_path; diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index 723ed555..11a64c97 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -1083,11 +1083,17 @@ sub on_add_hook { die "unsupported base type '$basecfg->{type}'" if $basecfg->{type} ne 'iscsi'; + # Managed multipath requires the coalesced map, never a transient single-path PV. + die "base volume '$volname' does not reference a multipath map - retry in a few seconds" + . " once the map has assembled, or check the multipath status of this node\n" + if $basecfg->{multipath} && $volname !~ m/\.dm-uuid-mpath-\S+$/; + my $path = PVE::Storage::path($cfg, $base); PVE::Storage::activate_storage($cfg, $baseid); - # Activating the base volume verifies that it belongs to its target. + # the base's activate_volume checks target membership and, with managed multipath, waits + # for the map to assemble before the VG is created on it PVE::Storage::activate_volumes($cfg, [$base]); # a failing hook does not remove a volume group it created, so check the device first diff --git a/src/test/Makefile b/src/test/Makefile index 3517db6e..97b28929 100644 --- a/src/test/Makefile +++ b/src/test/Makefile @@ -1,6 +1,6 @@ all: test -test: test_iscsi test_zfspoolplugin test_lvmplugin test_lvm_create test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_lvm_status test_status_command test_disklist test_disk_api test_bwlimit test_plugin test_ovf test_volume_access test_multipath test_san_api test_san_libraries +test: test_zfspoolplugin test_lvmplugin test_lvm_create test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_lvm_status test_status_command test_disklist test_disk_api test_bwlimit test_plugin test_ovf test_volume_access test_multipath test_iscsi test_lun_identity test_multipath_api test_san_api test_san_libraries test_zfspoolplugin: run_test_zfspoolplugin.pl ./run_test_zfspoolplugin.pl @@ -52,16 +52,23 @@ test_volume_access: run_volume_access_tests.pl test_multipath: run_multipath_tests.pl ./run_multipath_tests.pl -test_san_api: run_san_api_tests.pl - ./run_san_api_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 +test_iscsi: run_iscsi_logout_tests.pl run_iscsi_activation_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_activation_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 +test_lun_identity: run_lun_identity_tests.pl + ./run_lun_identity_tests.pl + +test_multipath_api: run_multipath_api_tests.pl + ./run_multipath_api_tests.pl + +test_san_api: run_san_api_tests.pl + ./run_san_api_tests.pl + test_san_libraries: run_san_library_tests.pl ./run_san_library_tests.pl diff --git a/src/test/run_iscsi_activation_tests.pl b/src/test/run_iscsi_activation_tests.pl new file mode 100755 index 00000000..39a1a735 --- /dev/null +++ b/src/test/run_iscsi_activation_tests.pl @@ -0,0 +1,171 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use Test::MockModule; +use Test::More; + +use lib ('.', '..'); +use PVE::Storage::ISCSIPlugin; + +my $plugin = Test::MockModule->new('PVE::Storage::ISCSIPlugin'); +my $identity = Test::MockModule->new('PVE::Multipath::Identity'); +my $mp = Test::MockModule->new('PVE::Multipath'); +my $cwd = Test::MockModule->new('Cwd'); +my $hires = Test::MockModule->new('Time::HiRes'); +my ($now, $link_ready, $identity_ready, $map_ready, $identity_error, $id_cost, $map_cost); +my ($resolved, $target, $reads, $queries, $waits, $last_udev); +my @sleeps; +my $scfg = { multipath => 1, target => 'iqn.test' }; +my $volname = '0.0.1.scsi-not-the-wwid'; +$plugin->mock( + clock_gettime => sub { $now }, + filesystem_path => sub { '/dev/disk/by-id/legacy' }, + file_read_firstline => sub { + return 'mpath-identified' if $_[0] =~ m{/dm/uuid$}; + return $target if $_[0] =~ m{/targetname$}; + die "unexpected read $_[0]"; + }, + run_command => sub { + my ($cmd, %options) = @_; + die 'unexpected command' if $cmd->[0] ne 'udevadm'; + $last_udev = $cmd->[-1]; + $options{outfunc}->('/devices/platform/host1/session1/target1:0:0/1:0:0:1/block/sda'); + }, +); +$cwd->mock( + realpath => sub { + $reads++; + return $now >= $link_ready ? $resolved : undef; + }, +); +$hires->mock( + sleep => sub { + push @sleeps, [$now, $_[0]]; + $now += $_[0]; + }, +); +$identity->mock( + collect => sub { + my %param = @_; + is_deeply( + $param{targets}, + { 'iqn.test' => 1 }, + 'identity collection restricted to target', + ); + $queries++; + $now += $id_cost; + die "identity query failed\n" if $identity_error; + return {}; + }, + path_wwid => sub { + is($_[1], 'sda', 'resolved path, not alias suffix, identifies the LUN'); + return $now >= $identity_ready ? 'identified' : undef; + }, +); +$mp->mock( + wait_for_map => sub { + $waits++; + is($_[0], 'identified', 'identified WWID passed to map wait'); + is($_[1], 0, 'single map query per budget round') if @_ > 1; + $now += $map_cost; + return $now >= $map_ready ? { wwid => 'identified' } : undef; + }, +); +my sub reset_case { + ($now, $link_ready, $identity_ready, $map_ready, $identity_error, $id_cost, $map_cost) = + (0, 0, 0, 0, 0, 0, 0); + ($resolved, $target, $reads, $queries, $waits, $last_udev) = + ('/dev/sda', 'iqn.test', 0, 0, 0, undef); + @sleeps = (); +} +my sub activate { + my ($name, $config) = @_; + my $warnings = ''; + local $SIG{__WARN__} = sub { $warnings .= $_[0] }; + eval { PVE::Storage::ISCSIPlugin->activate_volume('san', $config // $scfg, $name // $volname) }; + return ($@, $warnings); +} + +for my $case ( + ['delayed link', sub { $link_ready = 3 }], + ['delayed identity', sub { $identity_ready = 3 }], + ['sd path identity', sub { }], +) { + reset_case(); + $case->[1]->(); + ok(PVE::Storage::ISCSIPlugin::wait_for_path_map($scfg, '/dev/disk/by-id/legacy'), $case->[0]); + ok($now <= 10, 'within polling budget'); +} +for my $case ( + ['failed identity queries', sub { $identity_error = 1 }, 10], + ['slow identity query', sub { $id_cost = 11 }, 11], + ['slow map query', sub { $id_cost = 4; $map_cost = 10; $map_ready = 100 }, 14], +) { + reset_case(); + $case->[1]->(); + ok( + !PVE::Storage::ISCSIPlugin::wait_for_path_map($scfg, '/dev/disk/by-id/legacy'), + "$case->[0] times out", + ); + is($now, $case->[2], 'budget plus in-flight query only'); + ok(!(grep { $_->[0] >= 10 } @sleeps), 'no sleep starts after budget expires'); + is($waits, 0, 'no map query starts after identity consumes budget') if $id_cost == 11; +} + +SKIP: { + # Only stat metadata of these existing nodes is used. All device queries are mocked. + skip 'no /dev/sda block node for activation validation', 10 if !-b '/dev/sda'; + reset_case(); + my ($err, $warn) = activate(); + is($err, '', 'successful map wait with alias still naming a member'); + is($warn, '', 'no warning after confirmation'); + is($last_udev, '/dev/sda', 'post-wait resolution validates the original alias'); + is($reads, 2, 'alias resolved again after wait'); + reset_case(); + $map_ready = 100; + ($err, $warn) = activate(); + is($err, '', 'timeout continues normal activation of a valid member'); + is( + $warn, + "volume '$volname' of storage 'san': timed out waiting to confirm a multipath map; continuing normal volume activation - check this node's multipath status\n", + 'exact compatibility warning', + ); + reset_case(); + $target = 'iqn.wrong'; + ($err, $warn) = activate(); + like($err, qr/not part of target/, 'wrong-target device still refused'); + reset_case(); + ($err, $warn) = activate(undef, { %$scfg, multipath => 0 }); + is($err, '', 'storage without multipath unchanged'); + is($waits + $queries, 0, 'no wait or identity query without multipath'); + reset_case(); + ($err, $warn) = activate('0.0.1.dm-uuid-mpath-identified'); + is($queries, 0, 'map-named volumes keep their direct map wait'); +} +reset_case(); +$link_ready = 100; +my ($err, $warn) = activate(); +like($err, qr/failed to get realpath/, 'permanently absent link still fails full activation'); +like($warn, qr/continuing normal volume activation/, 'absent link warns before normal validation'); +reset_case(); +$resolved = '/dev/null'; +($err, $warn) = activate(); +like($err, qr/not a block device/, 'non-block device still refused'); +reset_case(); +$map_ready = 100; +($err, $warn) = activate('0.0.1.dm-uuid-mpath-identified'); +like($err, qr/did not assemble in time/, 'map-named timeout remains fatal'); +SKIP: { + skip 'no /dev/dm-0 block node for map validation', 2 if !-b '/dev/dm-0'; + reset_case(); + $resolved = '/dev/dm-0'; + ok( + PVE::Storage::ISCSIPlugin::wait_for_path_map($scfg, '/dev/disk/by-id/legacy'), + 'alias already resolves to map', + ); + is($queries + $waits, 0, 'existing map needs no daemon query'); +} + +done_testing(); diff --git a/src/test/run_lun_identity_tests.pl b/src/test/run_lun_identity_tests.pl new file mode 100755 index 00000000..df248b38 --- /dev/null +++ b/src/test/run_lun_identity_tests.pl @@ -0,0 +1,557 @@ +#!/usr/bin/perl + +# Unit test for the identities that dm-multipath gives the LUNs of iSCSI sessions: the path list of +# multipathd, the NAA designator of the cached VPD page 0x83, the resolution of a LUN from its maps +# and the daemon's lines bound to its devices, the collection from sysfs (a fake tree below a +# temporary directory) and the iSCSI plugin's use of it. The daemon is mocked. + +use strict; +use warnings; + +use lib '..'; +use File::Path qw(make_path remove_tree); +use File::Temp qw(tempdir); +use Test::MockModule; +use Test::More; + +use PVE::Multipath; +use PVE::Multipath::Identity; +use PVE::Storage; +use PVE::Storage::ISCSIPlugin; +use PVE::Multipath::Status; + +my $T = 'iqn.1992-08.com.netapp:sn.1:vs.5'; +my $O = 'iqn.2003-01.org.linux-iscsi.other:sn.2'; + +# --- the daemon's path list --- + +my $paths = PVE::Multipath::parse_path_identities(join( + "\n", + "sda|8:0|0:0:0:0|ok|[undef]|0QEMU_QEMU_HARDDISK_drive-scsi0", + "sdb|8:16|7:0:0:0|ok|$T|3600a0980383235536724", + "sdc|8:32|8:0:0:0|udev-missing|$T|", + "-|#:#|#:#:#:#|undef|[undef]|", + "sdd|8:48|9:0:0:1|ok|$T|3600a0980dup", + "sdd|8:64|9:0:0:2|ok|$T|3600a0980dup", + '', +)); +is_deeply( + $paths, + { + sda => { + devt => '8:0', + hctl => '0:0:0:0', + init => 'ok', + target => '[undef]', + wwid => '0QEMU_QEMU_HARDDISK_drive-scsi0', + }, + sdb => { + devt => '8:16', + hctl => '7:0:0:0', + init => 'ok', + target => $T, + wwid => '3600a0980383235536724', + }, + sdc => + { devt => '8:32', hctl => '8:0:0:0', init => 'udev-missing', target => $T, wwid => '' }, + }, + 'path list: every path, placeholders kept, a name listed twice left out', +); +is_deeply(PVE::Multipath::parse_path_identities("ok\n"), {}, 'path list: no paths'); +for my $case ( + ["fail\n", 'a busy daemon'], + ["timeout\n", 'a timeout'], + ["multipath-tools v0.11.1\nCLI commands reference:\n list|show paths\n", 'the help text'], + ["sdb|8:16|7:0:0:0|ok|$T\n", 'a line without all fields'], + ["sdb|8:x|7:0:0:0|ok|$T|3600a\n", 'a malformed field'], +) { + is(PVE::Multipath::parse_path_identities($case->[0]), undef, "path list: none from $case->[1]"); +} + +# --- the NAA designator of the VPD page 0x83, as the kernel cached it for a NetApp LUN: T10 +# vendor, NAA and EUI-64 of the LUN, NAA of the target port, port and group numbers, SCSI name --- + +my $vpd = pack( + 'H*', + '0083009c020100204e45544150502020204c554e203832355367242f747a716b7a202020202020200103' + . '0010600a09803832355367242f747a716b7a01020010242f747a716b7a0000a0983832355367011300' + . '10600a098000000002ac16036a00000cbc011400040000000201150004000003e8032800286e61612e' + . '363030413039383033383332333535333637323432463734374137313642373900000000', +); +ok( + PVE::Multipath::Identity::vpd_has_naa($vpd, '600A09803832355367242F747A716B7A'), + 'VPD: the NAA designator of the logical unit', +); +ok( + !PVE::Multipath::Identity::vpd_has_naa($vpd, '600a098000000002ac16036a00000cbc'), + 'VPD: not the one of the target port', +); +ok( + !PVE::Multipath::Identity::vpd_has_naa($vpd, '600a09803832355367242f747a716c30'), + 'VPD: not another LUN', +); +ok( + !PVE::Multipath::Identity::vpd_has_naa(substr($vpd, 0, 60), '600a09803832355367242f747a716b7a'), + 'VPD: nothing from a truncated page', +); +{ + # the NAA designator of the LUN, then one that claims 32 bytes the page does not have + my $page = + pack('C C n', 0, 0x83, 24) + . pack('C C x C', 0x01, 0x03, 16) + . pack('H*', '600a09803832355367242f747a716b7a') + . pack('C C x C', 0x01, 0x02, 32); + ok( + !PVE::Multipath::Identity::vpd_has_naa($page, '600a09803832355367242f747a716b7a'), + 'VPD: nothing from a page whose designators do not add up, even after a match', + ); +} +ok(!PVE::Multipath::Identity::vpd_has_naa(undef, '600a'), 'VPD: nothing without a page'); + +# --- resolving LUNs --- + +my sub disk { + my ($lun, $n, %extra) = @_; + return ( + "sd$n" => { + target => $extra{target} // $T, + lun => $lun, + devt => "8:$n", + hctl => "$n:0:0:$lun", + defined($extra{map}) ? (map => $extra{map}) : (), + }, + ); +} + +my sub line { + my ($n, $lun, $wwid, %extra) = @_; + return ( + "sd$n" => { + devt => $extra{devt} // "8:$n", + hctl => $extra{hctl} // "$n:0:0:$lun", + init => $extra{init} // 'ok', + target => $extra{target} // $T, + wwid => $wwid, + }, + ); +} + +my $naa_checked = []; +my $naa_ok = sub { + my ($disk, $hex) = @_; + push $naa_checked->@*, $hex; + return $hex ne 'bad'; +}; + +my sub resolved { + my ($disks, $daemon) = @_; + my $luns = PVE::Multipath::Identity::resolve($disks, $daemon, $naa_ok); + return { map { $_->{lun} => $_->{wwid} } values %$luns }; +} + +is_deeply( + resolved({ disk(0, 1), disk(0, 2, map => '3W0'), disk(0, 3) }, undef), + { 0 => '3W0' }, + 'resolve: a map on a later path, the others not listed', +); +is_deeply( + resolved({ disk(0, 1, map => '3W0'), disk(0, 2, map => '3X0') }, undef), + { 0 => undef }, + 'resolve: maps that disagree', +); +is_deeply( + resolved({ disk(0, 1, map => '3W0'), disk(0, 2) }, { line(2, 0, '3X0') }), + { 0 => undef }, + 'resolve: a map and a path the daemon names differently', +); +is_deeply( + resolved({ disk(0, 1, map => '3W0'), disk(0, 2) }, { line(2, 0, '3W0') }), + { 0 => '3W0' }, + 'resolve: a map and a path the daemon names alike', +); +is_deeply( + resolved({ disk(1, 1), disk(1, 2) }, { line(1, 1, '3W1'), line(2, 1, '3W1') }), + { 1 => '3W1' }, + 'resolve: paths without a map that the daemon names alike', +); +is_deeply( + resolved({ disk(2, 1), disk(2, 2) }, { line(1, 2, '3W2'), line(2, 2, '3X2') }), + { 2 => undef }, + 'resolve: paths the daemon names differently', +); +is_deeply( + resolved({ disk(3, 1), disk(3, 2) }, { line(1, 3, '3W3') }), + { 3 => undef }, + 'resolve: a path the daemon does not list', +); + +for my $case ( + [{ init => 'udev-missing' }, 'a path the daemon did not initialize'], + [{ hctl => '9:0:0:4' }, 'a line of another SCSI device under a reused name'], + [{ devt => '8:99' }, 'a line of another block device'], + [{ target => $O }, 'a line of another target'], +) { + my ($extra, $what) = @$case; + is_deeply( + resolved({ disk(4, 1), disk(4, 2) }, { line(1, 4, '3W4'), line(2, 4, '3W4', %$extra) }), + { 4 => undef }, + "resolve: not with $what", + ); +} +is_deeply( + resolved({ disk(5, 1) }, { line(1, 5, '') }), + { 5 => undef }, + 'resolve: not with an empty identity', +); +is_deeply( + resolved({ disk(6, 1) }, { line(1, 6, '3bad') }), + { 6 => undef }, + 'resolve: not with an NAA identity the device does not report', +); +$naa_checked = []; +is_deeply( + resolved({ disk(7, 1) }, { line(1, 7, 'SNETAPP_LUN_825S') }), + { 7 => 'SNETAPP_LUN_825S' }, + 'resolve: an identity of another kind', +); +is_deeply($naa_checked, [], 'which is not checked against the VPD page'); +is_deeply( + resolved({ disk(0, 1, map => '3W0'), disk(8, 2) }, undef), + { 0 => '3W0', 8 => undef }, + 'resolve: without the daemon only LUNs with a map', +); +is_deeply( + { + map { ("$_->{target} $_->{lun}" => $_->{wwid}) } values PVE::Multipath::Identity::resolve( + { disk(9, 1), disk(9, 2, target => $O) }, + { line(1, 9, '3W9'), line(2, 9, '3X9', target => $O) }, + $naa_ok, + )->%* + }, + { "$T 9" => '3W9', "$O 9" => '3X9' }, + 'resolve: the same LUN number of another target is another LUN', +); + +for my $case ( + [{ disk(0, 1) }, undef, 1, 'daemon unavailable'], + [{ disk(0, 1) }, {}, 0, 'unlisted path'], + [{ disk(0, 1) }, { line(1, 0, '') }, 1, 'listed without identity'], + [{ disk(0, 1), disk(0, 2) }, { line(1, 0, 'A'), line(2, 0, 'B') }, 1, 'paths disagree'], + [{ disk(0, 1, map => 'A'), disk(0, 2, map => 'B') }, undef, 1, 'maps disagree'], + [{ disk(0, 1, map => 'A'), disk(0, 2) }, undef, 0, 'trusted map without daemon'], +) { + my ($disks, $daemon, $expected, $description) = @$case; + my ($lun) = values PVE::Multipath::Identity::resolve($disks, $daemon, $naa_ok)->%*; + is($lun->{unresolved} // 0, $expected, "uncertainty: $description"); +} + +# --- collecting from sysfs --- + +my $sys = tempdir(CLEANUP => 1); +$PVE::Multipath::Identity::SYSFS = $sys; + +my sub put { + my ($path, $content) = @_; + my $file = "$sys/$path"; + make_path($file =~ s|/[^/]+$||r); + open(my $fh, '>', $file) or die "$file: $!\n"; + print $fh $content; + 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. +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 { + my ($hctl) = @_; + my ($scsi_target) = $hctl =~ m/^(\d+:\d+:\d+):\d+$/; + my $parent = $session_dirs{$scsi_target} // return; + make_path("$sys/$parent/$hctl", "$sys/bus/scsi/devices"); + symlink("$sys/$parent/$hctl", "$sys/bus/scsi/devices/$hctl") + if !-l "$sys/bus/scsi/devices/$hctl"; +} + +# a disk as sysfs shows it: below its SCSI device, linked from class/block, with a link back +my sub disk_device { + my ($hctl, $name, $devt) = @_; + my $dir = "bus/scsi/devices/$hctl/block/$name"; + scsi_device($hctl); + put("bus/scsi/devices/$hctl/type", "0\n"); + put("$dir/dev", "$devt\n"); + make_path("$sys/$dir/holders", "$sys/class/block"); + unlink("$sys/$dir/device"); + symlink("$sys/bus/scsi/devices/$hctl", "$sys/$dir/device") or die "$!\n"; + unlink("$sys/class/block/$name"); + symlink("$sys/$dir", "$sys/class/block/$name") or die "$!\n"; +} + +# the name of a disk going to another SCSI device, as the kernel does when it reuses the name +my sub move_disk { + my ($from, $to, $name, $devt) = @_; + remove_tree("$sys/bus/scsi/devices/$from/block/$name"); + disk_device($to, $name, $devt); +} + +# a map holding a path, linked from the holders of the path +my sub hold { + my ($hctl, $name, $dm, $wwid) = @_; + put("devices/virtual/block/$dm/dm/uuid", "mpath-$wwid\n"); + symlink("$sys/devices/virtual/block/$dm", "$sys/bus/scsi/devices/$hctl/block/$name/holders/$dm") + or die "$!\n"; +} + +my sub wwids_of { + my ($ids) = @_; + return { + map { + my $wwid = PVE::Multipath::Identity::path_wwid($ids, $_); + defined($wwid) ? ($_ => $wwid) : () + } qw(sdb sdc sdd sde sdf) + }; +} + +# 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'); +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'); +hold('7:0:0:0', 'sdb', 'dm-0', '3600a09803832355367242f747a716b7a'); +# what /sys/block would show for dm-0 if the number went to another map meanwhile +put('block/dm-0/dm/uuid', "mpath-3DECOY\n"); +put('bus/scsi/devices/7:0:0:1/vpd_pg83', $vpd); +# a tape drive of the session does not count +scsi_device('7:0:0:9'); +put('bus/scsi/devices/7:0:0:9/type', "1\n"); + +my ($queries, $reply) = (0); +my $multipath = Test::MockModule->new('PVE::Multipath'); +$multipath->mock( + query_path_identities => sub { + $queries++; + die $reply if !ref($reply); + return $reply; + }, +); + +$reply = { line(9, 0, 'unused') }; +my $ids = PVE::Multipath::Identity::collect(targets => { $O => 1 }); +is($queries, 1, 'collect: the daemon is asked for a path without a map'); +$queries = 0; +hold('8:0:0:0', 'sdd', 'dm-1', '3OTHER'); +$ids = PVE::Multipath::Identity::collect(targets => { $O => 1 }); +is($queries, 0, 'collect: not if every path of the targets has a map'); +is_deeply(wwids_of($ids), { sdd => '3OTHER' }, 'collect: only the LUNs of the targets asked for'); + +$reply = { + sdc => { + devt => '8:32', + hctl => '7:0:0:1', + init => 'ok', + target => $T, + wwid => '3600a09803832355367242f747a716b7a', + }, +}; +$ids = PVE::Multipath::Identity::collect(targets => { $T => 1 }); +is_deeply( + { map { $_->{lun} => $_->{wwid} } PVE::Multipath::Identity::target_luns($ids, $T)->@* }, + { 0 => '3600a09803832355367242f747a716b7a', 1 => '3600a09803832355367242f747a716b7a' }, + 'collect: a map, and a path the daemon and the VPD page of the device name alike', +); +$reply->{sdc}->{wwid} = '3600a09803832355367242f747a716c30'; +$ids = PVE::Multipath::Identity::collect(targets => { $T => 1 }); +is_deeply( + { map { $_->{lun} => $_->{wwid} } PVE::Multipath::Identity::target_luns($ids, $T)->@* }, + { 0 => '3600a09803832355367242f747a716b7a', 1 => undef }, + 'collect: not a path whose VPD page names another LUN', +); +is($ids->{paths}->{sdc}->{unresolved}, 1, 'collect: rejected identity marks the path unresolved'); +$reply = {}; +$ids = PVE::Multipath::Identity::collect(targets => { $T => 1 }); +ok(!$ids->{paths}->{sdc}->{unresolved}, 'collect: genuinely unlisted path not marked unresolved'); +$reply = "multipathd returned an invalid path list\n"; +$ids = PVE::Multipath::Identity::collect(targets => { $T => 1 }); +is($ids->{paths}->{sdc}->{unresolved}, 1, 'collect: failed daemon marks unmapped path unresolved'); +ok(!$ids->{paths}->{sdb}->{unresolved}, 'collect: trusted map not marked unresolved'); +is($ids->{error}, 'multipathd returned an invalid path list', 'collect: a failed query is kept'); +is_deeply( + wwids_of($ids), + { sdb => '3600a09803832355367242f747a716b7a' }, + 'collect: and the LUNs with a map still resolve, through the holder link of their paths', +); + +# a later device under the name of a path: another LUN, the same dev_t +$reply = { + sdc => { + devt => '8:32', + hctl => '7:0:0:1', + init => 'ok', + target => $T, + wwid => '3600a09803832355367242f747a716b7a', + }, +}; +$ids = PVE::Multipath::Identity::collect(targets => { $T => 1 }); +is( + PVE::Multipath::Identity::path_wwid($ids, 'sdc'), + '3600a09803832355367242f747a716b7a', + 'path: the WWID of its LUN', +); +move_disk('7:0:0:1', '7:0:0:5', 'sdc', '8:32'); +is(PVE::Multipath::Identity::path_wwid($ids, 'sdc'), undef, 'path: not once the name is reused'); +is($ids->{paths}->{sdc}->{unresolved}, 1, 'path: changed H:C:T:L marks identity unresolved'); +move_disk('7:0:0:5', '7:0:0:1', 'sdc', '8:32'); +ok(!-e "$sys/bus/scsi/devices/7:0:0:5/block/sdc", 'and the fixture keeps one device per name'); + +$ids = PVE::Multipath::Identity::collect(targets => { $T => 1 }); +put('bus/scsi/devices/7:0:0:1/block/sdc/dev', "8:99\n"); +is(PVE::Multipath::Identity::path_wwid($ids, 'sdc'), undef, 'path: changed device number rejected'); +is($ids->{paths}->{sdc}->{unresolved}, 1, 'path: changed device number marks identity unresolved'); +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'); +# a second session to it on the same SCSI host, as offload initiators run them +session(4, $P, '9:0: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'); +($queries, $reply) = ( + 0, + { sdf => { devt => '8:80', hctl => '9:0:1:0', init => 'ok', target => $P, wwid => '3X9' } }, +); +$ids = PVE::Multipath::Identity::collect(targets => { $P => 1 }); +is($queries, 1, 'collect: the daemon is asked for a LUN that a map holds on one path only'); +is($ids->{paths}->{$_}->{unresolved}, 1, "collect: disagreeing LUN marks $_ unresolved") + for qw(sde sdf); +is_deeply( + wwids_of($ids), + {}, + 'collect: and the LUN unresolved if it names the other path differently', +); + +# 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'); + +for my $case ([$Q, '3WQ', 'sdg'], [$R, '3WR', 'sdh']) { + my ($target, $wwid, $path) = @$case; + $ids = PVE::Multipath::Identity::collect(targets => { $target => 1 }); + is_deeply( + [ + map { [$_->{wwid}, $_->{paths}] } + PVE::Multipath::Identity::target_luns($ids, $target)->@* + ], + [[$wwid, [$path]]], + "collect: a shared SCSI host keeps the LUN of $target with its target", + ); +} + +{ + # the daemon command itself fails + $multipath->unmock('query_path_identities'); + $multipath->mock( + is_supported => sub { 1 }, + run_command => sub { die "command 'multipathd show paths' failed: exit code 1\n" }, + ); + $ids = PVE::Multipath::Identity::collect(targets => { $P => 1 }); + like($ids->{error}, qr/exit code 1/, 'collect: a failed daemon command is kept'); + is_deeply( + wwids_of($ids), + { sde => '3W9', sdf => '3W9' }, + 'collect: and the LUN of the map, a path without a line not counting against it', + ); + $multipath->unmock('is_supported', 'run_command'); + $multipath->mock( + query_path_identities => sub { $queries++; die $reply if !ref($reply); return $reply }); +} + +# --- the iSCSI plugin --- + +$ids = { + luns => PVE::Multipath::Identity::resolve( + { + disk(0, 1, map => '3W0'), + disk(0, 2), + disk(1, 3), + disk(1, 4), + disk(2, 5, target => $O), + }, + { line(3, 1, '3W1'), line(4, 1, '3X1') }, + $naa_ok, + ), +}; +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', +); + +my $identity = Test::MockModule->new('PVE::Multipath::Identity'); +$identity->mock( + collect => sub { $ids }, + path_wwid => sub { + my ($ids, $name) = @_; + my ($lun) = grep { + grep { $_ eq $name } $_->{paths}->@* + } values $ids->{luns}->%*; + return $lun ? $lun->{wwid} : undef; + }, +); +$iscsi->mock( + iscsi_session_device_map => sub { + my $disks = + { disk(0, 1), disk(0, 2), disk(1, 3), disk(1, 4), disk(2, 5, target => $O) }; + return { + map { + $_ => { + target => $disks->{$_}->{target}, + channel => 0, + id => 0, + lun => $disks->{$_}->{lun}, + } + } + keys %$disks + }; + }, + load_stable_scsi_paths => sub { {} }, + file_read_firstline => sub { $_[0] =~ m|^/sys/block/\w+/size$| ? 2048 : undef }, +); +is_deeply( + [sort keys PVE::Storage::ISCSIPlugin::iscsi_device_list(1, $T)->{$T}->%*], + ['0.0.0.dm-uuid-mpath-3W0'], + 'volumes: every path of a resolved LUN named after its map, an unresolved LUN left out', +); + +my $collected = 0; +$identity->mock(collect => sub { $collected++; return { luns => {} } }); +my $derived = PVE::Storage::derived_multipath_wwids({ + ids => { + a => { type => 'iscsi', target => $T, portal => '192.0.2.10', multipath => 1 }, + b => { type => 'iscsi', target => $O, portal => '192.0.2.20', multipath => 1 }, + c => { type => 'iscsi', target => $O, portal => '192.0.2.20' }, + }, +}); +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'); + +done_testing(); diff --git a/src/test/run_multipath_api_tests.pl b/src/test/run_multipath_api_tests.pl new file mode 100755 index 00000000..c6fdfb7c --- /dev/null +++ b/src/test/run_multipath_api_tests.pl @@ -0,0 +1,92 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use lib ('.', '..'); +use Test::More; +use Test::MockModule; + +use PVE::Multipath; +use PVE::Storage; +use PVE::Storage::ISCSIPlugin; +use PVE::Multipath::Status; + +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'); + $inotify->mock(nodename => sub { 'n1' }); + $cluster->mock(get_nodelist => sub { ['n1', 'n2', 'n3'] }); + my $kv = { n2 => '{"supported":1}' }; + $cluster->mock(get_node_kv => sub { +{%$kv} }); + my $mp = Test::MockModule->new('PVE::Multipath'); + $mp->mock(is_supported => sub { 1 }); + + is_deeply( + PVE::Multipath::Status::nodes_without_status(), + ['n3'], + 'the local node is not waited for', + ); + my $storage = Test::MockModule->new('PVE::Storage'); + $storage->mock(config => sub { +{ ids => {} } }); + my $enable = sub { + my ($scfg, $update, $delete) = @_; + eval { + PVE::Storage::ISCSIPlugin->on_update_hook_full( + 'san', + { type => 'iscsi', %$scfg }, + $update, + $delete // [], + {}, + ); + }; + return $@; + }; + like( + $enable->({}, { multipath => 1 }), + qr/node\(s\) n3 did not report/, + 'enabling is refused', + ); + eval { PVE::Storage::ISCSIPlugin->on_add_hook('san', { type => 'iscsi', multipath => 1 }) }; + like($@, qr/did not report/, 'as is adding a storage with it'); + is($enable->({}, { content => 'none' }), '', 'other changes are not affected'); + + $kv->{n3} = '{"supported":0}'; + like( + $enable->({}, { multipath => 1 }), + qr/multipath-tools is not installed on node\(s\) n3/, + 'a node of the storage without multipath-tools refuses it too', + ); + is( + $enable->({ nodes => { n1 => 1, n2 => 1 } }, { multipath => 1 }), + '', + 'unless the storage is not available on that node', + ); + like( + $enable->({ nodes => { n1 => 1 } }, { multipath => 1 }, ['nodes']), + qr/not installed on node\(s\) n3/, + 'dropping the restriction in the same update counts', + ); + my $widen = sub { + my ($update, $delete) = @_; + return $enable->({ multipath => 1, nodes => { n1 => 1, n2 => 1 } }, $update, $delete); + }; + like( + $widen->({ nodes => { n1 => 1, n3 => 1 } }), + qr/not installed on node\(s\) n3/, + 'an enabled storage cannot be extended to a node without the tools', + ); + like($widen->({}, ['nodes']), qr/n3/, 'nor lose its node restriction'); + is($widen->({ nodes => { n1 => 1 } }), '', 'restricting it further is fine'); + + $kv->{n3} = '{"supported":1}'; + is($enable->({}, { multipath => 1 }), '', 'once every node supports it, it can be enabled'); + $kv->{n3} = 'not json'; + is_deeply( + PVE::Multipath::Status::nodes_without_status(), + ['n3'], + 'a broken status does not count', + ); +}; + +done_testing(); -- 2.47.3