public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
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	[thread overview]
Message-ID: <20261105233336.2178064-35-t.lamprecht@proxmox.com> (raw)
In-Reply-To: <20261105233336.2178064-1-t.lamprecht@proxmox.com>

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 <t.lamprecht@proxmox.com>
---
 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




  parent reply	other threads:[~2026-11-06  0:57 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-05 23:30 [PATCH v3 proxmox,proxmox-perl-rs,cluster,storage,docs,manager 00/76] SAN: cluster-wide multipath, SAN views and a guided storage setup Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 proxmox 01/76] add proxmox-multipath crate Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 proxmox-perl-rs 02/76] pve-rs: add PVE::RS::Multipath bindings for multipathd status parsing Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 cluster 03/76] cfs: observe the multipath configuration files Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 04/76] storage: distinguish status-only activation in plugins Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 05/76] lvm: find a shared volume group created by another node Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 06/76] storage: bound LVM status commands Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 07/76] iscsi: rate-limit session rescans per target Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 08/76] iscsi: list the LUNs of each session, not of its SCSI host Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 09/76] lvm: refuse to create a volume group on a device that holds data Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 10/76] lvm: refuse a base volume of a target that serves disk images Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 11/76] iscsi: refuse virtual devices other than multipath maps as volumes Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 12/76] diskmanage: collect disk transport type from lsblk Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 13/76] diskmanage: add helper to list multipath devices Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 14/76] diskmanage: qualify NVMe over fabrics transport Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 15/76] disks: list: add include-remote parameter Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 16/76] diskmanage: include iSCSI session devices in disk enumeration Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 17/76] diskmanage: link multipath member disks to their map device Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 18/76] iscsi: factor out session device map from device list Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 19/76] api: scan: add a SAN LUN inventory Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 20/76] pvesm: expose the " Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 21/76] disks: lvm: allow volume groups on SAN LUNs and multipath maps Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 22/76] multipath: add a library around multipathd Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 23/76] diskmanage: collect native NVMe multipath path state Thomas Lamprecht
2026-11-05 23:30 ` [PATCH v3 storage 24/76] api: scan: san-luns: report " Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 25/76] diskmanage: collect local block device users Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 26/76] api: add node-level iSCSI status and maintenance Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 27/76] api: iscsi: add session logout with last-path protection Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 28/76] multipath: add a cluster-wide configuration Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 29/76] iscsi: add an option to manage storage LUNs with multipath Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 30/76] lvm: activate a missing base volume before using the volume group Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 31/76] multipath: apply the configuration on every node Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 32/76] iscsi: share the initiator identity reader Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 33/76] multipath: publish per-node status Thomas Lamprecht
2026-11-05 23:31 ` Thomas Lamprecht [this message]
2026-11-05 23:31 ` [PATCH v3 storage 35/76] api: multipath: add the cluster-wide health status Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 36/76] multipath: collect identities of non-iSCSI SCSI paths Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 37/76] api: scan: san-luns: report the multipath state of each LUN Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 38/76] api: scan: flag duplicate physical volumes on distinct LUNs Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 39/76] api: disks: recheck local users and retain wipe device references Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 40/76] iscsi: name plain LUNs after their identity link Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 41/76] api: disks: guard SAN wipes with stable identities and storage checks Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 storage 42/76] api: scan: refresh SAN usage with bounded probes Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 docs 43/76] storage: lvm: note the rescan after extending a shared volume group Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 docs 44/76] storage: document the cluster-wide multipath management Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 45/76] api: ceph: osd: keep iSCSI LUNs out of OSD creation Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 46/76] pvestatd: track pending storage status commands across re-exec Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 47/76] ui: storage: factor out the scan picker error mask Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 48/76] ui: storage: nfs: show scan errors and empty results Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 49/76] ui: storage: allow changing the node for NFS/CIFS scans Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 50/76] ui: storage: add guided remote storage wizard with NFS support Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 51/76] ui: storage wizard: add SMB/CIFS support Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 52/76] ui: storage wizard: preserve settings after partial setup Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 53/76] ui: storage wizard: add Fibre Channel shared LVM setup Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 54/76] ui: storage wizard: reconcile unanswered storage creation Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 55/76] ui: storage wizard: add ZFS over iSCSI support Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 56/76] ui: dc: storage: add remote storage wizard entry to the add menu Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 57/76] ui: node: add SAN LUNs panel Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 58/76] ui: san luns: show multipath path state Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 59/76] api: nodes: expose iSCSI status and maintenance Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 60/76] pvenode: add iscsi commands Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 61/76] ui: node: san: add iSCSI target and session management Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 62/76] pvestatd: apply the multipath configuration and report the node status Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 63/76] api: cluster: add the multipath endpoints Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 64/76] ui: dc: add a cluster-wide multipath view Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 65/76] ui: storage: iscsi: add the multipath option Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 66/76] ui: san luns: show multipath state and manage LUN entries Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 67/76] ui: storage wizard: add iSCSI support Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 68/76] ui: storage wizard: iscsi: show cluster initiator names Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 69/76] ui: storage wizard: iscsi: enable multipath for shared LVM Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 70/76] ui: node: san: wipe a LUN that no storage uses Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 71/76] ui: storage wizard: iscsi: reuse existing target connections Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 72/76] ui: storage wizard: fibre channel: enable multipath for a LUN Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 73/76] ui: storage wizard: let root@pam wipe a LUN with data Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 74/76] ui: storage wizard: do not offer LUNs in use for a new volume group Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 75/76] ui: san: show the local users of a LUN in its usage tooltip Thomas Lamprecht
2026-11-05 23:31 ` [PATCH v3 manager 76/76] ui: san: check the other nodes for users before wiping a LUN Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261105233336.2178064-35-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal