From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 879311FF0AF for ; Fri, 06 Nov 2026 01:57:10 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 4F59D216B7; Fri, 06 Nov 2026 01:51:17 +0100 (CET) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [PATCH v3 storage 34/76] api: add endpoints for the cluster-wide multipath configuration Date: Fri, 6 Nov 2026 00:31:09 +0100 Message-ID: <20261105233336.2178064-35-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: 1793926179628 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.256 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) POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes 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: WAGQT2BSH7ASYAMLOF6IP762SBUSDOEB X-Message-ID-Hash: WAGQT2BSH7ASYAMLOF6IP762SBUSDOEB 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: Expose defaults, verbatim overrides, and explicit LUN entries under /cluster/multipath, with digests protecting against concurrent edits. Reject aliases matching another LUN's alias or WWID: multipathd drops one of two maps claiming the same name. Refuse entry removal or node exclusion while LVM uses the map there as a base volume, unless another configuration keeps it allow-listed. For LVM without a base volume, such as Fibre Channel setups, the generator's local checks keep the LUN allow-listed while its map holds the volume group. Adding entries or expanding node coverage requires cluster support and the necessary tools. Maintenance, narrowing, and removal remain available during upgrades. Signed-off-by: Thomas Lamprecht --- src/PVE/API2/Makefile | 1 + src/PVE/API2/Multipath.pm | 501 ++++++++++++++++++++++++++++ src/PVE/Multipath.pm | 24 ++ src/PVE/Multipath/Status.pm | 8 +- src/test/run_multipath_api_tests.pl | 162 ++++++++- src/test/run_multipath_tests.pl | 38 +++ 6 files changed, 720 insertions(+), 14 deletions(-) create mode 100644 src/PVE/API2/Multipath.pm diff --git a/src/PVE/API2/Makefile b/src/PVE/API2/Makefile index 10e18f95..9ab978b0 100644 --- a/src/PVE/API2/Makefile +++ b/src/PVE/API2/Makefile @@ -4,5 +4,6 @@ install: install -D -m 0644 Disks.pm ${DESTDIR}${PERLDIR}/PVE/API2/Disks.pm install -D -m 0644 ISCSI.pm ${DESTDIR}${PERLDIR}/PVE/API2/ISCSI.pm + install -D -m 0644 Multipath.pm ${DESTDIR}${PERLDIR}/PVE/API2/Multipath.pm make -C Storage install make -C Disks install diff --git a/src/PVE/API2/Multipath.pm b/src/PVE/API2/Multipath.pm new file mode 100644 index 00000000..595cb00b --- /dev/null +++ b/src/PVE/API2/Multipath.pm @@ -0,0 +1,501 @@ +package PVE::API2::Multipath; + +use strict; +use warnings; + +use PVE::Cluster; +use PVE::Exception qw(raise_param_exc); +use PVE::JSONSchema qw(get_standard_option); +use PVE::Storage; +use PVE::Tools qw(extract_param); + +use PVE::Multipath; +use PVE::Multipath::Config; +use PVE::Multipath::ClusterConfig; + +use PVE::RESTHandler; +use PVE::Multipath::Status; + +use base qw(PVE::RESTHandler); + +# multipathd resolves aliases against all maps; an alias colliding with another alias or WWID +# can make it drop a map. Include derived LUNs, which need not have config sections. +my sub assert_map_name_free { + my ($cfg, $wwid, $name) = @_; + + for my $other (sort keys $cfg->{ids}->%*) { + next if $other eq $wwid; + my $section = $cfg->{ids}->{$other}; + next if ($section->{type} // '') ne 'wwid'; + die "'$name' is already the map name of WWID '$other'\n" + if $other eq $name || (defined($section->{alias}) && $section->{alias} eq $name); + } + return if $name eq $wwid; + + my $reports = PVE::Multipath::Status::read_status_broadcasts(); + for my $node (sort keys %$reports) { + my $luns = $reports->{$node}->{luns} // next; + die "'$name' is the WWID of a LUN with a multipath map on node '$node'\n" if $luns->{$name}; + } +} + +# An undefined entry node set means removal, whereas an empty set covers every node. +my sub assert_consumers_covered { + my ($wwid, $entry_nodes) = @_; + + my $uncovered = PVE::Multipath::uncovered_consumer_nodes( + $wwid, + $entry_nodes, + PVE::Storage::config(), + PVE::Cluster::get_nodelist(), + ); + for my $storeid (sort keys %$uncovered) { + die "storage '$storeid' uses the multipath map of WWID '$wwid' on node(s) " + . join(', ', $uncovered->{$storeid}->@*) + . ", which would no longer allow-list the LUN\n"; + } +} + +my sub apply_section_update { + my ($section, $param, $delete, $settable) = @_; + + eval { PVE::Multipath::Config::apply_section_update($section, $param, $delete, $settable) }; + raise_param_exc({ delete => $@ }) if $@; +} + +# Caller must hold the config lock through the subsequent write. +my sub read_config_unlocked { + my ($digest) = @_; + + my $cfg = PVE::Multipath::ClusterConfig::read_config(); + PVE::Tools::assert_if_modified($cfg->{digest}, $digest); + return $cfg; +} + +my sub edit_config_locked { + my ($digest, $errmsg, $code) = @_; + + PVE::Multipath::ClusterConfig::lock_config( + sub { + my $cfg = read_config_unlocked($digest); + $code->($cfg); + PVE::Multipath::ClusterConfig::write_config($cfg); + }, + $errmsg, + ); +} + +# The global defaults section shares the WWID namespace and must not be accessible here. +my sub assert_valid_wwid { + my ($wwid) = @_; + + raise_param_exc({ wwid => "does not look like a valid WWID" }) + if !defined($wwid) + || $wwid !~ $PVE::Multipath::Config::WWID_RE + || $wwid eq 'defaults'; +} + +__PACKAGE__->register_method({ + name => 'index', + path => '', + method => 'GET', + description => "Directory index.", + permissions => { user => 'all' }, + parameters => { + additionalProperties => 0, + properties => {}, + }, + returns => { + type => 'array', + items => { + type => 'object', + properties => { + name => { type => 'string' }, + }, + }, + links => [{ rel => 'child', href => "{name}" }], + }, + code => sub { + return [map { +{ name => $_ } } qw(defaults overrides wwid)]; + }, +}); + +__PACKAGE__->register_method({ + name => 'read_overrides', + path => 'overrides', + method => 'GET', + description => "Read the verbatim multipath.conf override sections.", + permissions => { + check => ['perm', '/', ['Sys.Audit']], + }, + parameters => { + additionalProperties => 0, + properties => {}, + }, + returns => { + type => 'object', + properties => { + overrides => { + type => 'string', + description => 'Verbatim override sections.', + optional => 1, + }, + digest => get_standard_option('pve-config-digest'), + }, + }, + code => sub { + my $overrides = PVE::Multipath::ClusterConfig::read_overrides(); + return { + digest => PVE::Multipath::ClusterConfig::overrides_digest($overrides), + defined($overrides) && length($overrides) ? (overrides => $overrides) : (), + }; + }, +}); + +__PACKAGE__->register_method({ + name => 'set_overrides', + path => 'overrides', + method => 'PUT', + protected => 1, + description => "Set the verbatim multipath.conf override sections, such as " + . "hardware-specific 'device {}' or 'overrides {}' blocks.", + permissions => { + check => ['perm', '/', ['Sys.Modify']], + }, + parameters => { + additionalProperties => 0, + properties => { + overrides => { + type => 'string', + description => 'Verbatim override sections. Omit to clear them.', + maxLength => 128 * 1024, + optional => 1, + }, + digest => get_standard_option('pve-config-digest'), + }, + }, + returns => { type => 'null' }, + code => sub { + my ($param) = @_; + + my $overrides = $param->{overrides}; + my $digest = extract_param($param, 'digest'); + eval { PVE::Multipath::Config::check_overrides($overrides) }; + raise_param_exc({ overrides => $@ }) if $@; + + PVE::Multipath::ClusterConfig::lock_overrides( + sub { + my $current = PVE::Multipath::ClusterConfig::read_overrides(); + PVE::Tools::assert_if_modified( + PVE::Multipath::ClusterConfig::overrides_digest($current), $digest, + ); + PVE::Multipath::ClusterConfig::write_overrides($overrides // ''); + }, + "updating multipath overrides failed", + ); + return undef; + }, +}); + +__PACKAGE__->register_method({ + name => 'read_defaults', + path => 'defaults', + method => 'GET', + description => "Read the global multipathd settings applied on every node, including" + . " configured values and managed defaults.", + permissions => { + check => ['perm', '/', ['Sys.Audit']], + }, + parameters => { + additionalProperties => 0, + properties => {}, + }, + returns => { + type => 'object', + properties => { + PVE::Multipath::Config::defaults_api_schema()->%*, + digest => get_standard_option('pve-config-digest'), + }, + }, + code => sub { + my $cfg = PVE::Multipath::ClusterConfig::read_config(); + my $res = PVE::Multipath::Config::effective_defaults($cfg); + $res->{digest} = $cfg->{digest}; + return $res; + }, +}); + +__PACKAGE__->register_method({ + name => 'set_defaults', + path => 'defaults', + method => 'PUT', + protected => 1, + description => "Set the global multipathd defaults applied on every node.", + permissions => { + check => ['perm', '/', ['Sys.Modify']], + }, + parameters => { + additionalProperties => 0, + properties => { + PVE::Multipath::Config::defaults_api_schema()->%*, + delete => { + type => 'string', + format => 'pve-configid-list', + description => 'Comma-separated list of settings to reset to their defaults.', + optional => 1, + }, + digest => get_standard_option('pve-config-digest'), + }, + }, + returns => { type => 'null' }, + code => sub { + my ($param) = @_; + + my $delete = extract_param($param, 'delete'); + my $digest = extract_param($param, 'digest'); + my $settable = PVE::Multipath::Config::defaults_api_schema(); + + edit_config_locked( + $digest, + "updating multipath defaults failed", + sub { + my ($cfg) = @_; + my $section = $cfg->{ids}->{defaults} //= { type => 'defaults' }; + apply_section_update($section, $param, $delete, $settable); + # An empty defaults section keeps the generator engaged and has no removal endpoint. + delete $cfg->{ids}->{defaults} if !grep { $_ ne 'type' } keys %$section; + }, + ); + return undef; + }, +}); + +__PACKAGE__->register_method({ + name => 'wwid_index', + path => 'wwid', + method => 'GET', + description => "List the explicit multipath entries with their alias and per-LUN" + . " multipathd settings.", + permissions => { + check => ['perm', '/', ['Sys.Audit']], + }, + parameters => { + additionalProperties => 0, + properties => {}, + }, + returns => { + type => 'array', + items => { + type => 'object', + properties => { + wwid => { type => 'string', description => 'The LUN WWID.' }, + PVE::Multipath::Config::wwid_api_schema()->%*, + }, + }, + links => [{ rel => 'child', href => '{wwid}' }], + }, + code => sub { + my $cfg = PVE::Multipath::ClusterConfig::read_config(); + my $sections = PVE::Multipath::Config::wwid_sections($cfg); + return [map { { wwid => $_, $sections->{$_}->%* } } sort keys %$sections]; + }, +}); + +__PACKAGE__->register_method({ + name => 'read_wwid', + path => 'wwid/{wwid}', + method => 'GET', + description => "Read the explicit multipath entry of a WWID, with the digest of the" + . " configuration it was read from.", + permissions => { + check => ['perm', '/', ['Sys.Audit']], + }, + parameters => { + additionalProperties => 0, + properties => { + wwid => { + type => 'string', + description => 'The WWID to read.', + maxLength => 128, + }, + }, + }, + returns => { + type => 'object', + properties => { + wwid => { type => 'string', description => 'The LUN WWID.' }, + PVE::Multipath::Config::wwid_api_schema()->%*, + digest => get_standard_option('pve-config-digest'), + }, + }, + code => sub { + my ($param) = @_; + + my $wwid = $param->{wwid}; + assert_valid_wwid($wwid); + + my $cfg = PVE::Multipath::ClusterConfig::read_config(); + my $entry = PVE::Multipath::Config::wwid_sections($cfg)->{$wwid} + or die "WWID '$wwid' is not explicitly allow-listed\n"; + return { wwid => $wwid, $entry->%*, digest => $cfg->{digest} }; + }, +}); + +__PACKAGE__->register_method({ + name => 'add_wwid', + path => 'wwid', + method => 'POST', + protected => 1, + description => "Add an explicit multipath entry for a LUN, with an optional alias, node" + . " restriction, and multipathd settings. For a LUN of an iSCSI storage with multipath" + . " enabled, an entry is only needed to set an alias or per-LUN settings. The entry" + . " keeps the LUN managed even if the storage's multipath option is disabled.", + permissions => { + check => ['perm', '/', ['Sys.Modify']], + }, + parameters => { + additionalProperties => 0, + properties => { + wwid => { + type => 'string', + description => 'The WWID of the LUN to manage via multipath.', + maxLength => 128, + }, + PVE::Multipath::Config::wwid_api_schema()->%*, + digest => get_standard_option('pve-config-digest'), + }, + }, + returns => { type => 'null' }, + code => sub { + my ($param) = @_; + + my $wwid = extract_param($param, 'wwid'); + my $digest = extract_param($param, 'digest'); + my $settable = PVE::Multipath::Config::wwid_api_schema(); + assert_valid_wwid($wwid); + + edit_config_locked( + $digest, + "adding WWID '$wwid' failed", + sub { + my ($cfg) = @_; + raise_param_exc({ wwid => "WWID '$wwid' is already allow-listed" }) + if $cfg->{ids}->{$wwid}; + assert_map_name_free($cfg, $wwid, $wwid); + assert_map_name_free($cfg, $wwid, $param->{alias}) if defined($param->{alias}); + my $section = $cfg->{ids}->{$wwid} = { type => 'wwid' }; + apply_section_update($section, $param, undef, $settable); + PVE::Multipath::Status::assert_cluster_supports_multipath($section->{nodes}); + }, + ); + return undef; + }, +}); + +__PACKAGE__->register_method({ + name => 'update_wwid', + path => 'wwid/{wwid}', + method => 'PUT', + protected => 1, + description => "Update the alias, node restriction, and multipathd settings of an" + . " explicit multipath entry.", + permissions => { + check => ['perm', '/', ['Sys.Modify']], + }, + parameters => { + additionalProperties => 0, + properties => { + wwid => { + type => 'string', + description => 'The WWID to update.', + maxLength => 128, + }, + PVE::Multipath::Config::wwid_api_schema()->%*, + delete => { + type => 'string', + format => 'pve-configid-list', + description => 'Comma-separated list of settings to unset.', + optional => 1, + }, + digest => get_standard_option('pve-config-digest'), + }, + }, + returns => { type => 'null' }, + code => sub { + my ($param) = @_; + + my $wwid = extract_param($param, 'wwid'); + my $delete = extract_param($param, 'delete'); + my $digest = extract_param($param, 'digest'); + my $settable = PVE::Multipath::Config::wwid_api_schema(); + assert_valid_wwid($wwid); + + edit_config_locked( + $digest, + "updating WWID '$wwid' failed", + sub { + my ($cfg) = @_; + my $section = $cfg->{ids}->{$wwid} + or die "WWID '$wwid' is not explicitly allow-listed, add it first\n"; + assert_map_name_free($cfg, $wwid, $param->{alias}) if defined($param->{alias}); + my $old_nodes = $section->{nodes} // {}; + apply_section_update($section, $param, $delete, $settable); + my $nodes = $section->{nodes} // {}; + if (%$old_nodes && (!%$nodes || grep { !$old_nodes->{$_} } keys %$nodes)) { + PVE::Multipath::Status::assert_cluster_supports_multipath($nodes); + } + assert_consumers_covered($wwid, $section->{nodes} // {}) + if defined($param->{nodes}) + || grep { $_ eq 'nodes' } PVE::Tools::split_list($delete // ''); + }, + ); + return undef; + }, +}); + +__PACKAGE__->register_method({ + name => 'remove_wwid', + path => 'wwid/{wwid}', + method => 'DELETE', + protected => 1, + description => "Remove an explicit multipath entry from the multipath allow-list. A" + . " node keeps the LUN allow-listed while its map is still in use there.", + permissions => { + check => ['perm', '/', ['Sys.Modify']], + }, + parameters => { + additionalProperties => 0, + properties => { + wwid => { + type => 'string', + description => 'The WWID to remove.', + maxLength => 128, + }, + digest => get_standard_option('pve-config-digest'), + }, + }, + returns => { type => 'null' }, + code => sub { + my ($param) = @_; + + my $wwid = $param->{wwid}; + my $digest = extract_param($param, 'digest'); + assert_valid_wwid($wwid); + + edit_config_locked( + $digest, + "removing WWID '$wwid' failed", + sub { + my ($cfg) = @_; + die "WWID '$wwid' is not explicitly allow-listed - a LUN of a multipath-enabled" + . " storage is managed through that storage's multipath option\n" + if !$cfg->{ids}->{$wwid}; + assert_consumers_covered($wwid, undef); + delete $cfg->{ids}->{$wwid}; + }, + ); + return undef; + }, +}); + +1; diff --git a/src/PVE/Multipath.pm b/src/PVE/Multipath.pm index 96f1e84d..acca5a11 100644 --- a/src/PVE/Multipath.pm +++ b/src/PVE/Multipath.pm @@ -337,6 +337,30 @@ sub set_managed_wwid_record($record) { file_set_contents($MANAGED_WWIDS_FILE, PVE::Multipath::Config::format_managed_wwids($record)); } +# An undefined entry node set means removal, whereas an empty set covers every node. +sub uncovered_consumer_nodes($wwid, $entry_nodes, $storage_cfg, $all_nodes) { + my $ids = $storage_cfg->{ids} // {}; + my $node_set = sub($nodes) { + return $nodes && %$nodes ? {%$nodes} : { map { $_ => 1 } $all_nodes->@* }; + }; + my $entry = defined($entry_nodes) ? $node_set->($entry_nodes) : {}; + + my $res = {}; + for my $storeid (sort keys %$ids) { + my $scfg = $ids->{$storeid}; + next if ($scfg->{type} // '') ne 'lvm' || !defined($scfg->{base}); + my ($baseid, $volname) = $scfg->{base} =~ m/^([a-zA-Z][a-zA-Z0-9\-_.]*):(\S+)$/; + next if !defined($volname) || $volname !~ m/(?:^|\.)dm-uuid-mpath-\Q$wwid\E$/; + + my $base = $ids->{$baseid}; + my $derived = $base && $base->{multipath} ? $node_set->($base->{nodes}) : {}; + my @uncovered = + grep { !$entry->{$_} && !$derived->{$_} } sort keys $node_set->($scfg->{nodes})->%*; + $res->{$storeid} = \@uncovered if @uncovered; + } + return $res; +} + sub reconfigure() { assert_supported(); run_command([$MULTIPATHD, 'reconfigure'], timeout => $RECONFIGURE_TIMEOUT); diff --git a/src/PVE/Multipath/Status.pm b/src/PVE/Multipath/Status.pm index cc8fe49d..67cc42b9 100644 --- a/src/PVE/Multipath/Status.pm +++ b/src/PVE/Multipath/Status.pm @@ -233,9 +233,7 @@ sub nodes_without_tools($nodes) { 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" + die "multipath-tools is not installed on node(s) " . join(', ', $no_tools->@*) . "\n" if scalar($no_tools->@*); return; } @@ -244,8 +242,8 @@ 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" + . " have not reported multipath support - upgrade every node and" + . " bring it online before enabling multipath\n" if scalar($silent->@*); assert_multipath_tools($nodes); return; diff --git a/src/test/run_multipath_api_tests.pl b/src/test/run_multipath_api_tests.pl index 3f0c8a92..055ade62 100755 --- a/src/test/run_multipath_api_tests.pl +++ b/src/test/run_multipath_api_tests.pl @@ -16,7 +16,9 @@ use Storable qw(dclone); use Test::More; use Test::MockModule; +use PVE::API2::Multipath; use PVE::Multipath; +use PVE::Multipath::Config; use PVE::Multipath::Generator; use PVE::Storage; use PVE::Storage::ISCSIPlugin; @@ -128,17 +130,17 @@ subtest 'enabling the multipath option needs every node to know it' => sub { }; like( $enable->({}, { multipath => 1 }), - qr/node\(s\) n3 did not report/, + qr/node\(s\) n3 have not reported/, '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'); + like($@, qr/have not reported/, 'as is adding a storage with it'); is($enable->({}, { content => 'none' }), '', 'other changes are not affected'); $kv->{n3} = '{"supported":0}'; - like( + is( $enable->({}, { multipath => 1 }), - qr/multipath-tools is not installed on node\(s\) n3/, + "multipath-tools is not installed on node(s) n3\n", 'a node of the storage without multipath-tools refuses it too', ); is( @@ -146,21 +148,25 @@ subtest 'enabling the multipath option needs every node to know it' => sub { '', 'unless the storage is not available on that node', ); - like( + is( $enable->({ nodes => { n1 => 1 } }, { multipath => 1 }, ['nodes']), - qr/not installed on node\(s\) n3/, + "multipath-tools is not installed on node(s) n3\n", '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( + is( $widen->({ nodes => { n1 => 1, n3 => 1 } }), - qr/not installed on node\(s\) n3/, + "multipath-tools is not installed on node(s) n3\n", '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']), + "multipath-tools is not installed on node(s) n3\n", + 'nor lose its node restriction', + ); is($widen->({ nodes => { n1 => 1 } }), '', 'restricting it further is fine'); $kv->{n3} = '{"supported":1}'; @@ -458,6 +464,144 @@ subtest 'regeneration prunes only conclusively closed blank maps' => sub { } }; +my sub mock_config { + my ($raw_ref, $storage_cfg) = @_; + + my $cc = Test::MockModule->new('PVE::Multipath::ClusterConfig'); + $cc->mock( + read_config => sub { PVE::Multipath::Config->parse_config('multipath.cfg', $$raw_ref) }, + ); + $cc->mock(write_config => + sub { $$raw_ref = PVE::Multipath::Config->write_config('multipath.cfg', $_[0]) }); + $cc->mock(lock_config => sub { $_[0]->() }); + my $storage = Test::MockModule->new('PVE::Storage'); + $storage->mock(config => sub { dclone($storage_cfg) }); + my $cluster = Test::MockModule->new('PVE::Cluster'); + $cluster->mock(get_nodelist => sub { ['n1', 'n2'] }); + my $mp = Test::MockModule->new('PVE::Multipath'); + my $status = Test::MockModule->new('PVE::Multipath::Status'); + $status->mock(read_status_broadcasts => sub { {} }); + return [$cc, $storage, $cluster, $mp]; +} + +subtest 'the configuration keeps the LUNs storages use allow-listed' => sub { + my $raw = "wwid: 3600abc\n\talias lun-a\n"; + my $storage_cfg = { + ids => { + san => { type => 'iscsi', nodes => { n1 => 1 } }, + vg => { type => 'lvm', base => 'san:0.0.1.dm-uuid-mpath-3600abc', vgname => 'vg' }, + }, + }; + my $mocks = mock_config(\$raw, $storage_cfg); + + eval { PVE::API2::Multipath->remove_wwid({ wwid => '3600abc' }) }; + like($@, qr/storage 'vg' uses the multipath map .* on node\(s\) n1, n2/, 'removal refused'); + like($raw, qr/3600abc/, 'and the entry is kept'); + + eval { PVE::API2::Multipath->update_wwid({ wwid => '3600abc', nodes => 'n1' }) }; + like($@, qr/on node\(s\) n2,/, + 'restricting the entry away from a consumer node is refused'); + unlike($raw, qr/nodes/, 'and leaves the entry as it was'); + + PVE::API2::Multipath->update_wwid({ wwid => '3600abc', alias => 'lun-b' }); + like($raw, qr/alias lun-b/, 'other changes go through'); + + $storage_cfg->{ids}->{san} = { type => 'iscsi', multipath => 1 }; + $mocks = mock_config(\$raw, $storage_cfg); + PVE::API2::Multipath->update_wwid({ wwid => '3600abc', nodes => 'n1' }); + like($raw, qr/nodes n1/, 'a base storage deriving the LUN covers the other nodes'); + PVE::API2::Multipath->remove_wwid({ wwid => '3600abc' }); + unlike($raw, qr/3600abc/, 'and lets the entry go'); +}; + +subtest 'a WWID that multipath would misread is refused' => sub { + my $raw = ''; + my $mocks = mock_config(\$raw, { ids => {} }); + my ($cc) = @$mocks; + my $writes = 0; + $cc->mock(write_config => sub { $writes++ }); + for my $wwid ('-W', '-F', '8:0') { + eval { PVE::API2::Multipath->add_wwid({ wwid => $wwid }) }; + like($@, qr/does not look like a valid WWID/, "'$wwid' is refused"); + } + is($writes, 0, 'without writing the configuration'); +}; + +subtest 'WWID enablement checks support before writing' => sub { + my $raw = ''; + my $mocks = mock_config(\$raw, { ids => {} }); + my ($cc, $storage, $cluster, $mp) = @$mocks; + my $writes = 0; + $cc->mock( + write_config => sub { + $writes++; + $raw = PVE::Multipath::Config->write_config('multipath.cfg', $_[0]); + }, + ); + my $inotify = Test::MockModule->new('PVE::INotify'); + $inotify->mock(nodename => sub { 'n1' }); + my $kv = {}; + $cluster->mock(get_node_kv => sub { $kv }); + my $supported = 1; + $mp->mock(is_supported => sub { $supported }); + my $support_error = "node(s) n2 have not reported multipath support - upgrade every node" + . " and bring it online before enabling multipath\n"; + for my $report (undef, 'old-node', '[]') { + $kv = defined($report) ? { n2 => $report } : {}; + eval { PVE::API2::Multipath->add_wwid({ wwid => 'w1', nodes => 'n1' }) }; + is($@, $support_error, 'silent or incompatible peer prevents a new entry'); + is($writes, 0, 'refusal does not write configuration'); + } + $kv = { n2 => '{"supported":0}' }; + eval { PVE::API2::Multipath->add_wwid({ wwid => 'w1' }) }; + is($@, "multipath-tools is not installed on node(s) n2\n", 'remote tools required'); + is($writes, 0, 'missing remote tools never write'); + $supported = 0; + eval { PVE::API2::Multipath->add_wwid({ wwid => 'w1', nodes => 'n1' }) }; + is($@, "multipath-tools is not installed on node(s) n1\n", 'local tools required'); + is($writes, 0, 'missing local tools never write'); + eval { PVE::API2::Multipath->add_wwid({ wwid => 'w1' }) }; + is( + $@, + "multipath-tools is not installed on node(s) n1, n2\n", + 'all affected nodes are listed', + ); + is($writes, 0, 'missing tools on multiple nodes never write'); + $supported = 1; + PVE::API2::Multipath->add_wwid({ wwid => 'w1', nodes => 'n1' }); + is($writes, 1, 'supported restricted entry can be added'); + + for my $report (undef, '{"supported":0}') { + $kv = defined($report) ? { n2 => $report } : {}; + for my $param ({ nodes => 'n1,n2' }, { delete => 'nodes' }) { + eval { PVE::API2::Multipath->update_wwid({ wwid => 'w1', %$param }) }; + is( + $@, + defined($report) + ? "multipath-tools is not installed on node(s) n2\n" + : $support_error, + 'widening coverage is refused', + ); + is($writes, 1, 'refused expansion does not write'); + } + } + $kv = {}; + PVE::API2::Multipath->update_wwid({ + wwid => 'w1', + alias => 'lun', + 'no-path-retry' => 'fail', + }); + is($writes, 2, 'maintenance works with silent peers'); + $kv = { n2 => '{"supported":1}' }; + PVE::API2::Multipath->update_wwid({ wwid => 'w1', nodes => 'n1,n2' }); + is($writes, 3, 'supported expansion works'); + $kv = {}; + PVE::API2::Multipath->update_wwid({ wwid => 'w1', nodes => 'n1' }); + is($writes, 4, 'narrowing works during upgrades'); + PVE::API2::Multipath->remove_wwid({ wwid => 'w1' }); + is($writes, 5, 'removal works during upgrades'); +}; + subtest 'unreadable configuration keeps the known LUNs reported' => sub { my $mp = Test::MockModule->new('PVE::Multipath'); $mp->mock(managed_wwid_record => sub { +{ '3600abc' => ['storage:san'] } }); diff --git a/src/test/run_multipath_tests.pl b/src/test/run_multipath_tests.pl index 1c28ac86..94683a50 100755 --- a/src/test/run_multipath_tests.pl +++ b/src/test/run_multipath_tests.pl @@ -740,4 +740,42 @@ is_deeply( 'but how many LUNs are in which state', ); +# --- consumers the configuration would leave without an allow-list entry --- +my $scfg = { + ids => { + san => { type => 'iscsi', multipath => 1, nodes => { n1 => 1, n2 => 1 } }, + plain => { type => 'iscsi' }, + shared => { type => 'lvm', base => 'san:0.0.1.dm-uuid-mpath-w1' }, + other => + { type => 'lvm', base => 'plain:0.0.2.dm-uuid-mpath-w2', nodes => { n1 => 1 } }, + local => { type => 'lvm', vgname => 'vg0' }, + }, +}; +my $all = ['n1', 'n2', 'n3']; +is_deeply( + PVE::Multipath::uncovered_consumer_nodes('w1', undef, $scfg, $all), + { shared => ['n3'] }, + 'removing the entry uncovers the nodes the deriving storage does not reach', +); +is_deeply( + PVE::Multipath::uncovered_consumer_nodes('w1', {}, $scfg, $all), + {}, + 'an unrestricted entry covers every node', +); +is_deeply( + PVE::Multipath::uncovered_consumer_nodes('w1', { n3 => 1 }, $scfg, $all), + {}, + 'an entry for the remaining node covers it', +); +is_deeply( + PVE::Multipath::uncovered_consumer_nodes('w2', { n2 => 1 }, $scfg, $all), + { other => ['n1'] }, + 'a base without the multipath option derives nothing', +); +is_deeply( + PVE::Multipath::uncovered_consumer_nodes('w3', undef, $scfg, $all), + {}, + 'a LUN nothing uses has no consumers to uncover', +); + done_testing(); -- 2.47.3