* [pve-devel] [PATCH storage] plugin: change name, separator and error message for dir overrides
2023-01-05 16:16 [pve-devel] [PATCH storage docs] follow-up: change formatting for dir overrides Leo Nunner
@ 2023-01-05 16:16 ` Leo Nunner
2023-01-05 16:16 ` [pve-devel] [PATCH docs] docs: rename "dirs" parameter, clarify paths Leo Nunner
2023-01-09 9:51 ` [pve-devel] applied: [PATCH storage docs] follow-up: change formatting for dir overrides Wolfgang Bumiller
2 siblings, 0 replies; 4+ messages in thread
From: Leo Nunner @ 2023-01-05 16:16 UTC (permalink / raw)
To: pve-devel
Rename the "dirs" parameter to "content-dirs". Switch from a "vtype:/dir"
format to "vtype=/dir", and remove the misleading error message talking
about "absolute" paths. One might expect these to be absolute over the
whole system, while in reality they are relative to the mountpoint of
the storage.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
PVE/Storage/CIFSPlugin.pm | 2 +-
PVE/Storage/DirPlugin.pm | 2 +-
PVE/Storage/NFSPlugin.pm | 2 +-
PVE/Storage/Plugin.pm | 18 +++++++++---------
test/get_subdir_test.pm | 4 ++--
5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index 4284c35..d238844 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -128,7 +128,7 @@ sub properties {
sub options {
return {
path => { fixed => 1 },
- dirs => { optional => 1 },
+ 'content-dirs' => { optional => 1 },
server => { fixed => 1 },
share => { fixed => 1 },
nodes => { optional => 1 },
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index 3c907ca..9e305f5 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -54,7 +54,7 @@ sub properties {
sub options {
return {
path => { fixed => 1 },
- dirs => { optional => 1 },
+ 'content-dirs' => { optional => 1 },
nodes => { optional => 1 },
shared => { optional => 1 },
disable => { optional => 1 },
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index b7e8318..0bdde84 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -79,7 +79,7 @@ sub properties {
sub options {
return {
path => { fixed => 1 },
- dirs => { optional => 1 },
+ 'content-dirs' => { optional => 1 },
server => { fixed => 1 },
export => { fixed => 1 },
nodes => { optional => 1 },
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 5cb3d90..a3aac61 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -181,8 +181,8 @@ my $defaultData = {
default => 'metadata',
optional => 1,
},
- dirs => {
- description => "Overrides for default directories",
+ 'content-dirs' => {
+ description => "Overrides for default content type directories.",
type => "string", format => "pve-dir-override-list",
optional => 1,
},
@@ -213,7 +213,7 @@ sub valid_content_types {
sub dirs_hash_to_string {
my $hash = shift;
- return join(',', map { "$_:$hash->{$_}" } sort keys %$hash);
+ return join(',', map { "$_=$hash->{$_}" } sort keys %$hash);
}
sub default_format {
@@ -350,8 +350,8 @@ PVE::JSONSchema::register_format('pve-dir-override', \&verify_dir_override);
sub verify_dir_override {
my ($value, $noerr) = @_;
- if($value =~ m/^([a-z]+):(.+)$/ &&
- verify_content($1, $noerr) && verify_path($2, $noerr)) {
+ if($value =~ m/^([a-z]+)=\/.+$/ &&
+ verify_content($1, $noerr)) {
return $value;
}
@@ -429,12 +429,12 @@ sub decode_value {
#}
return $res;
- } elsif ($key eq 'dirs') {
+ } elsif ($key eq 'content-dirs') {
my $valid_content = $def->{content}->[0];
my $res = {};
foreach my $dir (PVE::Tools::split_list($value)) {
- my ($content, $path) = split(/:/, $dir, 2);
+ my ($content, $path) = split(/=/, $dir, 2);
if (!$valid_content->{$content}) {
warn "storage does not support content type '$content'\n";
@@ -458,7 +458,7 @@ sub encode_value {
} elsif ($key eq 'content') {
my $res = content_hash_to_string($value) || 'none';
return $res;
- } elsif ($key eq 'dirs') {
+ } elsif ($key eq 'content-dirs') {
my $res = dirs_hash_to_string($value);
return $res;
}
@@ -654,7 +654,7 @@ sub get_subdir {
die "storage definition has no path\n" if !$path;
die "unknown vtype '$vtype'\n" if !exists($vtype_subdirs->{$vtype});
- my $subdir = $scfg->{dirs}->{$vtype} // "/".$vtype_subdirs->{$vtype};
+ my $subdir = $scfg->{"content-dirs"}->{$vtype} // "/".$vtype_subdirs->{$vtype};
return $path.$subdir;
}
diff --git a/test/get_subdir_test.pm b/test/get_subdir_test.pm
index 26c08d5..ff42985 100644
--- a/test/get_subdir_test.pm
+++ b/test/get_subdir_test.pm
@@ -30,8 +30,8 @@ foreach my $type (keys %$vtype_subdirs) {
# creates additional tests for overrides
foreach my $type (keys %$vtype_subdirs) {
my $override = "/${type}_override";
- my $scfg_with_override = { path => '/some/path', dirs => { $type => $override } };
- push @$tests, [ $scfg_with_override, $type, "$scfg_with_override->{path}$scfg_with_override->{dirs}->{$type}" ];
+ my $scfg_with_override = { path => '/some/path', 'content-dirs' => { $type => $override } };
+ push @$tests, [ $scfg_with_override, $type, "$scfg_with_override->{path}$scfg_with_override->{'content-dirs'}->{$type}" ];
}
plan tests => scalar @$tests;
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread* [pve-devel] [PATCH docs] docs: rename "dirs" parameter, clarify paths
2023-01-05 16:16 [pve-devel] [PATCH storage docs] follow-up: change formatting for dir overrides Leo Nunner
2023-01-05 16:16 ` [pve-devel] [PATCH storage] plugin: change name, separator and error message " Leo Nunner
@ 2023-01-05 16:16 ` Leo Nunner
2023-01-09 9:51 ` [pve-devel] applied: [PATCH storage docs] follow-up: change formatting for dir overrides Wolfgang Bumiller
2 siblings, 0 replies; 4+ messages in thread
From: Leo Nunner @ 2023-01-05 16:16 UTC (permalink / raw)
To: pve-devel
Chagne the name of the "dirs" parameter to "content-dirs" and be less
misleading with "absolute paths".
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
pve-storage-cifs.adoc | 2 +-
pve-storage-dir.adoc | 13 +++++++------
pve-storage-nfs.adoc | 2 +-
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/pve-storage-cifs.adoc b/pve-storage-cifs.adoc
index 4d93727..e0d4106 100644
--- a/pve-storage-cifs.adoc
+++ b/pve-storage-cifs.adoc
@@ -57,7 +57,7 @@ path::
The local mount point. Optional, defaults to `/mnt/pve/<STORAGE_ID>/`.
-dirs::
+content-dirs::
Overrides for the default directory layout. Optional.
diff --git a/pve-storage-dir.adoc b/pve-storage-dir.adoc
index ec1b957..cd3fa9e 100644
--- a/pve-storage-dir.adoc
+++ b/pve-storage-dir.adoc
@@ -50,14 +50,15 @@ This backend supports all common storage properties, and adds two
additional properties. The `path` property is used to specify the
directory. This needs to be an absolute file system path.
-The optional `dirs` property allows for the default layout to be
-changed. It consists of a comma-separated list of identifiers in
-the following format:
+The optional `content-dirs` property allows for the default layout
+to be changed. It consists of a comma-separated list of identifiers
+in the following format:
- vtype:path
+ vtype=path
Where `vtype` is one of the allowed content types for the storage, and
-`path` is an absolute file system path.
+`path` is a path relative to the mountpoint of the storage, preceded
+with /.
.Configuration Example (`/etc/pve/storage.cfg`)
----
@@ -66,7 +67,7 @@ dir: backup
content backup
prune-backups keep-last=7
max-protected-backups 3
- dirs backup:/custom/backup/dir
+ content-dirs backup=/custom/backup/dir
----
The above configuration defines a storage pool called `backup`. That pool can be
diff --git a/pve-storage-nfs.adoc b/pve-storage-nfs.adoc
index 95ee45d..ce62aa0 100644
--- a/pve-storage-nfs.adoc
+++ b/pve-storage-nfs.adoc
@@ -40,7 +40,7 @@ path::
The local mount point (defaults to `/mnt/pve/<STORAGE_ID>/`).
-dirs::
+content-dirs::
Overrides for the default directory layout. Optional.
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread