From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id ECFD91FF0AF for ; Fri, 06 Nov 2026 01:56:49 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 8DB9221C62; Fri, 06 Nov 2026 01:51:16 +0100 (CET) From: Thomas Lamprecht 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 Message-ID: <20261105233336.2178064-29-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: 1793926179157 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.308 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: FRBNSADEOLHYM4HFOUNA7QFJA62V7QBT X-Message-ID-Hash: FRBNSADEOLHYM4HFOUNA7QFJA62V7QBT 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: 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 --- 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 +# ':' 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|', + 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|', + 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 => ' ', + 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 '//' 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