From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id AC74C1FF15E for ; Mon, 10 Nov 2025 18:00:47 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C77921C27D; Mon, 10 Nov 2025 18:01:30 +0100 (CET) From: Mira Limbeck To: pve-devel@lists.proxmox.com Date: Mon, 10 Nov 2025 18:01:23 +0100 Message-Id: <20251110170124.3460419-4-m.limbeck@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20251110170124.3460419-1-m.limbeck@proxmox.com> References: <20251110170124.3460419-1-m.limbeck@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.595 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Subject: [pve-devel] [RFC storage 1/2] add basic mapping support and iSCSI mapping plugin X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" For some storages, for example iSCSI, it can make sense to have a per-node mapping rather than a cluster-wide storage configuration. This allows for example to have different portals and targets for each node, that all map to the same SAN and backing storage on the SAN. This helps with issues where not every portal is reachable from every node. In those cases there are currently lots of errors logged and the connection is retried regularly, increasing pvestatd update times unnecessarily. Signed-off-by: Mira Limbeck --- src/PVE/Storage.pm | 3 + src/PVE/Storage/ISCSIPlugin.pm | 207 ++++++++++++++++++++++++------ src/PVE/Storage/Makefile | 4 +- src/PVE/Storage/Mapping.pm | 44 +++++++ src/PVE/Storage/Mapping/ISCSI.pm | 54 ++++++++ src/PVE/Storage/Mapping/Makefile | 7 + src/PVE/Storage/Mapping/Plugin.pm | 74 +++++++++++ src/PVE/Storage/Plugin.pm | 6 + 8 files changed, 356 insertions(+), 43 deletions(-) create mode 100644 src/PVE/Storage/Mapping.pm create mode 100644 src/PVE/Storage/Mapping/ISCSI.pm create mode 100644 src/PVE/Storage/Mapping/Makefile create mode 100644 src/PVE/Storage/Mapping/Plugin.pm diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 935d457..d0a1c2c 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -24,6 +24,9 @@ use PVE::RPCEnvironment; use PVE::SSHInfo; use PVE::RESTEnvironment qw(log_warn); +# registers Mapping Plugins +use PVE::Storage::Mapping; + use PVE::Storage::Plugin; use PVE::Storage::DirPlugin; use PVE::Storage::LVMPlugin; diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm index 30f4178..f6a7ddf 100644 --- a/src/PVE/Storage/ISCSIPlugin.pm +++ b/src/PVE/Storage/ISCSIPlugin.pm @@ -9,6 +9,7 @@ use IO::File; use PVE::JSONSchema qw(get_standard_option); use PVE::Storage::Plugin; +use PVE::Storage::Mapping; use PVE::Tools qw(run_command file_read_firstline trim dir_glob_regex dir_glob_foreach $IPV4RE $IPV6RE); @@ -35,6 +36,44 @@ my sub assert_iscsi_support { # Example: 192.168.122.252:3260,1 iqn.2003-01.org.linux-iscsi.proxmox-nfs.x8664:sn.00567885ba8f my $ISCSI_TARGET_RE = qr/^(\S+:\d+)\,\S+\s+(\S+)\s*$/; +my $get_local_config = sub { + my ($scfg) = @_; + + die "neither 'target' nor 'mapping' defined\n" + if !defined($scfg->{target}) && !defined($scfg->{mapping}); + + my $res = {}; + if ($scfg->{mapping}) { + my $local_mappings = + PVE::Storage::Mapping::find_mapping_on_current_node($scfg->{mapping}); + for my $mapping ($local_mappings->@*) { + $res->{targets}->{ $mapping->{target} } //= []; + my $portals = [PVE::Tools::split_list($mapping->{portals})]; + + my $add_port = sub { + my ($val) = @_; + + my ($ip, $port) = PVE::Tools::parse_host_and_port($val); + if (defined($port)) { + return $val; + } else { + # add default port + return $ip . ':3260'; + } + }; + $portals->@* = map { $add_port->($_) } $portals->@*; + + push $res->{targets}->{ $mapping->{target} }->@*, $portals->@*; + } + return $res; + } else { + # iscsi_portals too heavy?? + $res->{discovery} = 1; + $res->{targets}->{ $scfg->{target} } = iscsi_portals($scfg->{target}, $scfg->{portal}); + return $res; + } +}; + sub iscsi_session_list { assert_iscsi_support(); @@ -145,7 +184,8 @@ sub iscsi_discovery { my ($portal, $target) = ($1, $2); # one target can have more than one portal (multipath) # and sendtargets should return all of them in single call - push @{ $res->{$target} }, $portal; + my $entry = { portal => $portal }; + push $res->{$target}->@*, $entry; } }, ); @@ -159,11 +199,11 @@ sub iscsi_discovery { } sub iscsi_login { - my ($target, $portals, $cache) = @_; + my ($target, $portals, $cache, $run_discovery) = @_; assert_iscsi_support(); - eval { iscsi_discovery($target, $portals, $cache); }; + eval { iscsi_discovery($target, $portals, $cache) if $run_discovery; }; warn $@ if $@; # Disable retries to avoid blocking pvestatd for too long, next iteration will retry anyway @@ -185,7 +225,37 @@ sub iscsi_login { }; warn $@ if $@; - run_command([$ISCSIADM, '--mode', 'node', '--targetname', $target, '--login']); + if ($run_discovery) { + my $cmd = [ + $ISCSIADM, '--mode', 'node', '--targetname', $target, '--login', + ]; + eval { run_command($cmd); }; + warn $@ if $@; + } else { + my $sessions = iscsi_session($cache, $target); + for my $portal ($portals->@*) { + my $logged_in_session = 0; + for my $session ($sessions->@*) { + next if $session->{portal} ne $portal; + $logged_in_session = iscsi_test_session($session->{session_id}); + last if $logged_in_session; + } + # skip login if we already have a session + next if $logged_in_session; + my $cmd = [ + $ISCSIADM, + '--mode', + 'node', + '--targetname', + $target, + '--portal', + $portal, + '--login', + ]; + eval { run_command($cmd); }; + warn $@ if $@; + } + } } sub iscsi_logout { @@ -354,8 +424,9 @@ sub properties { sub options { return { - portal => { fixed => 1 }, - target => { fixed => 1 }, + portal => { fixed => 1, optional => 1 }, + target => { fixed => 1, optional => 1 }, + mapping => { optional => 1 }, nodes => { optional => 1 }, disable => { optional => 1 }, content => { optional => 1 }, @@ -434,26 +505,30 @@ sub list_images { # we have no owner for iscsi devices - my $target = $scfg->{target}; + my $local_cfg = $get_local_config->($scfg); + my $targets = $local_cfg->{targets}; - if (my $dat = $cache->{iscsi_devices}->{$target}) { + for my $target (keys $targets->%*) { + if (my $dat = $cache->{iscsi_devices}->{$target}) { - foreach my $volname (keys %$dat) { + foreach my $volname (keys %$dat) { - my $volid = "$storeid:$volname"; + my $volid = "$storeid:$volname"; - if ($vollist) { - my $found = grep { $_ eq $volid } @$vollist; - next if !$found; - } else { - # we have no owner for iscsi devices - next if defined($vmid); - } + if ($vollist) { + my $found = grep { $_ eq $volid } @$vollist; + next if !$found; + } else { + # we have no owner for iscsi devices + next if defined($vmid); + } - my $info = $dat->{$volname}; - $info->{volid} = $volid; + my $info = $dat->{$volname}; + $info->{volid} = $volid; - push @$res, $info; + push @$res, $info; + } + last; } } @@ -469,8 +544,13 @@ sub iscsi_session { sub status { my ($class, $storeid, $scfg, $cache) = @_; - my $session = iscsi_session($cache, $scfg->{target}); - my $active = defined($session) ? 1 : 0; + my $local_cfg = $get_local_config->($scfg); + my $active = 0; + for my $target (keys $local_cfg->{targets}->%*) { + my $session = iscsi_session($cache, $target); + $active = 1 if defined($session); + last if defined($session); + } return (0, 0, 0, $active); } @@ -480,28 +560,48 @@ sub activate_storage { return if !assert_iscsi_support(1); - my $sessions = iscsi_session($cache, $scfg->{target}); - my $portals = iscsi_portals($scfg->{target}, $scfg->{portal}); - my $do_login = !defined($sessions); + my $local_cfg = $get_local_config->($scfg); + my $targets = {}; + for my $target (keys $local_cfg->{targets}->%*) { + my $sessions = iscsi_session($cache, $target); + $targets->{$target}->{sessions} = $sessions; + $targets->{$target}->{portals} = $local_cfg->{targets}->{$target}; + } + my $do_login = 0; + for my $target (keys $targets->%*) { + $do_login = 1 if !defined($targets->{$target}->{sessions}); + } if (!$do_login) { # We should check that sessions for all portals are available - my $session_portals = [map { $_->{portal} } (@$sessions)]; - - for my $portal (@$portals) { - if (!grep(/^\Q$portal\E$/, @$session_portals)) { - $do_login = 1; - last; + for my $target (keys $targets->%*) { + my $session_portals = [map { $_->{portal} } ($targets->{$target}->{sessions}->@*)]; + for my $portal ($targets->{$target}->{portals}->@*) { + if (!grep(/^\Q$portal\E(?::3260)?$/, $session_portals->@*)) { + $do_login = 1; + last; + } } } } if ($do_login) { - eval { iscsi_login($scfg->{target}, $portals, $cache); }; - warn $@ if $@; + for my $target (keys $targets->%*) { + eval { + iscsi_login( + $target, + $targets->{$target}->{portals}, + $cache, + $local_cfg->{discovery}, + ); + }; + warn $@ if $@; + } } else { # make sure we get all devices - iscsi_session_rescan($sessions); + for my $target (keys $targets->%*) { + iscsi_session_rescan($targets->{$target}->{sessions}); + } } } @@ -510,8 +610,11 @@ sub deactivate_storage { return if !assert_iscsi_support(1); - if (defined(iscsi_session($cache, $scfg->{target}))) { - iscsi_logout($scfg->{target}); + my $local_cfg = $get_local_config->($scfg); + for my $target (keys $local_cfg->{targets}->%*) { + if (defined(iscsi_session($cache, $target))) { + iscsi_logout($target); + } } } @@ -609,18 +712,26 @@ sub activate_volume { my $device_path = $udev_query_path->($real_path); my $resolved_paths = $resolve_virtual_devices->($device_path); - my $found = $check_devices_part_of_target->($resolved_paths, $scfg->{target}); - die "volume '$volname' not part of target '$scfg->{target}'\n" if !$found; + my $local_cfg = $get_local_config->($scfg); + my $found = 0; + for my $target ($local_cfg->{targets}->%*) { + $found ||= $check_devices_part_of_target->($resolved_paths, $target); + last if $found; + } + die "volume '$volname' not part of any matching target\n" if !$found; } sub check_connection { my ($class, $storeid, $scfg) = @_; + my $cache = {}; - my $portals = iscsi_portals($scfg->{target}, $scfg->{portal}); + my $local_cfg = $get_local_config->($scfg); - for my $portal (@$portals) { - my $result = iscsi_test_portal($scfg->{target}, $portal, $cache); - return $result if $result; + for my $target (keys $local_cfg->{targets}->%*) { + for my $portal ($local_cfg->{targets}->{$target}->@*) { + my $result = iscsi_test_portal($target, $portal, $cache); + return $result if $result; + } } return 0; @@ -703,4 +814,16 @@ sub volume_import { die "volume import is not possible on iscsi storage\n"; } +sub check_config { + my ($class, $sectionId, $config, $create, $skipSchemaCheck) = @_; + + my $checked = $class->SUPER::check_config($sectionId, $config, $create, $skipSchemaCheck); + + # check if either target or mapping is set + die "iscsi storage '$sectionId' has neither 'target' nor 'mapping' defined\n" + if !defined($checked->{target}) && !defined($checked->{mapping}); + + return $checked; +} + 1; diff --git a/src/PVE/Storage/Makefile b/src/PVE/Storage/Makefile index a67dc25..c5861da 100644 --- a/src/PVE/Storage/Makefile +++ b/src/PVE/Storage/Makefile @@ -14,10 +14,12 @@ SOURCES= \ PBSPlugin.pm \ BTRFSPlugin.pm \ LvmThinPlugin.pm \ - ESXiPlugin.pm + ESXiPlugin.pm \ + Mapping.pm .PHONY: install install: make -C Common install for i in ${SOURCES}; do install -D -m 0644 $$i ${DESTDIR}${PERLDIR}/PVE/Storage/$$i; done make -C LunCmd install + make -C Mapping install diff --git a/src/PVE/Storage/Mapping.pm b/src/PVE/Storage/Mapping.pm new file mode 100644 index 0000000..86763e7 --- /dev/null +++ b/src/PVE/Storage/Mapping.pm @@ -0,0 +1,44 @@ +package PVE::Storage::Mapping; + +use PVE::JSONSchema; + +use PVE::Storage::Mapping::ISCSI; +use PVE::Storage::Mapping::Plugin; + +PVE::Storage::Mapping::ISCSI->register(); +PVE::Storage::Mapping::Plugin->init(property_isolation => 1); + +sub find_mapping_on_current_node { + my ($id) = @_; + + my $cfg = PVE::Storage::Mapping::Plugin::config(); + my $nodename = PVE::INotify::nodename(); + + return get_node_mapping($cfg, $id, $nodename); +} + +sub get_node_mapping { + my ($cfg, $id, $nodename) = @_; + + my $mapping = $cfg->{ids}->{$id}; + return undef if !defined($mapping); + + my $plugin_type = $cfg->{ids}->{$id}->{type}; + my $plugin = PVE::Storage::Mapping::Plugin->lookup($plugin_type); + + my $map_key = $plugin->get_map_key(); + my $map_fmt = $plugin->get_map_format(); + warn "no 'map' property found\n" if !$map_fmt; + + my $res = []; + for my $map ($mapping->{$map_key}->@*) { + my $entry = eval { PVE::JSONSchema::parse_property_string($map_fmt, $map) }; + warn $@ if $@; + if ($entry && $entry->{node} eq $nodename) { + push $res->@*, $entry; + } + } + return $res; +} + +1; diff --git a/src/PVE/Storage/Mapping/ISCSI.pm b/src/PVE/Storage/Mapping/ISCSI.pm new file mode 100644 index 0000000..34d00c5 --- /dev/null +++ b/src/PVE/Storage/Mapping/ISCSI.pm @@ -0,0 +1,54 @@ +package PVE::Storage::Mapping::ISCSI; + +use strict; +use warnings; + +use Storable qw(dclone); + +use PVE::JSONSchema qw(get_standard_option); +use PVE::Storage::Mapping::Plugin; +use base qw(PVE::Storage::Mapping::Plugin); + +sub type { + return 'iscsi'; +} + +my $map_fmt = { + node => get_standard_option('pve-node'), + target => { + type => 'string', + description => 'Local iSCSI target.', + }, + portals => { + type => 'string', + description => 'List of iSCSI portals for the target.', + format => 'pve-storage-portal-dns-list', + }, +}; + +sub properties { + return { + map => { + type => 'array', + description => 'A list of maps.', + optional => 1, + items => { + type => 'string', + format => $map_fmt, + }, + }, + }; +} + +sub options { + return { + description => { optional => 1 }, + map => {}, + }; +} + +sub get_map_format { + return dclone($map_fmt); +} + +1; diff --git a/src/PVE/Storage/Mapping/Makefile b/src/PVE/Storage/Mapping/Makefile new file mode 100644 index 0000000..b4972fb --- /dev/null +++ b/src/PVE/Storage/Mapping/Makefile @@ -0,0 +1,7 @@ +SOURCES= \ + Plugin.pm \ + ISCSI.pm \ + +.PHONY: install +install: + for i in ${SOURCES}; do install -D -m 0644 $$i ${DESTDIR}${PERLDIR}/PVE/Storage/Mapping/$$i; done diff --git a/src/PVE/Storage/Mapping/Plugin.pm b/src/PVE/Storage/Mapping/Plugin.pm new file mode 100644 index 0000000..2da2e26 --- /dev/null +++ b/src/PVE/Storage/Mapping/Plugin.pm @@ -0,0 +1,74 @@ +package PVE::Storage::Mapping::Plugin; + +use strict; +use warnings; + +use PVE::Storage::Mapping::ISCSI; +use PVE::INotify; +use PVE::JSONSchema; +use PVE::Cluster qw( + cfs_lock_file + cfs_read_file + cfs_register_file + cfs_write_file +); + +use base qw(PVE::SectionConfig); + +my $FILENAME = 'mapping/storage.cfg'; + +cfs_register_file( + $FILENAME, + sub { __PACKAGE__->parse_config(@_); }, + sub { __PACKAGE__->write_config(@_); }, +); + +# from PVE::Storage::Plugin +sub parse_section_header { + my ($class, $line) = @_; + + if ($line =~ m/^(\S+):\s*(\S+)\s*$/) { + my ($type, $storeid) = (lc($1), $2); + my $errmsg = undef; # set if you want to skip whole section + eval { PVE::JSONSchema::parse_storage_id($storeid); }; + $errmsg = $@ if $@; + my $config = {}; # to return additional attributes + return ($type, $storeid, $errmsg, $config); + } + return undef; +} + +my $defaultData = { + propertyList => { + type => { description => "Storage type." }, + id => { + description => "The ID of the logical storage mapping.", + type => 'string', + format => 'pve-storage-id', + }, + description => { + description => "Description of the logical storage.", + type => 'string', + optional => 1, + maxLength => 4096, + }, + }, +}; + +sub private { + return $defaultData; +} + +sub config { + return cfs_read_file($FILENAME); +} + +sub get_map_key { + return 'map'; +} + +sub get_map_format { + die "implement in subclass\n"; +} + +1; diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 8acd214..3e26cde 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -228,6 +228,12 @@ my $defaultData = { default => 0, optional => 1, }, + mapping => { + description => "Logical per-node storage mapping.", + type => 'string', + format => 'pve-storage-id', + optional => 1, + }, }, }; -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel