[pve-devel] [PATCH-SERIES v3 storage/docs] fix #2920: add options parameter to CIFS plugin

[pve-devel] [PATCH-SERIES v3 storage/docs] fix #2920: add options parameter to CIFS plugin

[Fiona Ebner]

similar to the already existing parameter for NFS.

Changes v2 -> v3:
* Rebase on current master.
* Minor style fixes.

Changes v1 -> v2:
# pve-storage (1/2)
* fixed nitpicks

# pve-docs (2/2)
* extended options explanation
* changed example option to `echo_interval=30` as second parameter


storage:

Stefan Hrdlicka (1):
  fix #2920: cifs: add options parameter

 PVE/Storage/CIFSPlugin.pm | 4 +++-
 PVE/Storage/NFSPlugin.pm  | 4 ----
 PVE/Storage/Plugin.pm     | 6 ++++++
 3 files changed, 9 insertions(+), 5 deletions(-)


docs:

Stefan Hrdlicka (1):
  fix #2920: add cifs options parameter

 pve-storage-cifs.adoc | 8 ++++++++
 1 file changed, 8 insertions(+)

-- 
2.30.2

[pve-devel] [PATCH v3 storage 1/1] fix #2920: cifs: add options parameter

[Fiona Ebner]

From: Stefan Hrdlicka <s.hrdlicka@proxmox.com>

This makes it possible to add all mount options offered by mount.cifs.
NFS & CIFS now share the options parameter since they use it for the
same purpose.

Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
[FE: rebase + style fixes]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes from v2:
    * minor improvements in commit message
    * adapt to recently changed cifs_mount function interface

 PVE/Storage/CIFSPlugin.pm | 4 +++-
 PVE/Storage/NFSPlugin.pm  | 4 ----
 PVE/Storage/Plugin.pm     | 6 ++++++
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index 6e20f4b..996ef44 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -69,7 +69,7 @@ sub get_cred_file {
 sub cifs_mount : prototype($$$$$) {
     my ($scfg, $storeid, $smbver, $user, $domain) = @_;
 
-    my ($mountpoint, $server, $share) = $scfg->@{'path', 'server', 'share'};
+    my ($mountpoint, $server, $share, $options) = $scfg->@{'path', 'server', 'share', 'options'};
     my $subdir = $scfg->{subdir} // "/";
 
     $server = "[$server]" if Net::IP::ip_is_ipv6($server);
@@ -85,6 +85,7 @@ sub cifs_mount : prototype($$$$$) {
     }
 
     push @$cmd, '-o', defined($smbver) ? "vers=$smbver" : "vers=default";
+    push @$cmd, '-o', $options if $options;
 
     run_command($cmd, errmsg => "mount error");
 }
@@ -152,6 +153,7 @@ sub options {
 	mkdir => { optional => 1 },
 	bwlimit => { optional => 1 },
 	preallocation => { optional => 1 },
+	options => { optional => 1 },
     };
 }
 
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index c2d4176..54423cd 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 ca7a0d4..d08c8aa 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -191,6 +191,12 @@ my $defaultData = {
 	    type => "string", format => "pve-dir-override-list",
 	    optional => 1,
 	},
+	options => {
+	    description => "NFS/CIFS mount options (see 'man nfs' or 'man mount.cifs')",
+	    type => 'string',
+	    format => 'pve-storage-options',
+	    optional => 1,
+	},
     },
 };
 
-- 
2.30.2

[pve-devel] [PATCH v3 docs 1/1] fix #2920: add cifs options parameter

[Fiona Ebner]

From: Stefan Hrdlicka <s.hrdlicka@proxmox.com>

Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
[FE: rebase + style fix]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes from v2:
    * use {pve} instead of PVE

 pve-storage-cifs.adoc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/pve-storage-cifs.adoc b/pve-storage-cifs.adoc
index e0d4106..6008730 100644
--- a/pve-storage-cifs.adoc
+++ b/pve-storage-cifs.adoc
@@ -61,6 +61,13 @@ content-dirs::
 
 Overrides for the default directory layout. Optional.
 
+options::
+
+Additional CIFS mount options (see `man mount.cifs`). Some options are set
+automatically and shouldn't be set here. {pve} will always set the option
+`soft`. Depending on the configuration, these options are set automatically:
+`username`, `credentials`, `guest`, `domain`, `vers`.
+
 .Configuration Example (`/etc/pve/storage.cfg`)
 ----
 cifs: backup
@@ -68,6 +75,7 @@ cifs: backup
 	server 10.0.0.11
 	share VMData
 	content backup
+	options noserverino,echo_interval=30
 	username anna
 	smbversion 3
 
-- 
2.30.2

Re: [pve-devel] [PATCH-SERIES v3 storage/docs] fix #2920: add options parameter to CIFS plugin

[Friedrich Weber]

Tested-by: Friedrich Weber <f.weber@proxmox.com>

I think that would be nice to have, e.g. to set noserverino [1] or 
actimeo [2] without having to mount manually.

[1] 
https://forum.proxmox.com/threads/proxmox-backup-problem.123560/#post-537586
[2] 
https://forum.proxmox.com/threads/pve-cifs-connection-timed-out-596-communication-failure-0.123216/post-537551

