Please use "fix #2920" as a prefix
Am 13.05.22 um 15:55 schrieb Stefan Hrdlicka:
> this makes it possible to add all mount options offered by mount.cifs
> NFS & CIFS now share the options parameter since the use it for
> the same prupose
>
> Signed-off-by: Stefan Hrdlicka
Just style nits :), so you can add
Reviewed-by: Fiona Ebner
after addressing them.
> ---
> PVE/Storage/CIFSPlugin.pm | 10 --
> PVE/Storage/NFSPlugin.pm | 4
> PVE/Storage/Plugin.pm | 6 ++
> 3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
> index 982040a..e4ea320 100644
> --- a/PVE/Storage/CIFSPlugin.pm
> +++ b/PVE/Storage/CIFSPlugin.pm
> @@ -64,7 +64,7 @@ sub get_cred_file {
> }
>
> sub cifs_mount {
> -my ($server, $share, $mountpoint, $storeid, $smbver, $user, $domain) =
> @_;
> +my ($server, $share, $mountpoint, $storeid, $smbver, $user, $domain,
> $options) = @_;
>
> $server = "[$server]" if Net::IP::ip_is_ipv6($server);
> my $source = "//${server}/$share";
> @@ -80,6 +80,10 @@ sub cifs_mount {
>
> push @$cmd, '-o', defined($smbver) ? "vers=$smbver" : "vers=default";
>
> +if ($options) {
> +push @$cmd, '-o', $options;
> +}
Style nit: could be a single line with post-if
> +
> run_command($cmd, errmsg => "mount error");
> }
>
> @@ -144,6 +148,7 @@ sub options {
> mkdir => { optional => 1 },
> bwlimit => { optional => 1 },
> preallocation => { optional => 1 },
> + options => { optional => 1 },
> };
> }
>
> @@ -223,6 +228,7 @@ sub activate_storage {
> my $path = $scfg->{path};
> my $server = $scfg->{server};
> my $share = $scfg->{share};
> +my $options = $scfg->{options};
Style nit: single-use variable that can be avoided by using
$scfg->{options} directly below.
>
> if (!cifs_is_mounted($server, $share, $path, $cache->{mountdata})) {
>
> @@ -232,7 +238,7 @@ sub activate_storage {
> "directory '$path' does not exist\n" if ! -d $path;
>
> cifs_mount($server, $share, $path, $storeid, $scfg->{smbversion},
> - $scfg->{username}, $scfg->{domain});
> + $scfg->{username}, $scfg->{domain}, $options);
Style nit: already pre-existing problem, but nowadays, we put each
argument on a single line once it gets too long.
> }
>
> $class->SUPER::activate_storage($storeid, $scfg, $cache);
> diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
> index 5bd7313..f777483 100644
> --- a/PVE/Storage/NFSPlugin.pm
> +++ b/PVE/Storage/NFSPlugin.pm
> @@ -69,10 +69,6 @@ sub properties {
> description => "Server IP or DNS name.",
> type => 'string', format => 'pve-storage-server',
> },
> - options => {
> - description => "NFS mount options (see 'man nfs')",
> - type => 'string', format => 'pve-storage-options',
> - },
> };
> }
>
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 0a100b7..cf65b3c 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -180,6 +180,12 @@ my $defaultData = {
> default => 'metadata',
> optional => 1,
> },
> + options => {
> + description => "NFS/CIFS mount options (see 'man nfs' " .
> +"or 'man mount.cifs')",
Style nit: wrong indentation
> + type => 'string', format => 'pve-storage-options',
> +optional => 1,
here as well
> + },
> },
> };
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
