* [PATCH storage 0/2] fix #6050: normalize paths in mount status checks
@ 2026-08-07 9:36 David Riley
2026-08-07 9:36 ` [PATCH pve-storage 1/2] fix #6050: nfs: strip trailing slashes from mount status check David Riley
2026-08-07 9:36 ` [PATCH pve-storage 2/2] fix #6050: cifs: normalize share and subdir paths for mount and status David Riley
0 siblings, 2 replies; 6+ messages in thread
From: David Riley @ 2026-08-07 9:36 UTC (permalink / raw)
To: pve-devel
Manual edits in the `storage.cfg` can introduce trailing or extra
slashes into storage paths. While these storages mount and function
properly, the kernel normalizes entries in `/proc/mounts` by omitting
trailing slashes.
As a result, status verification checks in the NFS and CIFS/SMB
Storage Plugin fail due to string mismatches, causing working storages
to continuously report as 'inactive' in both the web UI and via
`pvesm status`.
This series sanitizes the export, share, and subdirectory paths prior
to checking `/proc/mounts`, ensuring active mounts are correctly
recognized.
pve-storage:
David Riley (2):
fix #6050: nfs: strip trailing slashes from mount status check
fix #6050: cifs: normalize share and subdir paths for mount and status
src/PVE/Storage/CIFSPlugin.pm | 32 ++++++++++++++++++++++++++++----
src/PVE/Storage/NFSPlugin.pm | 6 +++++-
2 files changed, 33 insertions(+), 5 deletions(-)
Summary over all repositories:
2 files changed, 33 insertions(+), 5 deletions(-)
--
Generated by murpp 0.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH pve-storage 1/2] fix #6050: nfs: strip trailing slashes from mount status check
2026-08-07 9:36 [PATCH storage 0/2] fix #6050: normalize paths in mount status checks David Riley
@ 2026-08-07 9:36 ` David Riley
2026-08-18 9:35 ` Jakob Klocker
2026-08-07 9:36 ` [PATCH pve-storage 2/2] fix #6050: cifs: normalize share and subdir paths for mount and status David Riley
1 sibling, 1 reply; 6+ messages in thread
From: David Riley @ 2026-08-07 9:36 UTC (permalink / raw)
To: pve-devel
Manual edits to the storage configuration bypass strict API
validation. If an NFS export path includes a trailing slash, the share
mounts successfully but continuously reports as 'inactive' in both the
web UI and pvesm status.
This occurs because mount status verification compares the
configuration string against the entries in /proc/mounts.
Normalize the export path by stripping trailing slashes prior to
verifying the mount status.
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6050
Signed-off-by: David Riley <d.riley@proxmox.com>
---
src/PVE/Storage/NFSPlugin.pm | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
index 4cc02c9..6c67848 100644
--- a/src/PVE/Storage/NFSPlugin.pm
+++ b/src/PVE/Storage/NFSPlugin.pm
@@ -2,9 +2,11 @@ package PVE::Storage::NFSPlugin;
use strict;
use warnings;
+
+use File::Path;
+use File::Spec;
use IO::File;
use Net::IP;
-use File::Path;
use PVE::Network;
use PVE::Tools qw(run_command);
@@ -19,6 +21,8 @@ use base qw(PVE::Storage::Plugin);
sub nfs_is_mounted {
my ($server, $export, $mountpoint, $mountdata) = @_;
+ $export = File::Spec->canonpath($export);
+
$server = "[$server]" if Net::IP::ip_is_ipv6($server);
my $source = "$server:$export";
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH pve-storage 2/2] fix #6050: cifs: normalize share and subdir paths for mount and status
2026-08-07 9:36 [PATCH storage 0/2] fix #6050: normalize paths in mount status checks David Riley
2026-08-07 9:36 ` [PATCH pve-storage 1/2] fix #6050: nfs: strip trailing slashes from mount status check David Riley
@ 2026-08-07 9:36 ` David Riley
2026-08-18 10:23 ` Jakob Klocker
1 sibling, 1 reply; 6+ messages in thread
From: David Riley @ 2026-08-07 9:36 UTC (permalink / raw)
To: pve-devel
Manual edits to the storage configuration bypass strict API
validation. If a user adds extra or trailing slashes to the share or
subdirectory paths, it causes two distinct issues for CIFS storages:
* The Linux VFS CIFS module expects strict UNC [0] formatting and
will reject the mount attempt with a 'Malformed UNC in devname'
error.
* The internal path used by the status daemon fails to match the
normalized path reported in /proc/mounts, causing working storages
to continuously report as 'inactive'.
Sanitize the share and subdirectory strings by stripping leading and
trailing slashes before executing mounts, verifying mount status, and
testing the SMB connection. This ensures working storages mount
successfully and are correctly recognized as active.
[0] https://docs.kernel.org/admin-guide/cifs/usage.html#cifs-vfs-mount-options
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6050
Signed-off-by: David Riley <d.riley@proxmox.com>
---
src/PVE/Storage/CIFSPlugin.pm | 32 ++++++++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/PVE/Storage/CIFSPlugin.pm b/src/PVE/Storage/CIFSPlugin.pm
index 54f0f4e..78ed88c 100644
--- a/src/PVE/Storage/CIFSPlugin.pm
+++ b/src/PVE/Storage/CIFSPlugin.pm
@@ -2,15 +2,20 @@ package PVE::Storage::CIFSPlugin;
use strict;
use warnings;
+
+use File::Path;
use Net::IP;
+
use PVE::Tools qw(run_command);
use PVE::ProcFSTools;
-use File::Path;
use PVE::Storage::Plugin;
use PVE::JSONSchema qw(get_standard_option);
use base qw(PVE::Storage::Plugin);
+my $RM_LEADING_SLASHES = qr{^/+};
+my $RM_TRAILING_SLASHES = qr{/+$};
+
# CIFS helper functions
sub cifs_is_mounted : prototype($$) {
@@ -19,8 +24,16 @@ sub cifs_is_mounted : prototype($$) {
my ($mountpoint, $server, $share) = $scfg->@{ 'path', 'server', 'share' };
my $subdir = $scfg->{subdir} // '';
+ for my $path ($share, $subdir) {
+ $path =~ s/$RM_LEADING_SLASHES// if $path;
+ $path =~ s/$RM_TRAILING_SLASHES// if $path;
+ }
+
$server = "[$server]" if Net::IP::ip_is_ipv6($server);
- my $source = "//${server}/$share$subdir";
+ my $source = "//${server}";
+ $source .= "/$share";
+ $source .= "/$subdir" if $subdir;
+
$mountdata = PVE::ProcFSTools::parse_proc_mounts() if !$mountdata;
return $mountpoint if grep {
@@ -83,8 +96,15 @@ sub cifs_mount : prototype($$$$$) {
my ($mountpoint, $server, $share, $options) = $scfg->@{ 'path', 'server', 'share', 'options' };
my $subdir = $scfg->{subdir} // '';
+ for my $path ($share, $subdir) {
+ $path =~ s/$RM_LEADING_SLASHES// if $path;
+ $path =~ s/$RM_TRAILING_SLASHES// if $path;
+ }
+
$server = "[$server]" if Net::IP::ip_is_ipv6($server);
- my $source = "//${server}/$share$subdir";
+ my $source = "//${server}";
+ $source .= "/$share";
+ $source .= "/$subdir" if $subdir;
my $cmd = ['/bin/mount', '-t', 'cifs', $source, $mountpoint, '-o', 'soft'];
@@ -285,7 +305,11 @@ sub deactivate_storage {
sub check_connection {
my ($class, $storeid, $scfg) = @_;
- my $servicename = '//' . $scfg->{server} . '/' . $scfg->{share};
+ my $share = $scfg->{share};
+ $share =~ s/$RM_LEADING_SLASHES// if $share;
+ $share =~ s/$RM_TRAILING_SLASHES// if $share;
+
+ my $servicename = '//' . $scfg->{server} . '/' . $share;
my $cmd = ['/usr/bin/smbclient', $servicename, '-d', '0'];
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH pve-storage 1/2] fix #6050: nfs: strip trailing slashes from mount status check
2026-08-07 9:36 ` [PATCH pve-storage 1/2] fix #6050: nfs: strip trailing slashes from mount status check David Riley
@ 2026-08-18 9:35 ` Jakob Klocker
0 siblings, 0 replies; 6+ messages in thread
From: Jakob Klocker @ 2026-08-18 9:35 UTC (permalink / raw)
To: David Riley, pve-devel
Tested this by adding leading and trailing slashes to the `export`
property in an NFS storage. Before the patch, the storage was
displayed as inactive, even though it was usable. After the patch
it shows correctly as active.
Reviewed-by: Jakob Klocker <j.klocker@proxmox.com>
Tested-by: Jakob Klocker <j.klocker@proxmox.com>
On Fri Aug 7, 2026 at 11:36 AM CEST, David Riley wrote:
> Manual edits to the storage configuration bypass strict API
> validation. If an NFS export path includes a trailing slash, the share
> mounts successfully but continuously reports as 'inactive' in both the
> web UI and pvesm status.
>
> This occurs because mount status verification compares the
> configuration string against the entries in /proc/mounts.
>
> Normalize the export path by stripping trailing slashes prior to
> verifying the mount status.
>
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6050
> Signed-off-by: David Riley <d.riley@proxmox.com>
> ---
> src/PVE/Storage/NFSPlugin.pm | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
> index 4cc02c9..6c67848 100644
> --- a/src/PVE/Storage/NFSPlugin.pm
> +++ b/src/PVE/Storage/NFSPlugin.pm
> @@ -2,9 +2,11 @@ package PVE::Storage::NFSPlugin;
>
> use strict;
> use warnings;
> +
> +use File::Path;
> +use File::Spec;
> use IO::File;
> use Net::IP;
> -use File::Path;
>
> use PVE::Network;
> use PVE::Tools qw(run_command);
> @@ -19,6 +21,8 @@ use base qw(PVE::Storage::Plugin);
> sub nfs_is_mounted {
> my ($server, $export, $mountpoint, $mountdata) = @_;
>
> + $export = File::Spec->canonpath($export);
> +
> $server = "[$server]" if Net::IP::ip_is_ipv6($server);
> my $source = "$server:$export";
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH pve-storage 2/2] fix #6050: cifs: normalize share and subdir paths for mount and status
2026-08-07 9:36 ` [PATCH pve-storage 2/2] fix #6050: cifs: normalize share and subdir paths for mount and status David Riley
@ 2026-08-18 10:23 ` Jakob Klocker
2026-08-18 12:33 ` David Riley
0 siblings, 1 reply; 6+ messages in thread
From: Jakob Klocker @ 2026-08-18 10:23 UTC (permalink / raw)
To: David Riley, pve-devel
Comments inline. I'd also suggest mentioning why you didn't reuse the
`canonpath` function you used in the previous patch here.
On Fri Aug 7, 2026 at 11:36 AM CEST, David Riley wrote:
> Manual edits to the storage configuration bypass strict API
> validation. If a user adds extra or trailing slashes to the share or
> subdirectory paths, it causes two distinct issues for CIFS storages:
>
> * The Linux VFS CIFS module expects strict UNC [0] formatting and
> will reject the mount attempt with a 'Malformed UNC in devname'
> error.
> * The internal path used by the status daemon fails to match the
> normalized path reported in /proc/mounts, causing working storages
> to continuously report as 'inactive'.
>
> Sanitize the share and subdirectory strings by stripping leading and
> trailing slashes before executing mounts, verifying mount status, and
> testing the SMB connection. This ensures working storages mount
> successfully and are correctly recognized as active.
>
> [0] https://docs.kernel.org/admin-guide/cifs/usage.html#cifs-vfs-mount-options
>
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6050
> Signed-off-by: David Riley <d.riley@proxmox.com>
> ---
> src/PVE/Storage/CIFSPlugin.pm | 32 ++++++++++++++++++++++++++++----
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/src/PVE/Storage/CIFSPlugin.pm b/src/PVE/Storage/CIFSPlugin.pm
> index 54f0f4e..78ed88c 100644
> --- a/src/PVE/Storage/CIFSPlugin.pm
> +++ b/src/PVE/Storage/CIFSPlugin.pm
> @@ -2,15 +2,20 @@ package PVE::Storage::CIFSPlugin;
>
> use strict;
> use warnings;
> +
> +use File::Path;
> use Net::IP;
> +
> use PVE::Tools qw(run_command);
> use PVE::ProcFSTools;
> -use File::Path;
> use PVE::Storage::Plugin;
> use PVE::JSONSchema qw(get_standard_option);
>
> use base qw(PVE::Storage::Plugin);
>
> +my $RM_LEADING_SLASHES = qr{^/+};
> +my $RM_TRAILING_SLASHES = qr{/+$};
Since you're removing the slashes at three different locations in this
file, I'd prefere having a function which does that. Would make the code
easier to read. You could also think about moving that function into the
common module, since I can imagine this is something we could use from
time to time. What do you think?
> +
> # CIFS helper functions
>
> sub cifs_is_mounted : prototype($$) {
> @@ -19,8 +24,16 @@ sub cifs_is_mounted : prototype($$) {
> my ($mountpoint, $server, $share) = $scfg->@{ 'path', 'server', 'share' };
> my $subdir = $scfg->{subdir} // '';
>
> + for my $path ($share, $subdir) {
Is stripping share actually needed? The documented workflow
(pvesm scan cifs <address>) never returns slashes, and since share
is a single name rather than a path, slashes there look more like
invalid input than a formatting variant.
> + $path =~ s/$RM_LEADING_SLASHES// if $path;
> + $path =~ s/$RM_TRAILING_SLASHES// if $path;
> + }
> +
> $server = "[$server]" if Net::IP::ip_is_ipv6($server);
> - my $source = "//${server}/$share$subdir";
> + my $source = "//${server}";
> + $source .= "/$share";
> + $source .= "/$subdir" if $subdir;
> +
> $mountdata = PVE::ProcFSTools::parse_proc_mounts() if !$mountdata;
>
> return $mountpoint if grep {
> @@ -83,8 +96,15 @@ sub cifs_mount : prototype($$$$$) {
> my ($mountpoint, $server, $share, $options) = $scfg->@{ 'path', 'server', 'share', 'options' };
> my $subdir = $scfg->{subdir} // '';
>
> + for my $path ($share, $subdir) {
> + $path =~ s/$RM_LEADING_SLASHES// if $path;
> + $path =~ s/$RM_TRAILING_SLASHES// if $path;
> + }
> +
> $server = "[$server]" if Net::IP::ip_is_ipv6($server);
> - my $source = "//${server}/$share$subdir";
> + my $source = "//${server}";
> + $source .= "/$share";
> + $source .= "/$subdir" if $subdir;
>
> my $cmd = ['/bin/mount', '-t', 'cifs', $source, $mountpoint, '-o', 'soft'];
>
> @@ -285,7 +305,11 @@ sub deactivate_storage {
> sub check_connection {
> my ($class, $storeid, $scfg) = @_;
>
> - my $servicename = '//' . $scfg->{server} . '/' . $scfg->{share};
> + my $share = $scfg->{share};
> + $share =~ s/$RM_LEADING_SLASHES// if $share;
> + $share =~ s/$RM_TRAILING_SLASHES// if $share;
> +
> + my $servicename = '//' . $scfg->{server} . '/' . $share;
>
> my $cmd = ['/usr/bin/smbclient', $servicename, '-d', '0'];
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH pve-storage 2/2] fix #6050: cifs: normalize share and subdir paths for mount and status
2026-08-18 10:23 ` Jakob Klocker
@ 2026-08-18 12:33 ` David Riley
0 siblings, 0 replies; 6+ messages in thread
From: David Riley @ 2026-08-18 12:33 UTC (permalink / raw)
To: Jakob Klocker, pve-devel
On 8/18/26 12:23 PM, Jakob Klocker wrote:
> Comments inline. I'd also suggest mentioning why you didn't reuse the
> `canonpath` function you used in the previous patch here.
Thanks for taking a look.
You are right I should have given some context as to why I did not use
canonpath for this patch.
The main reason is that subdir for example could be written as:
/<subdir>
and canonpath [0] is totally fine with this as this is a valid path and
therefore I opted to use regexes for this.
[0] https://perldoc.perl.org/File::Spec#canonpath
>
> On Fri Aug 7, 2026 at 11:36 AM CEST, David Riley wrote:
>> Manual edits to the storage configuration bypass strict API
>> validation. If a user adds extra or trailing slashes to the share or
>> subdirectory paths, it causes two distinct issues for CIFS storages:
>>
>> * The Linux VFS CIFS module expects strict UNC [0] formatting and
>> will reject the mount attempt with a 'Malformed UNC in devname'
>> error.
>> * The internal path used by the status daemon fails to match the
>> normalized path reported in /proc/mounts, causing working storages
>> to continuously report as 'inactive'.
>>
>> Sanitize the share and subdirectory strings by stripping leading and
>> trailing slashes before executing mounts, verifying mount status, and
>> testing the SMB connection. This ensures working storages mount
>> successfully and are correctly recognized as active.
>>
>> [0] https://docs.kernel.org/admin-guide/cifs/usage.html#cifs-vfs-mount-options
>>
>> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6050
>> Signed-off-by: David Riley <d.riley@proxmox.com>
>> ---
>> src/PVE/Storage/CIFSPlugin.pm | 32 ++++++++++++++++++++++++++++----
>> 1 file changed, 28 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/PVE/Storage/CIFSPlugin.pm b/src/PVE/Storage/CIFSPlugin.pm
>> index 54f0f4e..78ed88c 100644
>> --- a/src/PVE/Storage/CIFSPlugin.pm
>> +++ b/src/PVE/Storage/CIFSPlugin.pm
>> @@ -2,15 +2,20 @@ package PVE::Storage::CIFSPlugin;
>>
>> use strict;
>> use warnings;
>> +
>> +use File::Path;
>> use Net::IP;
>> +
>> use PVE::Tools qw(run_command);
>> use PVE::ProcFSTools;
>> -use File::Path;
>> use PVE::Storage::Plugin;
>> use PVE::JSONSchema qw(get_standard_option);
>>
>> use base qw(PVE::Storage::Plugin);
>>
>> +my $RM_LEADING_SLASHES = qr{^/+};
>> +my $RM_TRAILING_SLASHES = qr{/+$};
> Since you're removing the slashes at three different locations in this
> file, I'd prefere having a function which does that. Would make the code
> easier to read. You could also think about moving that function into the
> common module, since I can imagine this is something we could use from
> time to time. What do you think?
Fair. I considered this initially but was on the fence.
I agree though, and I'll pack this into a function in v2.
>
>> +
>> # CIFS helper functions
>>
>> sub cifs_is_mounted : prototype($$) {
>> @@ -19,8 +24,16 @@ sub cifs_is_mounted : prototype($$) {
>> my ($mountpoint, $server, $share) = $scfg->@{ 'path', 'server', 'share' };
>> my $subdir = $scfg->{subdir} // '';
>>
>> + for my $path ($share, $subdir) {
> Is stripping share actually needed? The documented workflow
> (pvesm scan cifs <address>) never returns slashes, and since share
> is a single name rather than a path, slashes there look more like
> invalid input than a formatting variant.
You are right that, strictly speaking, the share parameter is an entity name
and not a file path.
However, I opted to sanitize it as well to handle manual configuration errors
gracefully. I intuitively wrote 'share /share' out of habit, especially since it
ultimately gets concatenated into a path-like string (//server/share/subdir).
I'll add a brief note about this to the commit message in v2 as well.
Example:
cifs: smb-debian
path /mnt/pve/smb-debian
server 172.16.99.4
share /share
content import,rootdir,vztmpl,backup,images,iso,snippets
prune-backups keep-all=1
subdir /some/path/
>
>> + $path =~ s/$RM_LEADING_SLASHES// if $path;
>> + $path =~ s/$RM_TRAILING_SLASHES// if $path;
>> + }
>> +
>> $server = "[$server]" if Net::IP::ip_is_ipv6($server);
>> - my $source = "//${server}/$share$subdir";
>> + my $source = "//${server}";
>> + $source .= "/$share";
>> + $source .= "/$subdir" if $subdir;
>> +
>> $mountdata = PVE::ProcFSTools::parse_proc_mounts() if !$mountdata;
>>
>> return $mountpoint if grep {
>> @@ -83,8 +96,15 @@ sub cifs_mount : prototype($$$$$) {
>> my ($mountpoint, $server, $share, $options) = $scfg->@{ 'path', 'server', 'share', 'options' };
>> my $subdir = $scfg->{subdir} // '';
>>
>> + for my $path ($share, $subdir) {
>> + $path =~ s/$RM_LEADING_SLASHES// if $path;
>> + $path =~ s/$RM_TRAILING_SLASHES// if $path;
>> + }
>> +
>> $server = "[$server]" if Net::IP::ip_is_ipv6($server);
>> - my $source = "//${server}/$share$subdir";
>> + my $source = "//${server}";
>> + $source .= "/$share";
>> + $source .= "/$subdir" if $subdir;
>>
>> my $cmd = ['/bin/mount', '-t', 'cifs', $source, $mountpoint, '-o', 'soft'];
>>
>> @@ -285,7 +305,11 @@ sub deactivate_storage {
>> sub check_connection {
>> my ($class, $storeid, $scfg) = @_;
>>
>> - my $servicename = '//' . $scfg->{server} . '/' . $scfg->{share};
>> + my $share = $scfg->{share};
>> + $share =~ s/$RM_LEADING_SLASHES// if $share;
>> + $share =~ s/$RM_TRAILING_SLASHES// if $share;
>> +
>> + my $servicename = '//' . $scfg->{server} . '/' . $share;
>>
>> my $cmd = ['/usr/bin/smbclient', $servicename, '-d', '0'];
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-18 12:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-07 9:36 [PATCH storage 0/2] fix #6050: normalize paths in mount status checks David Riley
2026-08-07 9:36 ` [PATCH pve-storage 1/2] fix #6050: nfs: strip trailing slashes from mount status check David Riley
2026-08-18 9:35 ` Jakob Klocker
2026-08-07 9:36 ` [PATCH pve-storage 2/2] fix #6050: cifs: normalize share and subdir paths for mount and status David Riley
2026-08-18 10:23 ` Jakob Klocker
2026-08-18 12:33 ` David Riley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.