On 01/03/2023 13:13, Fiona Ebner wrote:
> similar to the already existing parameter for NFS.
> 
> Changes v2 -> v3:
> * Rebase on current master.
> * Minor style fixes.
> 
> Changes v1 -> v2:
> # pve-storage (1/2)
> * fixed nitpicks
> 
> # pve-docs (2/2)
> * extended options explanation
> * changed example option to `echo_interval=30` as second parameter
> 
> 
> storage:
> 
> Stefan Hrdlicka (1):
>    fix #2920: cifs: add options parameter
> 
>   PVE/Storage/CIFSPlugin.pm | 4 +++-
>   PVE/Storage/NFSPlugin.pm  | 4 ----
>   PVE/Storage/Plugin.pm     | 6 ++++++
>   3 files changed, 9 insertions(+), 5 deletions(-)
> 
> 
> docs:
> 
> Stefan Hrdlicka (1):
>    fix #2920: add cifs options parameter
> 
>   pve-storage-cifs.adoc | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 

Re: [pve-devel] [PATCH-SERIES v3 storage/docs] fix #2920: add options parameter to CIFS plugin

[Friedrich Weber]

Ping -- I think this would be quite useful.

On 13/03/2023 10:38, Friedrich Weber wrote:
> Tested-by: Friedrich Weber <f.weber@proxmox.com>
> 
> I think that would be nice to have, e.g. to set noserverino [1] or
> actimeo [2] without having to mount manually.
> 
> [1]
> https://forum.proxmox.com/threads/proxmox-backup-problem.123560/#post-537586
> [2]
> https://forum.proxmox.com/threads/pve-cifs-connection-timed-out-596-communication-failure-0.123216/post-537551
> 
> On 01/03/2023 13:13, Fiona Ebner wrote:
>> similar to the already existing parameter for NFS.
>>
>> Changes v2 -> v3:
>> * Rebase on current master.
>> * Minor style fixes.
>>
>> Changes v1 -> v2:
>> # pve-storage (1/2)
>> * fixed nitpicks
>>
>> # pve-docs (2/2)
>> * extended options explanation
>> * changed example option to `echo_interval=30` as second parameter
>>
>>
>> storage:
>>
>> Stefan Hrdlicka (1):
>>    fix #2920: cifs: add options parameter
>>
>>   PVE/Storage/CIFSPlugin.pm | 4 +++-
>>   PVE/Storage/NFSPlugin.pm  | 4 ----
>>   PVE/Storage/Plugin.pm     | 6 ++++++
>>   3 files changed, 9 insertions(+), 5 deletions(-)
>>
>>
>> docs:
>>
>> Stefan Hrdlicka (1):
>>    fix #2920: add cifs options parameter
>>
>>   pve-storage-cifs.adoc | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

[pve-devel] applied: [PATCH v3 storage 1/1] fix #2920: cifs: add options parameter

[Thomas Lamprecht]

Am 01/03/2023 um 13:13 schrieb Fiona Ebner:
> From: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> 
> This makes it possible to add all mount options offered by mount.cifs.
> NFS & CIFS now share the options parameter since they use it for the
> same purpose.
> 
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> [FE: rebase + style fixes]
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v2:
>     * minor improvements in commit message
>     * adapt to recently changed cifs_mount function interface
> 
>  PVE/Storage/CIFSPlugin.pm | 4 +++-
>  PVE/Storage/NFSPlugin.pm  | 4 ----
>  PVE/Storage/Plugin.pm     | 6 ++++++
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
>

applied, with trivial context merge conflict resolved & Friedrich's T-b added,
thanks!

[pve-devel] applied: [PATCH v3 docs 1/1] fix #2920: add cifs options parameter

[Thomas Lamprecht]

Am 01/03/2023 um 13:13 schrieb Fiona Ebner:
> From: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> 
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> [FE: rebase + style fix]
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v2:
>     * use {pve} instead of PVE
> 
>  pve-storage-cifs.adoc | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
>

applied, thanks!

Re: [pve-devel] applied: [PATCH v3 docs 1/1] fix #2920: add cifs options parameter

[Friedrich Weber]

Is it possible that this pve-docs patch got lost and was not actually
applied? At least I don't see it here:

https://git.proxmox.com/?p=pve-docs.git;a=history;f=pve-storage-cifs.adoc;h=df63b58d6eefc5c7e2ee302e4ac57fa52c8c372e;hb=0aa61e04787ca6ac791fe6bce28686c9a9fc9ade

... whereas the pve-storage patch was applied fine:

https://git.proxmox.com/?p=pve-storage.git;a=commit;h=13ee4fc8593f4c247a65448b9c746643ce5d3c0c

On 07/06/2023 17:46, Thomas Lamprecht wrote:
> Am 01/03/2023 um 13:13 schrieb Fiona Ebner:
>> From: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
>>
>> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
>> [FE: rebase + style fix]
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>
>> Changes from v2:
>>     * use {pve} instead of PVE
>>
>>  pve-storage-cifs.adoc | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>>
> 
> applied, thanks!
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

[pve-devel] applied: [PATCH v3 docs 1/1] fix #2920: add cifs options parameter

[Thomas Lamprecht]

Am 01/03/2023 um 13:13 schrieb Fiona Ebner:
> From: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> 
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> [FE: rebase + style fix]
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v2:
>     * use {pve} instead of PVE
> 
>  pve-storage-cifs.adoc | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
>

Now actually applied, thanks to you and also to Friedrich noticing
this error of mine!