Re: [pve-devel] [PATCH pve-storage v1 3/3] plugin: improve error handling when creating a content subdir fails

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On December 5, 2025 4:43 pm, Max R. Carrara wrote:
> Wrap the call to `mkpath` in an `eval` and add some additional context
> to the error message, namely that creating the content directory for a
> given vtype failed.
> 
> Handle "Permission denied" and "Read-only file system" errors
> separately and tell the user to ensure that the storage has read and
> write access when such errors occurs.
> 
> Do all this with the help of an anonymous subroutine, because the body
> inside the loop would otherwise get a little convoluted / hard to
> parse. Also, use the `/x` flag for the regexes to make them a tiny bit
> easier to read.
> 
> Spotted in the community forum:
> https://forum.proxmox.com/threads/bug-creating-nfs-volume-storage.177354/
> 
> Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
> ---
> 
> Notes:
>     I ran into the second branch (the one for "Read-only file system")
>     when testing things using my local NFS share—I figured I might as
>     well handle that one too, since it's similar in nature.
> 
>  src/PVE/Storage/Plugin.pm | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 617b0f8..975641b 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1903,6 +1903,28 @@ sub activate_storage {
>          return;
>      }
>  
> +    my $try_create_subdir = sub {
> +        my ($vtype) = @_;
> +
> +        my $subdir = $class->get_subdir($scfg, $vtype);
> +
> +        eval { mkpath $subdir; };
> +        if ($@) {
> +            my $msg = "failed to create content directory '$subdir' for content '$vtype'";
> +            my $msg_prompt = "ensure that the storage has read and write access!";
> +
> +            if ($@ =~ m/permission \s denied/ix) {
> +                die "$msg - permission denied - $msg_prompt\n";
> +            }

if we do this, then using make_path with the `error` option would IMHO
be better for extracting/matching the cause

> +            if ($@ =~ m/read -? only \s file \s? system/ix) {
> +                die "$msg - read-only file system - $msg_prompt\n";
> +            }
> +
> +            die "$msg - $@\n";
> +        }
> +    };
> +
>      # TODO: mkdir is basically deprecated since 8.0, but we don't warn here until 8.4 or 9.0, as we
>      # only got the replacement in 8.0, so no real replacement window, and its really noisy.
>  
> @@ -1918,8 +1940,7 @@ sub activate_storage {
>                  defined($scfg->{content}->{$vtype})
>                  || ($vtype eq 'backup' && defined($scfg->{content}->{'rootdir'}))
>              ) {
> -                my $subdir = $class->get_subdir($scfg, $vtype);
> -                mkpath $subdir;
> +                $try_create_subdir->($vtype);
>              }
>          }
>      }
> -- 
> 2.47.3
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 


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