* [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
* 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
* [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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox