* [pve-devel] [PATCH v2 storage] deprecate mkdir option for create-path and create-sub-dirs
@ 2021-05-28 15:05 Aaron Lauterer
0 siblings, 0 replies; only message in thread
From: Aaron Lauterer @ 2021-05-28 15:05 UTC (permalink / raw)
To: pve-devel
The `mkdir` option has two meanings[0][1] which are split up in `create-path`
and `create-sub-dirs`.
The `create-path` option decides if the path to the storage is
automatically created or not.
The `create-sub-dirs` 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-path` option
is now run after the `is_mountpoint` check in the `activate_storage`
method in DirPlugin.pm.
Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
[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>
---
changes from v1: (thx @fabian for the review [2])
* adjusted parameters to use - instead of _
* renamed populate_directory with create-sub-dirs
One thing we should also check is how to handle default storages, mainly
`local` which AFAICT has `mkdir 1` set which will trigger deprecation
warnings in the syslog.
[2] https://lists.proxmox.com/pipermail/pve-devel/2021-April/047880.html
PVE/Storage/CIFSPlugin.pm | 4 +++-
PVE/Storage/CephFSPlugin.pm | 4 +++-
PVE/Storage/DirPlugin.pm | 19 ++++++++++++++++---
PVE/Storage/GlusterfsPlugin.pm | 4 +++-
PVE/Storage/NFSPlugin.pm | 4 +++-
PVE/Storage/Plugin.pm | 22 ++++++++++++++++++++++
6 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index be06cc7..ce3410e 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -142,6 +142,8 @@ sub options {
domain => { optional => 1},
smbversion => { optional => 1},
mkdir => { optional => 1 },
+ 'create-path' => { optional => 1 },
+ 'create-sub-dirs' => { optional => 1 },
bwlimit => { optional => 1 },
};
}
@@ -225,7 +227,7 @@ sub activate_storage {
if (!cifs_is_mounted($server, $share, $path, $cache->{mountdata})) {
- mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir});
+ mkpath $path if $class->should_create_path($scfg);
die "unable to activate storage '$storeid' - " .
"directory '$path' does not exist\n" if ! -d $path;
diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
index 480dc57..0925f0a 100644
--- a/PVE/Storage/CephFSPlugin.pm
+++ b/PVE/Storage/CephFSPlugin.pm
@@ -147,6 +147,8 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
+ 'create-path' => { optional => 1 },
+ 'create-sub-dirs' => { optional => 1 },
fuse => { optional => 1 },
bwlimit => { optional => 1 },
maxfiles => { optional => 1 },
@@ -203,7 +205,7 @@ sub activate_storage {
if (!cephfs_is_mounted($scfg, $storeid, $cache->{mountdata})) {
my $path = $scfg->{path};
- mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir});
+ mkpath $path if $class->should_create_path($scfg);
die "unable to activate storage '$storeid' - " .
"directory '$path' does not exist\n" if ! -d $path;
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index 2267f11..0dd5201 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -30,10 +30,22 @@ sub properties {
type => 'string', format => 'pve-storage-path',
},
mkdir => {
+ description =>
+ "Deprecated, use the 'create-path' and " .
+ "'create-sub-dirs' options instead.",
+ type => 'boolean',
+ default => 'yes',
+ },
+ 'create-path' => {
description => "Create the directory if it doesn't exist.",
type => 'boolean',
default => 'yes',
},
+ 'create-sub-dirs' => {
+ description => "Populate the directory with the default structure.",
+ type => 'boolean',
+ default => 'yes',
+ },
is_mountpoint => {
description =>
"Assume the given path is an externally managed mountpoint " .
@@ -57,6 +69,8 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
+ 'create-path' => { optional => 1 },
+ 'create-sub-dirs' => { optional => 1 },
is_mountpoint => { optional => 1 },
bwlimit => { optional => 1 },
};
@@ -133,9 +147,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})) {
@@ -143,6 +154,8 @@ sub activate_storage {
"directory is expected to be a mount point but is not mounted: '$mp'\n";
}
+ mkpath $path if $class->should_create_path($scfg);
+
$class->SUPER::activate_storage($storeid, $scfg, $cache);
}
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index 5ec2f42..17f8165 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -136,6 +136,8 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
+ 'create-path' => { optional => 1 },
+ 'create-sub-dirs' => { optional => 1 },
bwlimit => { optional => 1 },
};
}
@@ -300,7 +302,7 @@ sub activate_storage {
if (!glusterfs_is_mounted($volume, $path, $cache->{mountdata})) {
- mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir});
+ mkpath $path if $class->should_create_path($scfg);
die "unable to activate storage '$storeid' - " .
"directory '$path' does not exist\n" if ! -d $path;
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index 39bf15a..3adfcc3 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -89,6 +89,8 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
+ 'create-path' => { optional => 1 },
+ 'create-sub-dirs' => { optional => 1 },
bwlimit => { optional => 1 },
};
}
@@ -131,7 +133,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});
+ mkpath $path if $class->should_create_path($scfg);
die "unable to activate storage '$storeid' - " .
"directory '$path' does not exist\n" if ! -d $path;
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 4a10a1f..4ab9ca4 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1163,7 +1163,13 @@ sub activate_storage {
"directory '$path' does not exist or is unreachable\n";
}
+ warn "'mkdir' option is deprecated. Use 'create-path' or 'create-sub-dirs' instead.\n"
+ if defined($scfg->{mkdir});
+ return if defined($scfg->{'create-sub-dirs'})
+ && !$scfg->{'create-sub-dirs'};
+
+ # FIXME The mkdir option is deprecated and create-sub-dirs should be used
return if defined($scfg->{mkdir}) && !$scfg->{mkdir};
if (defined($scfg->{content})) {
@@ -1463,4 +1469,20 @@ sub volume_import_formats {
return ();
}
+sub should_create_path {
+ my ($class, $scfg) = @_;
+
+ # FIXME the mkdir parameter is deprecated and create_path should be used
+ my $mkpath = 0;
+ if (!defined($scfg->{'create-path'}) && !defined($scfg->{mkdir})) {
+ $mkpath = 1;
+ } elsif (defined($scfg->{'create-path'})) {
+ $mkpath = 1 if $scfg->{'create-path'};
+ } else {
+ $mkpath = 1 if $scfg->{mkdir};
+ }
+
+ return $mkpath;
+}
+
1;
--
2.20.1
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-05-28 15:05 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-28 15:05 [pve-devel] [PATCH v2 storage] deprecate mkdir option for create-path and create-sub-dirs Aaron Lauterer
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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal