public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Leo Nunner <l.nunner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH storage] plugin: change name, separator and error message for dir overrides
Date: Thu,  5 Jan 2023 17:16:57 +0100	[thread overview]
Message-ID: <20230105161658.236402-2-l.nunner@proxmox.com> (raw)
In-Reply-To: <20230105161658.236402-1-l.nunner@proxmox.com>

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





  reply	other threads:[~2023-01-05 16:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-05 16:16 [pve-devel] [PATCH storage docs] follow-up: change formatting " Leo Nunner
2023-01-05 16:16 ` Leo Nunner [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230105161658.236402-2-l.nunner@proxmox.com \
    --to=l.nunner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal