From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id E884970B9B
for <pve-devel@lists.proxmox.com>; Thu, 30 Sep 2021 13:42:27 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id E18171EF16
for <pve-devel@lists.proxmox.com>; Thu, 30 Sep 2021 13:42:27 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS id 891EC1ED4B
for <pve-devel@lists.proxmox.com>; Thu, 30 Sep 2021 13:42:20 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6237F44C3E
for <pve-devel@lists.proxmox.com>; Thu, 30 Sep 2021 13:42:20 +0200 (CEST)
From: Fabian Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Thu, 30 Sep 2021 13:42:06 +0200
Message-Id: <20210930114215.240095-4-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.30.2
In-Reply-To: <20210930114215.240095-1-f.ebner@proxmox.com>
References: <20210930114215.240095-1-f.ebner@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.047 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [cifsplugin.pm, btrfsplugin.pm, plugin.pm, storage.pm,
pbsplugin.pm, nfsplugin.pm, cephfsplugin.pm, dirplugin.pm, content.pm]
URI_NOVOWEL 0.5 URI hostname has long non-vowel sequence
Subject: [pve-devel] [PATCH v2 storage 3/7] add generalized functions to
manage volume attributes
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 30 Sep 2021 11:42:27 -0000
replacing the ones for handling notes. To ensure backwards
compatibility with external plugins, all plugins that do not just call
another implementation need to call $class->{get, update}_volume_notes
when the attribute is 'notes' to catch any derived implementations.
This is mainly done to avoid the need to add new methods every time a
new attribute is added.
Not adding a timeout parameter like the notes functions have, because
it was not used and can still be added if it ever is needed in the
future.
For get_volume_attribute, undef will indicate that the attribute is
not supported. This makes it possible to distinguish "not supported"
from "error getting the attribute", which is useful when the attribute
is important for an operation. For example, free_image checking for
protection (introduced in a later patch) can abort if getting the
'protected' attribute fails.
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
Changes from v1:
* Use fall-back in DirPlugin and PBSPlugin too, and keep exisitng
{get, update}_volume_notes implementations around for now.
Requires an APIAGE+APIVER bump, but so does the whole series.
The alternative to undef indicating "not supported" would be to add a
separate method like supports_volume_attribute. Or adapt
volume_has_feature, but it doesn't really feel like the right fit as
it's currently intended for features for guest images only.
PVE/API2/Storage/Content.pm | 7 ++++---
PVE/Storage.pm | 12 ++++++------
PVE/Storage/BTRFSPlugin.pm | 4 ++--
PVE/Storage/CIFSPlugin.pm | 13 +++++++++++++
PVE/Storage/CephFSPlugin.pm | 12 ++++++++++++
PVE/Storage/DirPlugin.pm | 24 ++++++++++++++++++++++++
PVE/Storage/NFSPlugin.pm | 13 +++++++++++++
PVE/Storage/PBSPlugin.pm | 24 ++++++++++++++++++++++++
PVE/Storage/Plugin.pm | 36 ++++++++++++++++++++++++++++++++++++
9 files changed, 134 insertions(+), 11 deletions(-)
diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
index 4d0ceb6..b3dc593 100644
--- a/PVE/API2/Storage/Content.pm
+++ b/PVE/API2/Storage/Content.pm
@@ -321,11 +321,12 @@ __PACKAGE__->register_method ({
format => $format,
};
- # not all storages/types support notes, so ignore errors here
+ # keep going if fetching an optional attribute fails
eval {
- my $notes = PVE::Storage::get_volume_notes($cfg, $volid);
+ my $notes = PVE::Storage::get_volume_attribute($cfg, $volid, 'notes');
$entry->{notes} = $notes if defined($notes);
};
+ warn $@ if $@;
return $entry;
}});
@@ -371,7 +372,7 @@ __PACKAGE__->register_method ({
PVE::Storage::check_volume_access($rpcenv, $authuser, $cfg, undef, $volid);
if (exists $param->{notes}) {
- PVE::Storage::update_volume_notes($cfg, $volid, $param->{notes});
+ PVE::Storage::update_volume_attribute($cfg, $volid, 'notes', $param->{notes});
}
return undef;
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 7b319fe..729c90e 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -215,24 +215,24 @@ sub file_size_info {
return PVE::Storage::Plugin::file_size_info($filename, $timeout);
}
-sub get_volume_notes {
- my ($cfg, $volid, $timeout) = @_;
+sub get_volume_attribute {
+ my ($cfg, $volid, $attribute) = @_;
my ($storeid, $volname) = parse_volume_id($volid);
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
- return $plugin->get_volume_notes($scfg, $storeid, $volname, $timeout);
+ return $plugin->get_volume_attribute($scfg, $storeid, $volname, $attribute);
}
-sub update_volume_notes {
- my ($cfg, $volid, $notes, $timeout) = @_;
+sub update_volume_attribute {
+ my ($cfg, $volid, $attribute, $value) = @_;
my ($storeid, $volname) = parse_volume_id($volid);
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
- $plugin->update_volume_notes($scfg, $storeid, $volname, $notes, $timeout);
+ return $plugin->update_volume_attribute($scfg, $storeid, $volname, $attribute, $value);
}
sub volume_size_info {
diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm
index b30000b..c3e9b32 100644
--- a/PVE/Storage/BTRFSPlugin.pm
+++ b/PVE/Storage/BTRFSPlugin.pm
@@ -136,9 +136,9 @@ sub status {
return PVE::Storage::DirPlugin::status($class, $storeid, $scfg, $cache);
}
-# TODO: sub get_volume_notes {}
+# TODO: sub get_volume_attribute {}
-# TODO: sub update_volume_notes {}
+# TODO: sub update_volume_attribute {}
# croak would not include the caller from within this module
sub __error {
diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index c5f3894..890b378 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -286,13 +286,26 @@ sub check_connection {
return 1;
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use get_volume_attribute instead.
sub get_volume_notes {
my $class = shift;
PVE::Storage::DirPlugin::get_volume_notes($class, @_);
}
+
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use update_volume_attribute instead.
sub update_volume_notes {
my $class = shift;
PVE::Storage::DirPlugin::update_volume_notes($class, @_);
}
+sub get_volume_attribute {
+ return PVE::Storage::DirPlugin::get_volume_attribute(@_);
+}
+
+sub update_volume_attribute {
+ return PVE::Storage::DirPlugin::update_volume_attribute(@_);
+}
+
1;
diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
index 3b9a791..1fd4c47 100644
--- a/PVE/Storage/CephFSPlugin.pm
+++ b/PVE/Storage/CephFSPlugin.pm
@@ -232,14 +232,26 @@ sub deactivate_storage {
}
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use get_volume_attribute instead.
sub get_volume_notes {
my $class = shift;
PVE::Storage::DirPlugin::get_volume_notes($class, @_);
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use update_volume_attribute instead.
sub update_volume_notes {
my $class = shift;
PVE::Storage::DirPlugin::update_volume_notes($class, @_);
}
+sub get_volume_attribute {
+ return PVE::Storage::DirPlugin::get_volume_attribute(@_);
+}
+
+sub update_volume_attribute {
+ return PVE::Storage::DirPlugin::update_volume_attribute(@_);
+}
+
1;
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index 0ac9b06..b496d06 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -90,6 +90,8 @@ sub parse_is_mountpoint {
return $is_mp; # contains a path
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use get_volume_attribute instead.
sub get_volume_notes {
my ($class, $scfg, $storeid, $volname, $timeout) = @_;
@@ -104,6 +106,8 @@ sub get_volume_notes {
return '';
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use update_volume_attribute instead.
sub update_volume_notes {
my ($class, $scfg, $storeid, $volname, $notes, $timeout) = @_;
@@ -121,6 +125,26 @@ sub update_volume_notes {
return;
}
+sub get_volume_attribute {
+ my ($class, $scfg, $storeid, $volname, $attribute) = @_;
+
+ if ($attribute eq 'notes') {
+ return $class->get_volume_notes($scfg, $storeid, $volname);
+ }
+
+ return;
+}
+
+sub update_volume_attribute {
+ my ($class, $scfg, $storeid, $volname, $attribute, $value) = @_;
+
+ if ($attribute eq 'notes') {
+ return $class->update_volume_notes($scfg, $storeid, $volname, $value);
+ }
+
+ die "attribute '$attribute' is not supported for storage type '$scfg->{type}'\n";
+}
+
sub status {
my ($class, $storeid, $scfg, $cache) = @_;
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index 39bf15a..bc57e35 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -188,13 +188,26 @@ sub check_connection {
return 1;
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use get_volume_attribute instead.
sub get_volume_notes {
my $class = shift;
PVE::Storage::DirPlugin::get_volume_notes($class, @_);
}
+
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use update_volume_attribute instead.
sub update_volume_notes {
my $class = shift;
PVE::Storage::DirPlugin::update_volume_notes($class, @_);
}
+sub get_volume_attribute {
+ return PVE::Storage::DirPlugin::get_volume_attribute(@_);
+}
+
+sub update_volume_attribute {
+ return PVE::Storage::DirPlugin::update_volume_attribute(@_);
+}
+
1;
diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index bb1c382..76699e6 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -782,6 +782,8 @@ sub deactivate_volume {
return 1;
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use get_volume_attribute instead.
sub get_volume_notes {
my ($class, $scfg, $storeid, $volname, $timeout) = @_;
@@ -792,6 +794,8 @@ sub get_volume_notes {
return $data->{notes};
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use update_volume_attribute instead.
sub update_volume_notes {
my ($class, $scfg, $storeid, $volname, $notes, $timeout) = @_;
@@ -802,6 +806,26 @@ sub update_volume_notes {
return undef;
}
+sub get_volume_attribute {
+ my ($class, $scfg, $storeid, $volname, $attribute) = @_;
+
+ if ($attribute eq 'notes') {
+ return $class->get_volume_notes($scfg, $storeid, $volname);
+ }
+
+ return;
+}
+
+sub update_volume_attribute {
+ my ($class, $scfg, $storeid, $volname, $attribute, $value) = @_;
+
+ if ($attribute eq 'notes') {
+ return $class->update_volume_notes($scfg, $storeid, $volname, $value);
+ }
+
+ die "attribute '$attribute' is not supported for storage type '$scfg->{type}'\n";
+}
+
sub volume_size_info {
my ($class, $scfg, $storeid, $volname, $timeout) = @_;
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 417d1fd..1182008 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -851,18 +851,52 @@ sub file_size_info {
return wantarray ? ($size, $format, $used, $parent, $st->ctime) : $size;
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use get_volume_attribute instead.
sub get_volume_notes {
my ($class, $scfg, $storeid, $volname, $timeout) = @_;
die "volume notes are not supported for $class";
}
+# FIXME remove on the next APIAGE reset.
+# Deprecated, use update_volume_attribute instead.
sub update_volume_notes {
my ($class, $scfg, $storeid, $volname, $notes, $timeout) = @_;
die "volume notes are not supported for $class";
}
+# Returns undef if the attribute is not supported for the volume.
+# Should die if there is an error fetching the attribute.
+# Possible attributes:
+# notes - user-provided comments/notes.
+sub get_volume_attribute {
+ my ($class, $scfg, $storeid, $volname, $attribute) = @_;
+
+ if ($attribute eq 'notes') {
+ my $notes = eval { $class->get_volume_notes($scfg, $storeid, $volname); };
+ if (my $err = $@) {
+ return if $err =~ m/^volume notes are not supported/;
+ die $err;
+ }
+ return $notes;
+ }
+
+ return;
+}
+
+# Dies if the attribute is not supported for the volume.
+sub update_volume_attribute {
+ my ($class, $scfg, $storeid, $volname, $attribute, $value) = @_;
+
+ if ($attribute eq 'notes') {
+ $class->update_volume_notes($scfg, $storeid, $volname, $value);
+ }
+
+ die "attribute '$attribute' is not supported for storage type '$scfg->{type}'\n";
+}
+
sub volume_size_info {
my ($class, $scfg, $storeid, $volname, $timeout) = @_;
my $path = $class->filesystem_path($scfg, $volname);
@@ -1098,6 +1132,8 @@ my $get_subdir_files = sub {
return $res;
};
+# If attributes are set on a volume, they should be included in the result.
+# See get_volume_attribute for a list of possible attributes.
sub list_volumes {
my ($class, $storeid, $scfg, $vmid, $content_types) = @_;
--
2.30.2