From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id CE6881FF0E5 for ; Wed, 15 Jul 2026 12:44:34 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id E413C21441; Wed, 15 Jul 2026 12:44:33 +0200 (CEST) Message-ID: Date: Wed, 15 Jul 2026 12:43:55 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 storage 02/15] mapping: add base plugin To: Dietmar Maurer , pve-devel@lists.proxmox.com References: <20260430173220.441001-1-m.limbeck@proxmox.com> <20260430173220.441001-3-m.limbeck@proxmox.com> Content-Language: en-US From: Mira Limbeck In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784112217810 X-SPAM-LEVEL: Spam detection results: 0 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_LOW -0.7 Sender listed at https://www.dnswl.org/, low 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: HKLXY73U5IE4ORPII4WC5DYF64NYTWYJ X-Message-ID-Hash: HKLXY73U5IE4ORPII4WC5DYF64NYTWYJ X-MailFrom: m.limbeck@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: On 7/13/26 11:53 AM, Dietmar Maurer wrote: > stupid question, but why don't we simply add the "map" property to the > iscsi > storage  definition: > > in ISCSIPlugin.pm: > > sub properties { >     return { >         target => { >             description => "iSCSI target.", >             type => 'string', >         }, >        'iscsi-map' => { >             type => 'array', >             description => 'A list of maps.', >             optional => 1, >             items => { >                 type => 'string', >                 format => $map_fmt, >             }, >         }, >         portal => { >             description => "iSCSI portal (IP or DNS name with optional > port).", >             type => 'string', >             format => 'pve-storage-portal-dns', >         }, >     }; > } > > Defining that extra mapping plugin seems unnecessary to me. > > note: not tested - just curious... That could work. The only downside I can see at the moment is that it pollutes the list of options even further, since those are not isolated. But that's already the case for anything that's introduced by a plugin anyway. The inspiration to write it as a plugin on top of the current storage plugins mostly came from the PCI and USB mapping plugins we already had. I'll look into handling them directly within the storage plugins. > > > On 4/30/26 7:32 PM, Mira Limbeck wrote: >> 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. >> >> To facilitate such a setup we introduce mappings via a base mapping >> plugin that can be extended for each type of storage. >> >> --- >>   src/PVE/Storage.pm                |  3 ++ >>   src/PVE/Storage/Makefile          |  4 +- >>   src/PVE/Storage/Mapping.pm        | 44 ++++++++++++++++++ >>   src/PVE/Storage/Mapping/Makefile  |  6 +++ >>   src/PVE/Storage/Mapping/Plugin.pm | 74 +++++++++++++++++++++++++++++++ >>   src/PVE/Storage/Plugin.pm         |  6 +++ >>   6 files changed, 136 insertions(+), 1 deletion(-) >>   create mode 100644 src/PVE/Storage/Mapping.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 6e87bac..d783788 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/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..b607156 >> --- /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_key' 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/Makefile b/src/PVE/Storage/ >> Mapping/Makefile >> new file mode 100644 >> index 0000000..168bea6 >> --- /dev/null >> +++ b/src/PVE/Storage/Mapping/Makefile >> @@ -0,0 +1,6 @@ >> +SOURCES= \ >> +    Plugin.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 afd3141..61cda22 100644 >> --- a/src/PVE/Storage/Plugin.pm >> +++ b/src/PVE/Storage/Plugin.pm >> @@ -247,6 +247,12 @@ my $defaultData = { >>               default => 0, >>               optional => 1, >>           }, >> +        mapping => { >> +            description => "Logical per-node storage mapping.", >> +            type => 'string', >> +            format => 'pve-storage-id', >> +            optional => 1, >> +        }, >>       }, >>   }; >>