yesBad/pve-storage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

deprecate mkdir option for create-base-path and create-subdirs

Browse Source 
The `mkdir` option has two meanings[0][1] which are split up in `create-path`
and `create-sub-dirs`.

The `create-base-path` option decides if the path to the storage is
automatically created or not.
The `create-subdirs` options decides if the default directory
structure (dump, images, ...) at the storage location is created.

The `mkdir` option is still working but will trigger a warning in the
logs.

As a side effect, this also fixes #3214 because the `create-base-path` option
is now run after the `is_mountpoint` check in the `activate_storage`
method in DirPlugin.pm.

The 'mkpath' command has been moved into a new helper function that
first determines if the conditions to create the path is true, called
'config_aware_base_mkdir'.

[0] https://lists.proxmox.com/pipermail/pve-devel/2020-December/046575.html
[1] https://lists.proxmox.com/pipermail/pve-devel/2020-December/046576.html

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
This commit is contained in:
Aaron Lauterer
2023-05-31 14:46:09 +02:00
committed by Wolfgang Bumiller
parent 39d9fb2440
commit 7c242295c9
7 changed files with 50 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/PVE/Storage/BTRFSPlugin.pm
View File

@ -74,6 +74,8 @@ sub options {
is_mountpoint => { optional => 1 },
nocow => { optional => 1 },
mkdir => { optional => 1 },
'create-base-path' => { optional => 1 },
'create-subdirs' => { optional => 1 },
preallocation => { optional => 1 },
# TODO: The new variant of mkdir with `populate` vs `create`...
};
@ -118,9 +120,7 @@ sub activate_storage {
my ($class, $storeid, $scfg, $cache) = @_;
my $path = $scfg->{path};
if (!defined($scfg->{mkdir}) || $scfg->{mkdir}) {
mkpath $path;
}
$class->config_aware_base_mkdir($scfg, $path);
my $mp = PVE::Storage::DirPlugin::parse_is_mountpoint($scfg);
if (defined($mp) && !PVE::Storage::DirPlugin::path_is_mounted($mp, $cache->{mountdata})) {

4
src/PVE/Storage/CIFSPlugin.pm
View File

@ -150,6 +150,8 @@ sub options {
domain => { optional => 1},
smbversion => { optional => 1},
mkdir => { optional => 1 },
'create-base-path' => { optional => 1 },
'create-subdirs' => { optional => 1 },
bwlimit => { optional => 1 },
preallocation => { optional => 1 },
};
@ -228,7 +230,7 @@ sub activate_storage {
if (!cifs_is_mounted($scfg, $cache->{mountdata})) {
mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir});
$class->config_aware_base_mkdir($scfg, $path);
die "unable to activate storage '$storeid' - " .
"directory '$path' does not exist\n" if ! -d $path;

4
src/PVE/Storage/CephFSPlugin.pm
View File

@ -147,6 +147,8 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
'create-base-path' => { optional => 1 },
'create-subdirs' => { optional => 1 },
fuse => { optional => 1 },
bwlimit => { optional => 1 },
maxfiles => { optional => 1 },
@ -214,7 +216,7 @@ sub activate_storage {
if (!cephfs_is_mounted($scfg, $storeid, $cache->{mountdata})) {
my $path = $scfg->{path};
mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir});
$class->config_aware_base_mkdir($scfg, $path);
die "unable to activate storage '$storeid' - " .
"directory '$path' does not exist\n" if ! -d $path;

19
src/PVE/Storage/DirPlugin.pm
View File

@ -35,7 +35,18 @@ sub properties {
type => 'string', format => 'pve-storage-path',
},
mkdir => {
description => "Create the directory if it doesn't exist.",
description => "Create the directory if it doesn't exist and populate it with default sub-dirs."
." NOTE: Deprecated, use the 'create-base-path' and 'create-subdirs' options instead.",
type => 'boolean',
default => 'yes',
},
'create-base-path' => {
description => "Create the base directory if it doesn't exist.",
type => 'boolean',
default => 'yes',
},
'create-subdirs' => {
description => "Populate the directory with the default structure.",
type => 'boolean',
default => 'yes',
},
@ -64,6 +75,8 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
'create-base-path' => { optional => 1 },
'create-subdirs' => { optional => 1 },
is_mountpoint => { optional => 1 },
bwlimit => { optional => 1 },
preallocation => { optional => 1 },
@ -213,9 +226,6 @@ sub activate_storage {
my ($class, $storeid, $scfg, $cache) = @_;
my $path = $scfg->{path};
if (!defined($scfg->{mkdir}) || $scfg->{mkdir}) {
mkpath $path;
}
my $mp = parse_is_mountpoint($scfg);
if (defined($mp) && !path_is_mounted($mp, $cache->{mountdata})) {
@ -223,6 +233,7 @@ sub activate_storage {
"directory is expected to be a mount point but is not mounted: '$mp'\n";
}
$class->config_aware_base_mkdir($scfg, $path);
$class->SUPER::activate_storage($storeid, $scfg, $cache);
}

5
src/PVE/Storage/GlusterfsPlugin.pm
View File

@ -137,6 +137,8 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
'create-base-path' => { optional => 1 },
'create-subdirs' => { optional => 1 },
bwlimit => { optional => 1 },
preallocation => { optional => 1 },
};
@ -302,8 +304,7 @@ sub activate_storage {
my $volume = $scfg->{volume};
if (!glusterfs_is_mounted($volume, $path, $cache->{mountdata})) {
mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir});
$class->config_aware_base_mkdir($scfg, $path);
die "unable to activate storage '$storeid' - " .
"directory '$path' does not exist\n" if ! -d $path;

4
src/PVE/Storage/NFSPlugin.pm
View File

@ -91,6 +91,8 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
'create-base-path' => { optional => 1 },
'create-subdirs' => { optional => 1 },
bwlimit => { optional => 1 },
preallocation => { optional => 1 },
};
@ -134,7 +136,7 @@ sub activate_storage {
if (!nfs_is_mounted($server, $export, $path, $cache->{mountdata})) {
# NOTE: only call mkpath when not mounted (avoid hang when NFS server is offline
mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir});
$class->config_aware_base_mkdir($scfg, $path);
die "unable to activate storage '$storeid' - " .
"directory '$path' does not exist\n" if ! -d $path;

20
src/PVE/Storage/Plugin.pm
View File

@ -1362,7 +1362,12 @@ sub activate_storage {
"directory '$path' does not exist or is unreachable\n";
}
warn "${storeid}: 'mkdir' option is deprecated. Use 'create-base-path' or 'create-subdirs' instead.\n"
if defined($scfg->{mkdir});
return if defined($scfg->{'create-subdirs'}) && !$scfg->{'create-subdirs'};
# FIXME The mkdir option is deprecated. Remove with PVE 9?
return if defined($scfg->{mkdir}) && !$scfg->{mkdir};
if (defined($scfg->{content})) {
@ -1698,4 +1703,19 @@ sub rename_volume {
return "${storeid}:${base}${target_vmid}/${target_volname}";
}
sub config_aware_base_mkdir {
my ($class, $scfg, $path) = @_;
# FIXME the mkdir parameter is deprecated and create-base-path should be used
my $mkpath = 0;
if (!defined($scfg->{'create-base-path'}) && !defined($scfg->{mkdir})) {
$mkpath = 1;
} elsif (defined($scfg->{'create-base-path'}) && $scfg->{'create-base-path'}) {
$mkpath = 1;
} elsif ($scfg->{mkdir}) {
$mkpath = 1;
}
mkpath $path if $mkpath;
}
1;