[pve-devel] [PATCH storage] fix 3214: storage dir structure creation with mkdir 0

[pve-devel] [PATCH storage] fix 3214: storage dir structure creation with mkdir 0

[Aaron Lauterer]

We fail early when `mkdir 0` is set for the storage to avoid creating
the directories in the storage path.

This means that once `mkdir 0` is set, the code to create the needed
directory structure (e.g. dump, image, ...) at the storage location will
never run.

Adding an additional check to only return early if the storage path
currently does not exist solves the problem.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/Storage/Plugin.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 57c58a9..bda1beb 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1158,8 +1158,8 @@ sub activate_storage {
 	"directory '$path' does not exist or is unreachable\n";
     }
 
-
-    return if defined($scfg->{mkdir}) && !$scfg->{mkdir};
+    my $st = File::stat::stat($path);
+    return if defined($scfg->{mkdir}) && !$scfg->{mkdir} && !defined($st);
 
     if (defined($scfg->{content})) {
 	foreach my $vtype (keys %$vtype_subdirs) {
-- 
2.20.1

Re: [pve-devel] [PATCH storage] fix 3214: storage dir structure creation with mkdir 0

[Wolfgang Bumiller]

On Tue, Dec 22, 2020 at 11:53:40AM +0100, Aaron Lauterer wrote:
> We fail early when `mkdir 0` is set for the storage to avoid creating
> the directories in the storage path.
> 
> This means that once `mkdir 0` is set, the code to create the needed
> directory structure (e.g. dump, image, ...) at the storage location will
> never run.
> 
> Adding an additional check to only return early if the storage path
> currently does not exist solves the problem.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  PVE/Storage/Plugin.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 57c58a9..bda1beb 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -1158,8 +1158,8 @@ sub activate_storage {
>  	"directory '$path' does not exist or is unreachable\n";
>      }
>  
> -
> -    return if defined($scfg->{mkdir}) && !$scfg->{mkdir};
> +    my $st = File::stat::stat($path);
> +    return if defined($scfg->{mkdir}) && !$scfg->{mkdir} && !defined($st);

Instead of calling stat you can just use the `-d` prefix operator.

>  
>      if (defined($scfg->{content})) {
>  	foreach my $vtype (keys %$vtype_subdirs) {
> -- 
> 2.20.1

Re: [pve-devel] [PATCH storage] fix 3214: storage dir structure creation with mkdir 0

[Wolfgang Bumiller]

On Tue, Dec 22, 2020 at 01:57:25PM +0100, Wolfgang Bumiller wrote:
> On Tue, Dec 22, 2020 at 11:53:40AM +0100, Aaron Lauterer wrote:
> > We fail early when `mkdir 0` is set for the storage to avoid creating
> > the directories in the storage path.
> > 
> > This means that once `mkdir 0` is set, the code to create the needed
> > directory structure (e.g. dump, image, ...) at the storage location will
> > never run.
> > 
> > Adding an additional check to only return early if the storage path
> > currently does not exist solves the problem.
> > 
> > Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> > ---
> >  PVE/Storage/Plugin.pm | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> > index 57c58a9..bda1beb 100644
> > --- a/PVE/Storage/Plugin.pm
> > +++ b/PVE/Storage/Plugin.pm
> > @@ -1158,8 +1158,8 @@ sub activate_storage {
> >  	"directory '$path' does not exist or is unreachable\n";
> >      }
> >  
> > -
> > -    return if defined($scfg->{mkdir}) && !$scfg->{mkdir};
> > +    my $st = File::stat::stat($path);
> > +    return if defined($scfg->{mkdir}) && !$scfg->{mkdir} && !defined($st);
> 
> Instead of calling stat you can just use the `-d` prefix operator.

scratch that, we already perform that check right above this code...
and `mkdir` here changes meaning from "create the path to the storage"
to "create the directries *inside* the storage"... this is bad :|