Re: [PATCH v2 storage 04/15] iscsi: introduce mapping support

[Dietmar Maurer <dietmar@proxmox.com>]

This is a miss-understanding. The iscsi "portal" property is
not really the iscsi "portal" - instead, this refers to the "discovery 
address"
(we probably want to rename that in the future).

The iscsiadm tool has the same name confusion, using "portal" in the 
discovery
command cli instead of discovery-address.

So I would leave "portal" a required property.

I also wonder what happens when we have both, a "target" and a "mapping"?

- Dietmar

On 4/30/26 7:31 PM, Mira Limbeck wrote:
> `portal` and `target` should be optional then. There is no need for
> those when a mapping is specified. In return the config needs to be
> checked to make sure either portal and target are defined, or a mapping.
>
> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
> ---
>   src/PVE/Storage/ISCSIPlugin.pm | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
> index 30f4178..88a35be 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);
>   
> @@ -354,8 +355,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 },
> @@ -703,4 +705,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;