[pve-devel] [PATCH storage docs] follow-up: change formatting for dir overrides

[pve-devel] [PATCH storage docs] follow-up: change formatting for dir overrides

[Leo Nunner]

The "dirs" parameter was renamed to "content-dirs" for clarity.

Changed the separator for overrides to '=' instead of ':' (so,
"vtype=/dir"), and remove a misleading error message talking about
absolute paths; rationale in commit message.

storage:

Leo Nunner (1):
  plugin: change name, separator and error message for dir overrides

 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(-)

docs:

Leo Nunner (1):
  docs: rename "dirs" parameter, clarify paths

 pve-storage-cifs.adoc |  2 +-
 pve-storage-dir.adoc  | 13 +++++++------
 pve-storage-nfs.adoc  |  2 +-
 3 files changed, 9 insertions(+), 8 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH storage] plugin: change name, separator and error message for dir overrides

[Leo Nunner]

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

[pve-devel] [PATCH docs] docs: rename "dirs" parameter, clarify paths

[Leo Nunner]

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

[pve-devel] applied: [PATCH storage docs] follow-up: change formatting for dir overrides

[Wolfgang Bumiller]

applied both patches, thanks