Re: [PATCH v2 storage 02/15] mapping: add base plugin

[Mira Limbeck <m.limbeck@proxmox.com>]

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,
>> +        },
>>       },
>>   };
>>