Re: [pve-devel] [PATCH storage] plugin: move definition for 'port' option to base plugin

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 15/04/2024 um 14:48 schrieb Fiona Ebner:
> Commit 7020491 ("esxi: add 'port' config parameter") started using
> the 'port' option in a second plugin, but the definition stayed in the
> PBS plugin. Avoid the hidden dependency and move the definition to the
> base plugin instead.
> 
> It is necessary to mark it as optional or it would be required always.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/Storage/PBSPlugin.pm | 6 ------
>  src/PVE/Storage/Plugin.pm    | 8 ++++++++
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
> index 08ceb88..0808bcc 100644
> --- a/src/PVE/Storage/PBSPlugin.pm
> +++ b/src/PVE/Storage/PBSPlugin.pm
> @@ -49,12 +49,6 @@ sub properties {
>  	    description => "Base64-encoded, PEM-formatted public RSA key. Used to encrypt a copy of the encryption-key which will be added to each encrypted backup.",
>  	    type => 'string',
>  	},
> -	port => {
> -	    description => "For non default port.",
> -	    type => 'integer',
> -	    minimum => 1,
> -	    maximum => 65535,
> -	},
>      };
>  }
>  
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 22a9729..5f49830 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -205,6 +205,14 @@ my $defaultData = {
>  	    format => 'pve-storage-options',
>  	    optional => 1,
>  	},
> +	port => {
> +	    description => "For PBS/ESXi, use this port to connect to the storage instead of the"

I'd probably avoid hard-coding "PBS/ESXi" here, it would work as good if that
part would be omitted:

"Use this port to connect to the storage instead of the default one."

In the long run we should switch to a per-plugin schema, like the (IIRC)
mappings have, as then we could correctly define defaults and descriptions
without having to be overly general to fit a common denominator.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel