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 28/76] multipath: add a cluster-wide configuration
Date: Fri,  6 Nov 2026 00:31:03 +0100	[thread overview]
Message-ID: <20261105233336.2178064-29-t.lamprecht@proxmox.com> (raw)
In-Reply-To: <20261105233336.2178064-1-t.lamprecht@proxmox.com>

Per-node multipath configuration can drift, leaving some nodes using
individual paths while others use maps. Keep the allow-list, map names,
and daemon settings in the cluster file system instead.

Provide global defaults, per-LUN entries with aliases and node
restrictions, and a verbatim file for hardware-specific settings.
Use find_multipaths strict to map only allow-listed LUNs, WWID-based
names for consistency across nodes, and auto_resize grow_only so maps
follow target-side LUN growth before the PV is enlarged.

Place global settings also supplied by device profiles, such as
no_path_retry, in an overrides section so the profiles cannot supersede
them. Applying the configuration is the generator's responsibility.

Refuse entries whose WWID multipath would read as an option or a
device number, also in a hand-edited file.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/Makefile                   |   2 +
 src/PVE/Multipath/ClusterConfig.pm |  64 ++++
 src/PVE/Multipath/Config.pm        | 451 +++++++++++++++++++++++++++++
 src/test/run_multipath_tests.pl    | 255 ++++++++++++++++
 src/test/run_san_library_tests.pl  |  36 +++
 5 files changed, 808 insertions(+)
 create mode 100644 src/PVE/Multipath/ClusterConfig.pm
 create mode 100644 src/PVE/Multipath/Config.pm

diff --git a/src/PVE/Makefile b/src/PVE/Makefile
index 91f22282..b92059bd 100644
--- a/src/PVE/Makefile
+++ b/src/PVE/Makefile
@@ -5,6 +5,8 @@ install:
 	install -D -m 0644 Storage.pm ${DESTDIR}${PERLDIR}/PVE/Storage.pm
 	install -D -m 0644 Diskmanage.pm ${DESTDIR}${PERLDIR}/PVE/Diskmanage.pm
 	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 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/ClusterConfig.pm b/src/PVE/Multipath/ClusterConfig.pm
new file mode 100644
index 00000000..5d5fa765
--- /dev/null
+++ b/src/PVE/Multipath/ClusterConfig.pm
@@ -0,0 +1,64 @@
+package PVE::Multipath::ClusterConfig;
+
+use v5.36;
+
+use Digest::SHA ();
+
+use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
+
+use PVE::Multipath::Config;
+
+my $FILENAME = 'multipath.cfg';
+my $OVERRIDES_FILENAME = 'multipath-overrides.conf';
+
+cfs_register_file(
+    $FILENAME,
+    sub($filename, $data) { PVE::Multipath::Config->parse_config($filename, $data); },
+    sub($filename, $data) { PVE::Multipath::Config->write_config($filename, $data); },
+);
+
+cfs_register_file(
+    $OVERRIDES_FILENAME,
+    \&PVE::Multipath::Config::parse_overrides,
+    \&PVE::Multipath::Config::write_overrides,
+);
+
+sub read_config() {
+    return cfs_read_file($FILENAME);
+}
+
+sub write_config($cfg) {
+    cfs_write_file($FILENAME, $cfg);
+}
+
+sub read_overrides() {
+    return cfs_read_file($OVERRIDES_FILENAME);
+}
+
+sub write_overrides($text) {
+    cfs_write_file($OVERRIDES_FILENAME, $text);
+}
+
+# Digest over the raw override text, to detect concurrent modifications; the SectionConfig digest
+# of multipath.cfg does not cover this separate file.
+sub overrides_digest($text) {
+    return Digest::SHA::sha1_hex($text // '');
+}
+
+# Preserve exception objects so parameter errors retain their API error type.
+my sub rethrow($err, $errmsg) {
+    die $err if ref($err) || !$errmsg;
+    die "$errmsg - $err";
+}
+
+sub lock_config($code, $errmsg = undef) {
+    cfs_lock_file($FILENAME, undef, $code);
+    rethrow($@, $errmsg) if $@;
+}
+
+sub lock_overrides($code, $errmsg = undef) {
+    cfs_lock_file($OVERRIDES_FILENAME, undef, $code);
+    rethrow($@, $errmsg) if $@;
+}
+
+1;
diff --git a/src/PVE/Multipath/Config.pm b/src/PVE/Multipath/Config.pm
new file mode 100644
index 00000000..4d9901e0
--- /dev/null
+++ b/src/PVE/Multipath/Config.pm
@@ -0,0 +1,451 @@
+package PVE::Multipath::Config;
+
+use v5.36;
+
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::SectionConfig;
+use PVE::Tools;
+
+use base qw(PVE::SectionConfig);
+
+# Strict discovery leaves unrelated and boot disks alone; WWID names avoid node-local bindings.
+# Grow-only resizing follows target expansion without shrinking a map under its consumers.
+my $MANAGED_DEFAULTS = {
+    'find-multipaths' => 'strict',
+    'user-friendly-names' => 'no',
+    'polling-interval' => 5,
+    'auto-resize' => 'grow_only',
+};
+
+sub managed_defaults() { return { $MANAGED_DEFAULTS->%* }; }
+
+# multipath reads an argument starting with '-' as an option and one starting with
+# '<major>:<minor>' as a device number, so a WWID must not start with either.
+our $WWID_PATTERN = '(?!-|\d+:-?\d)[a-zA-Z0-9._:-]+';
+our $WWID_RE = qr/^(?:$WWID_PATTERN)\z/;
+
+my $shared_knobs = {
+    'no-path-retry' => {
+        type => 'string',
+        pattern => '(?:queue|fail|\d+)',
+        typetext => 'queue|fail|<count>',
+        description =>
+            "How to react when all paths are down: keep queuing, fail at once, or retry"
+            . " for the given number of polling intervals.",
+        optional => 1,
+    },
+    'path-grouping-policy' => {
+        type => 'string',
+        enum =>
+            [qw(failover multibus group_by_serial group_by_prio group_by_node_name group_by_tpg)],
+        description => "How paths are grouped into priority groups. 'group_by_tpg' groups by"
+            . " the ALUA target port group and needs the alua prioritizer, which the hardware"
+            . " table of multipath-tools selects for arrays that support it.",
+        optional => 1,
+    },
+    failback => {
+        type => 'string',
+        pattern => '(?:manual|immediate|followover|\d+)',
+        typetext => 'manual|immediate|followover|<seconds>',
+        description => "When to fail back to a restored higher-priority path group.",
+        optional => 1,
+    },
+    'path-selector' => {
+        type => 'string',
+        pattern =>
+            '(?:round-robin|queue-length|service-time|historical-service-time|io-affinity) \d+',
+        typetext => '<selector> <number>',
+        description => "Path selector algorithm used within a priority group, for example"
+            . " 'service-time 0'.",
+        optional => 1,
+    },
+};
+
+my $defaults_only_knobs = {
+    'find-multipaths' => {
+        type => 'string',
+        enum => [qw(yes no strict greedy smart)],
+        default => 'strict',
+        description => "Which devices multipathd assembles into a map. 'strict' only uses WWIDs"
+            . " in the node's allow-list, including those added for multipath-enabled storages.",
+        optional => 1,
+    },
+    'user-friendly-names' => {
+        type => 'string',
+        enum => [qw(yes no)],
+        default => 'no',
+        description => "Whether to use node-local mpathN names. Keep 'no' for stable WWID-based"
+            . " names across the cluster.",
+        optional => 1,
+    },
+    'polling-interval' => {
+        type => 'integer',
+        minimum => 1,
+        default => 5,
+        description => "Interval between path checks, in seconds.",
+        optional => 1,
+    },
+    'auto-resize' => {
+        type => 'string',
+        enum => [qw(never grow_only grow_shrink)],
+        default => 'grow_only',
+        description => "Whether multipathd resizes a map once all its paths report a new size."
+            . " 'grow_only' follows a LUN grown on the target, 'never' leaves it to"
+            . " 'multipathd resize map'.",
+        optional => 1,
+    },
+};
+
+my $wwid_only_knobs = {
+    alias => {
+        type => 'string',
+        pattern => '[a-zA-Z0-9][a-zA-Z0-9._-]*',
+        maxLength => 64,
+        description => "Human-readable name that multipathd uses for this WWID's map.",
+        optional => 1,
+    },
+    'rr-min-io-rq' => {
+        type => 'integer',
+        minimum => 1,
+        description => "Number of I/O requests to send through a path before switching to"
+            . " another path, for request-based multipath devices.",
+        optional => 1,
+    },
+    'rr-weight' => {
+        type => 'string',
+        enum => [qw(priorities uniform)],
+        description => "Whether to weight paths by priority when balancing I/O.",
+        optional => 1,
+    },
+};
+
+# Per-LUN settings that are not multipathd knobs and stay out of the rendered configuration.
+my $wwid_only_props = {
+    nodes => get_standard_option(
+        'pve-node-list',
+        {
+            description => "The nodes that see this LUN and are expected to assemble its map."
+                . " Other nodes leave it off their allow-list. Unset means every node.",
+            optional => 1,
+        },
+    ),
+};
+
+my $defaultData = {
+    propertyList => {
+        type => { description => "Section type ('defaults' or 'wwid')." },
+        id => {
+            type => 'string',
+            description =>
+                "Section ID: the literal 'defaults', or a LUN WWID for 'wwid' sections.",
+            pattern => $WWID_PATTERN,
+            maxLength => 128,
+        },
+        $shared_knobs->%*,
+        $defaults_only_knobs->%*,
+        $wwid_only_knobs->%*,
+        $wwid_only_props->%*,
+    },
+};
+
+sub private($class) { return $defaultData; }
+
+# The file can be edited by hand; skip WWID sections whose ID multipath would misread.
+sub parse_section_header($class, $line) {
+    my ($type, $id, $errmsg, $config) = $class->SUPER::parse_section_header($line);
+    return undef if !defined($type);
+    $errmsg //= "invalid WWID '$id'" if $type eq 'wwid' && $id !~ $WWID_RE;
+    return ($type, $id, $errmsg, $config);
+}
+
+sub decode_value($class, $type, $key, $value) {
+    if ($key eq 'nodes') {
+        my $res = {};
+        for my $node (PVE::Tools::split_list($value)) {
+            $res->{$node} = 1 if PVE::JSONSchema::pve_verify_node_name($node);
+        }
+        return $res;
+    }
+    return $value;
+}
+
+sub encode_value($class, $type, $key, $value) {
+    return join(',', sort keys %$value) if $key eq 'nodes';
+    return $value;
+}
+
+package PVE::Multipath::Config::Defaults;
+
+use base qw(PVE::Multipath::Config);
+
+sub type($class) { return 'defaults'; }
+
+sub options($class) {
+    return {
+        'find-multipaths' => { optional => 1 },
+        'user-friendly-names' => { optional => 1 },
+        'polling-interval' => { optional => 1 },
+        'auto-resize' => { optional => 1 },
+        'no-path-retry' => { optional => 1 },
+        'path-grouping-policy' => { optional => 1 },
+        failback => { optional => 1 },
+        'path-selector' => { optional => 1 },
+    };
+}
+
+__PACKAGE__->register();
+
+package PVE::Multipath::Config::Wwid;
+
+use base qw(PVE::Multipath::Config);
+
+sub type($class) { return 'wwid'; }
+
+sub options($class) {
+    return {
+        alias => { optional => 1 },
+        nodes => { optional => 1 },
+        'no-path-retry' => { optional => 1 },
+        'path-grouping-policy' => { optional => 1 },
+        failback => { optional => 1 },
+        'path-selector' => { optional => 1 },
+        'rr-min-io-rq' => { optional => 1 },
+        'rr-weight' => { optional => 1 },
+    };
+}
+
+__PACKAGE__->register();
+
+package PVE::Multipath::Config;
+
+__PACKAGE__->init();
+
+# multipaths is generated from WWID sections and must not also occur in overrides.
+my $OVERRIDE_KEYWORDS =
+    { devices => 1, overrides => 1, defaults => 1, blacklist => 1, blacklist_exceptions => 1 };
+
+# This is a structural guard, not a complete multipath.conf parser.
+sub check_overrides($text) {
+    return if !defined($text) || $text !~ /\S/;
+
+    # walk every block opener and brace in order, so a section keyword after a closing brace on
+    # the same line is still checked at its real depth
+    my $depth = 0;
+    for my $line (split(/\n/, $text)) {
+        next if $line =~ /^\s*#/;
+        while ($line =~ /(\w+)\s*\{|([{}])/g) {
+            if (defined($1)) {
+                my $kw = $1;
+                die "multipath overrides: 'multipaths' sections are generated from the multipath"
+                    . " entries, do not set them here\n"
+                    if $kw eq 'multipaths';
+                die "multipath overrides: unknown top-level section '$kw'\n"
+                    if $depth == 0 && !$OVERRIDE_KEYWORDS->{$kw};
+                $depth++;
+            } elsif ($2 eq '{') {
+                $depth++;
+            } else {
+                die "multipath overrides: unbalanced braces\n" if $depth == 0;
+                $depth--;
+            }
+        }
+    }
+    die "multipath overrides: unbalanced braces\n" if $depth != 0;
+
+    return;
+}
+
+sub parse_overrides($filename, $raw) {
+    return $raw // '';
+}
+
+sub write_overrides($filename, $text) {
+    $text //= '';
+    $text =~ s/\s+$//;
+    return length($text) ? "$text\n" : '';
+}
+
+my $MANAGED_HEADER =
+    "# This file is managed by Proxmox VE - do not edit by hand.\n"
+    . "# Hardware-/node-specific overrides belong in the override config.\n";
+
+# multipath.conf requires quotes around whitespace; schemas exclude embedded quotes.
+my sub render_value($value) {
+    return $value =~ /\s/ ? "\"$value\"" : $value;
+}
+
+my sub render_section($name, $kv) {
+    my $out = "$name {\n";
+    for my $key (sort keys $kv->%*) {
+        (my $keyword = $key) =~ tr/-/_/;
+        $out .= "\t$keyword " . render_value($kv->{$key}) . "\n";
+    }
+    $out .= "}\n";
+    return $out;
+}
+
+# Device profiles override defaults, so these global settings need an overrides section.
+# Per-LUN settings still take precedence. Admin overrides use a separate file to avoid duplicate
+# section keywords in one file.
+my $OVERRIDE_KNOBS = {
+    'no-path-retry' => 1,
+    'path-grouping-policy' => 1,
+    failback => 1,
+    'path-selector' => 1,
+};
+
+sub generate_managed_conf($defaults = undef) {
+    $defaults //= managed_defaults();
+
+    my ($global, $overrides) = ({}, {});
+    ($OVERRIDE_KNOBS->{$_} ? $overrides : $global)->{$_} = $defaults->{$_} for keys %$defaults;
+    # the path grouping that multipathd detects for ALUA arrays overrides even the policy of a
+    # single LUN, so turn the detection off when a policy is configured
+    $overrides->{'detect-pgpolicy'} = 'no' if defined($overrides->{'path-grouping-policy'});
+
+    my $conf = $MANAGED_HEADER . "\n" . render_section('defaults', $global);
+    $conf .= "\n" . render_section('overrides', $overrides) if %$overrides;
+    return $conf;
+}
+
+# The WWID allow-list file (/etc/multipath/wwids) holds one '/<wwid>/' per line.
+sub parse_wwids($text) {
+    my $wwids = [];
+    for my $line (split(/\n/, $text // '')) {
+        next if $line =~ /^\s*#/;
+        next if $line =~ /^\s*$/;
+        if ($line =~ m{^/(.+)/\s*$}) {
+            push $wwids->@*, $1;
+        }
+    }
+    return $wwids;
+}
+
+sub parse_managed_wwids($text) {
+    my $record = {};
+    for my $line (split(/\n/, $text // '')) {
+        next if $line =~ /^\s*#/;
+        next if $line !~ m/^\s*(\S+)\s+(\S+)\s*$/;
+        my ($wwid, $sources) = ($1, $2);
+        my @sources = grep { m/^(?:config|storage:.+)$/ } split(/,/, $sources);
+        $record->{$wwid} = [sort @sources] if scalar(@sources);
+    }
+    return $record;
+}
+
+sub format_managed_wwids($record) {
+    my $out = "# Multipath WWIDs added by Proxmox VE, with the reasons each was added\n";
+    $out .= "$_ " . join(',', sort $record->{$_}->@*) . "\n" for sort keys %$record;
+    return $out;
+}
+
+sub build_multipaths_block($wwid_opts) {
+    my @entries = grep { %{ $wwid_opts->{$_} } } sort keys %$wwid_opts;
+    return '' if !@entries;
+
+    my $out = "multipaths {\n";
+    for my $wwid (@entries) {
+        my $opts = $wwid_opts->{$wwid};
+        $out .= "\tmultipath {\n";
+        $out .= "\t\twwid $wwid\n";
+        for my $key (sort keys %$opts) {
+            (my $keyword = $key) =~ tr/-/_/;
+            $out .= "\t\t$keyword " . render_value($opts->{$key}) . "\n";
+        }
+        $out .= "\t}\n";
+    }
+    $out .= "}\n";
+    return $out;
+}
+
+sub apply_section_update($section, $param, $delete, $settable) {
+    my @delete = PVE::Tools::split_list($delete // '');
+    for my $key (@delete) {
+        die "'$key' is not a settable property\n" if !$settable->{$key};
+        die "cannot set and delete property '$key' at the same time\n" if defined($param->{$key});
+    }
+    for my $key (sort keys %$param) {
+        die "'$key' is not a settable property\n" if !$settable->{$key};
+        $section->{$key} = __PACKAGE__->decode_value($section->{type}, $key, $param->{$key});
+    }
+    delete $section->{$_} for @delete;
+    return;
+}
+
+# Omitted update parameters must remain unchanged, not inherit schema defaults.
+my sub api_schema($props) {
+    my $res = {};
+    for my $key (keys %$props) {
+        $res->{$key} = { $props->{$key}->%* };
+        delete $res->{$key}->{default};
+    }
+    return $res;
+}
+
+sub defaults_api_schema() {
+    return api_schema({ $shared_knobs->%*, $defaults_only_knobs->%* });
+}
+
+sub wwid_api_schema() {
+    return api_schema({ $shared_knobs->%*, $wwid_only_knobs->%*, $wwid_only_props->%* });
+}
+
+sub effective_defaults($cfg) {
+    my $defaults = managed_defaults();
+    if (my $section = $cfg->{ids}->{defaults}) {
+        $defaults->{$_} = $section->{$_} for grep { $_ ne 'type' } keys %$section;
+    }
+    return $defaults;
+}
+
+sub wwid_list($cfg) {
+    return [sort grep { ($cfg->{ids}->{$_}->{type} // '') eq 'wwid' } keys $cfg->{ids}->%*];
+}
+
+sub wwids_for_node($cfg, $node) {
+    return [
+        grep {
+            my $nodes = $cfg->{ids}->{$_}->{nodes};
+            !$nodes || !%$nodes || $nodes->{$node}
+        } wwid_list($cfg)->@*
+    ];
+}
+
+# The per-WWID sections as the API reports them, { wwid => { alias?, nodes?, knob => value } },
+# with the node restriction as a list.
+sub wwid_sections($cfg) {
+    my $res = {};
+    for my $wwid (wwid_list($cfg)->@*) {
+        my $section = $cfg->{ids}->{$wwid};
+        my $entry = { map { $_ => $section->{$_} } grep { $_ ne 'type' } keys %$section };
+        $entry->{nodes} = __PACKAGE__->encode_value('wwid', 'nodes', $entry->{nodes})
+            if $entry->{nodes};
+        $res->{$wwid} = $entry;
+    }
+    return $res;
+}
+
+# { wwid => alias } for the WWIDs that have one.
+sub aliases($cfg) {
+    my $res = {};
+    for my $wwid (keys $cfg->{ids}->%*) {
+        my $section = $cfg->{ids}->{$wwid};
+        next if ($section->{type} // '') ne 'wwid';
+        $res->{$wwid} = $section->{alias} if defined($section->{alias});
+    }
+    return $res;
+}
+
+sub wwid_opts($cfg) {
+    my $res = {};
+    for my $wwid (wwid_list($cfg)->@*) {
+        my $section = $cfg->{ids}->{$wwid};
+        $res->{$wwid} = {
+            map { $_ => $section->{$_} }
+            grep { $_ ne 'type' && !$wwid_only_props->{$_} } keys %$section
+        };
+    }
+    return $res;
+}
+
+1;
diff --git a/src/test/run_multipath_tests.pl b/src/test/run_multipath_tests.pl
index 484e9348..0e247284 100755
--- a/src/test/run_multipath_tests.pl
+++ b/src/test/run_multipath_tests.pl
@@ -7,6 +7,7 @@ use Test::More;
 
 use lib ('.', '..');
 use PVE::Multipath;
+use PVE::Multipath::Config;
 
 # A recorded `multipathd show maps json` reply with three maps exercising each
 # health state: an all-active map, a partially-failed map and an all-failed map.
@@ -130,4 +131,258 @@ is_deeply($empty, [], 'no maps parses to empty list');
 eval { PVE::Multipath::parse_maps_json('not json') };
 ok($@ =~ m/could not parse multipathd maps JSON/, 'invalid JSON raises a clear error');
 
+# --- config generation / WWID allow-list ---
+my $conf = PVE::Multipath::Config::generate_managed_conf();
+like($conf, qr/managed by Proxmox VE/, 'managed conf carries the managed header');
+like($conf, qr/user_friendly_names no/, 'baseline sets user_friendly_names no');
+like($conf, qr/find_multipaths strict/, 'baseline opts in explicitly via find_multipaths strict');
+is(
+    scalar(() = $conf =~ /^defaults \{/mg),
+    1,
+    'baseline has exactly one defaults block (a second would be a duplicate-keyword error)',
+);
+
+like($conf, qr/^\tauto_resize grow_only$/m, 'baseline lets maps grow with their LUN');
+unlike($conf, qr/^overrides \{/m, 'baseline overrides nothing of the device profiles');
+
+# multipath.conf only allows whitespace inside double-quoted values
+like(
+    PVE::Multipath::Config::generate_managed_conf({
+        PVE::Multipath::Config::managed_defaults()->%*, 'path-selector' => 'service-time 0',
+    }),
+    qr/^\tpath_selector "service-time 0"$/m,
+    'a value containing whitespace renders double-quoted, single words stay bare',
+);
+
+# what the device profiles set too goes into overrides, or the profiles would beat it
+my $with_policy = PVE::Multipath::Config::generate_managed_conf({
+    PVE::Multipath::Config::managed_defaults()->%*,
+    'no-path-retry' => 'fail',
+    'path-grouping-policy' => 'multibus',
+});
+my ($defaults_block) = $with_policy =~ m/^(defaults \{.*?^\})/ms;
+my ($overrides_block) = $with_policy =~ m/^(overrides \{.*?^\})/ms;
+unlike($defaults_block, qr/no_path_retry|path_grouping_policy/, 'device knobs: not in defaults');
+like($overrides_block, qr/^\tno_path_retry fail$/m, 'device knobs: in overrides');
+like(
+    $overrides_block,
+    qr/^\tdetect_pgpolicy no$/m,
+    'device knobs: a grouping policy turns off the detected grouping',
+);
+
+my $wwids = PVE::Multipath::Config::parse_wwids("# Multipath wwids\n/3600abc/\n/3600def/\n");
+is_deeply($wwids, ['3600abc', '3600def'], 'parse_wwids extracts the wwids');
+
+# --- cluster config (pmxcfs source of truth): SectionConfig parse/write ---
+my $raw =
+    "defaults: defaults\n\tfind-multipaths strict\n\tno-path-retry queue\n\n"
+    . "wwid: 3600def\n\talias san-b-lun0\n\n"
+    . "wwid: 3600abc\n\talias san-a-lun0\n\tno-path-retry 18\n";
+my $cc = PVE::Multipath::Config->parse_config('multipath.cfg', $raw);
+is_deeply(
+    PVE::Multipath::Config::wwid_list($cc),
+    ['3600abc', '3600def'],
+    'wwid sections become the allow-list (sorted)',
+);
+is_deeply(
+    PVE::Multipath::Config::aliases($cc),
+    { '3600abc' => 'san-a-lun0', '3600def' => 'san-b-lun0' },
+    'aliases read from the wwid sections',
+);
+is(
+    PVE::Multipath::Config::effective_defaults($cc)->{'no-path-retry'},
+    'queue',
+    'defaults section knob is read',
+);
+is(
+    PVE::Multipath::Config::effective_defaults($cc)->{'user-friendly-names'},
+    'no',
+    'an unset defaults knob falls back to the managed default',
+);
+
+my $written = PVE::Multipath::Config->write_config('multipath.cfg', $cc);
+my $cc2 = PVE::Multipath::Config->parse_config('multipath.cfg', $written);
+is_deeply(
+    PVE::Multipath::Config::wwid_list($cc2),
+    ['3600abc', '3600def'],
+    'wwids survive the SectionConfig round-trip',
+);
+is_deeply(
+    PVE::Multipath::Config::aliases($cc2),
+    PVE::Multipath::Config::aliases($cc),
+    'aliases survive the round-trip',
+);
+is($cc2->{ids}->{'3600abc'}->{'no-path-retry'}, 18, 'a per-WWID knob survives the round-trip');
+
+is_deeply(
+    PVE::Multipath::Config::wwid_list(PVE::Multipath::Config->parse_config('multipath.cfg', '')),
+    [],
+    'an empty cluster config has no WWIDs',
+);
+{
+    my @warnings;
+    local $SIG{__WARN__} = sub { push @warnings, @_ };
+    my $cfg = PVE::Multipath::Config->parse_config(
+        'multipath.cfg',
+        "wwid: -W\n\nwwid: 8:0\n\nwwid: 3600abc\n\nwwid: 36001405:ab\n",
+    );
+    is_deeply(
+        PVE::Multipath::Config::wwid_list($cfg),
+        ['36001405:ab', '3600abc'],
+        'a WWID that multipath would read as an option or a device number is no section',
+    );
+    is(scalar(@warnings), 2, 'both rejected sections are reported');
+}
+
+# a node restriction stays out of the rendered multipathd settings and filters per node
+my $restricted = PVE::Multipath::Config->parse_config(
+    'multipath.cfg',
+    "wwid: 3600abc\n\talias fc-lun0\n\tnodes n2,n1\n\nwwid: 3600def\n",
+);
+is_deeply(
+    $restricted->{ids}->{'3600abc'}->{nodes},
+    { n1 => 1, n2 => 1 },
+    'the node list is decoded into a set',
+);
+is_deeply(
+    PVE::Multipath::Config::wwid_opts($restricted)->{'3600abc'},
+    { alias => 'fc-lun0' },
+    'the rendered per-WWID options omit the node restriction',
+);
+is_deeply(
+    PVE::Multipath::Config::wwid_sections($restricted)->{'3600abc'},
+    { alias => 'fc-lun0', nodes => 'n1,n2' },
+    'the API view lists the node restriction as a sorted list',
+);
+is_deeply(
+    PVE::Multipath::Config::wwids_for_node($restricted, 'n1'),
+    ['3600abc', '3600def'],
+    'a listed node carries the restricted and the unrestricted WWID',
+);
+is_deeply(
+    PVE::Multipath::Config::wwids_for_node($restricted, 'n3'),
+    ['3600def'],
+    'an unlisted node carries only the unrestricted WWID',
+);
+like(
+    PVE::Multipath::Config->write_config('multipath.cfg', $restricted),
+    qr/^\tnodes n1,n2$/m,
+    'the node list is written back as a list',
+);
+
+# --- API updates land in the config decoded, guarded to the settable properties ---
+{
+    my $settable = PVE::Multipath::Config::wwid_api_schema();
+    my $section = { type => 'wwid', alias => 'old', 'no-path-retry' => 'queue' };
+    PVE::Multipath::Config::apply_section_update(
+        $section,
+        { nodes => 'n2,n1', alias => 'new' },
+        'no-path-retry',
+        $settable,
+    );
+    is_deeply(
+        $section,
+        { type => 'wwid', alias => 'new', nodes => { n1 => 1, n2 => 1 } },
+        'a node list is decoded into the set the config layer works with, deletes are applied',
+    );
+    like(
+        PVE::Multipath::Config->write_config('multipath.cfg', { ids => { w => $section } }),
+        qr/^\tnodes n1,n2$/m,
+        'the updated section writes back as a config',
+    );
+
+    eval {
+        PVE::Multipath::Config::apply_section_update(
+            $section,
+            { type => 'defaults' },
+            undef,
+            $settable,
+        );
+    };
+    like($@, qr/not a settable property/, 'the section type cannot be set through an update');
+    eval { PVE::Multipath::Config::apply_section_update($section, {}, 'type', $settable) };
+    like($@, qr/not a settable property/, 'the section type cannot be deleted through an update');
+    eval {
+        PVE::Multipath::Config::apply_section_update(
+            $section,
+            { alias => 'x' },
+            'alias',
+            $settable,
+        );
+    };
+    like($@, qr/set and delete/, 'a property is never set and deleted at once');
+}
+
+# --- multipaths{} block (alias plus per-WWID knobs) ---
+my $block = PVE::Multipath::Config::build_multipaths_block({
+    '3600def' => { alias => 'san-b-lun0' },
+    '3600abc' => { alias => 'san-a-lun0', 'no-path-retry' => 18 },
+    '3600nul' => {},
+});
+like($block, qr/^multipaths \{/m, 'block opens with multipaths {');
+is(
+    scalar(() = $block =~ /^\tmultipath \{/mg),
+    2,
+    'one multipath{} per WWID that has an alias or a knob (the empty WWID is skipped)',
+);
+like(
+    $block,
+    qr/wwid 3600abc.*?alias san-a-lun0.*?no_path_retry 18/s,
+    'block carries the alias and the per-WWID knob',
+);
+my $abc_pos = index($block, 'wwid 3600abc');
+my $def_pos = index($block, 'wwid 3600def');
+ok($abc_pos < $def_pos, 'block emits entries in WWID-sorted order');
+is(PVE::Multipath::Config::build_multipaths_block({}), '', 'no WWIDs render to the empty string');
+like(
+    PVE::Multipath::Config::build_multipaths_block({
+        '3600abc' => { 'path-selector' => 'round-robin 0' },
+    }),
+    qr/^\t\tpath_selector "round-robin 0"$/m,
+    'a per-WWID value containing whitespace renders double-quoted',
+);
+
+# --- ownership record file format ---
+is_deeply(
+    PVE::Multipath::Config::parse_managed_wwids(
+        "# header\n3600abc config\n3600def storage:san1,config\nbogus line here\n/3600old/\n"
+            . "3600bad nonsense\n"
+    ),
+    { '3600abc' => ['config'], '3600def' => ['config', 'storage:san1'] },
+    'parse_managed_wwids reads wwid and reasons, ignores what does not fit',
+);
+is(
+    PVE::Multipath::Config::format_managed_wwids({
+        '3600def' => ['storage:san1', 'config'],
+        '3600abc' => ['config'],
+    }),
+    "# Multipath WWIDs added by Proxmox VE, with the reasons each was added\n"
+        . "3600abc config\n3600def config,storage:san1\n",
+    'format_managed_wwids sorts and round-trips',
+);
+# --- override guard ---
+eval { PVE::Multipath::Config::check_overrides("devices {\n\tdevice {\n\t\tvendor X\n\t}\n}\n") };
+is($@, '', 'a well-formed devices{} block passes the guard');
+eval { PVE::Multipath::Config::check_overrides("multipaths {\n}\n") };
+like($@, qr/generated from the multipath/, 'a multipaths{} block is rejected, it is generated');
+eval { PVE::Multipath::Config::check_overrides("devices {\n") };
+like($@, qr/unbalanced braces/, 'unbalanced braces are rejected');
+eval { PVE::Multipath::Config::check_overrides("frobnicate {\n}\n") };
+like($@, qr/unknown top-level section/, 'an unknown top-level section is rejected');
+eval { PVE::Multipath::Config::check_overrides("blacklist { } multipaths {\n}\n") };
+like(
+    $@,
+    qr/generated from the multipath/,
+    'a multipaths{} block cannot hide behind a same-line close',
+);
+eval { PVE::Multipath::Config::check_overrides("blacklist { } frobnicate {\n}\n") };
+like($@, qr/unknown top-level section/, 'an unknown section cannot hide behind a same-line close');
+eval { PVE::Multipath::Config::check_overrides("}\ndevices {\n") };
+like($@, qr/unbalanced braces/, 'a closing brace before any open is rejected');
+is(
+    PVE::Multipath::Config::write_overrides('x', "text  \n\n"),
+    "text\n",
+    'the overrides writer trims trailing whitespace',
+);
+
 done_testing();
diff --git a/src/test/run_san_library_tests.pl b/src/test/run_san_library_tests.pl
index 6d509979..84a8539c 100755
--- a/src/test/run_san_library_tests.pl
+++ b/src/test/run_san_library_tests.pl
@@ -11,6 +11,42 @@ use lib ('.', '..');
 use PVE::Cluster;
 use PVE::Storage::StatusCommand;
 
+my %registered;
+my $cluster = Test::MockModule->new('PVE::Cluster');
+$cluster->mock(cfs_register_file => sub { $registered{ $_[0] } = [@_[1, 2]] });
+require PVE::Multipath::ClusterConfig;
+
+subtest 'CFS callbacks preserve filenames, data and method receivers' => sub {
+    my ($parse, $write) = $registered{'multipath.cfg'}->@*;
+    my $cfg = $parse->('multipath.cfg', undef);
+    is_deeply($cfg->{ids}, {}, 'an absent configuration parses as empty');
+    $cfg = $parse->('multipath.cfg', "wwid: 3600abc\n\talias san-lun\n\tnodes n2,n1\n");
+    is_deeply($cfg->{ids}->{'3600abc'}->{nodes}, { n1 => 1, n2 => 1 }, 'nodes are decoded');
+    my $again = $parse->('multipath.cfg', $write->('multipath.cfg', $cfg));
+    is_deeply($again->{ids}, $cfg->{ids}, 'sections survive the registered writer and parser');
+    ($parse, $write) = $registered{'multipath-overrides.conf'}->@*;
+    is($parse->('multipath-overrides.conf', undef), '', 'absent overrides parse as empty');
+    is($write->('multipath-overrides.conf', undef), '', 'undefined overrides write as empty');
+    is($write->('multipath-overrides.conf', "devices { }  \n"), "devices { }\n",
+        'writer trims');
+};
+
+subtest 'omitted optional parameters retain explicit undef behavior' => sub {
+    my $config = Test::MockModule->new('PVE::Multipath::ClusterConfig');
+    $config->mock(cfs_lock_file => sub { $@ = "lock unavailable\n" });
+    for my $lock (
+        \&PVE::Multipath::ClusterConfig::lock_config,
+        \&PVE::Multipath::ClusterConfig::lock_overrides,
+    ) {
+        for my $args ([], [undef]) {
+            eval {
+                $lock->(sub { die "unexpected callback\n" }, @$args);
+            };
+            is($@, "lock unavailable\n", 'an omitted error prefix leaves the error intact');
+        }
+    }
+};
+
 subtest 'shared status retains callback context and named command parameters' => sub {
     for my $cache (undef, { status_only => 1 }) {
         my $code = sub { return wantarray ? ('a', 'b') : 'scalar' };
-- 
2.47.3




  parent reply	other threads:[~2026-11-06  0:56 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 ` Thomas Lamprecht [this message]
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 ` [PATCH v3 storage 34/76] api: add endpoints for the cluster-wide multipath configuration Thomas Lamprecht
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-29-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