[pve-devel] [RFC v1 pve-storage 6/6] introduce check for duplicate plugin properties

[Max Carrara <m.carrara@proxmox.com>]

For each property that a plugin defines, check if the property is
already defined by another plugin before registering plugins.

This is mostly done for the sake of developer-friendliness. The same
check is made in `PVE::SectionConfig::init()` -- the difference here
is that the plugin which defined the property is preserved in the
error message.

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Storage.pm | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 4cf591f..55a8668 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -313,6 +313,7 @@ my sub do_pre_register_check_plugin : prototype(%) {
 
     my $plugin_name = $params{name};
     my $plugin_types = $params{plugin_types};
+    my $plugin_properties = $params{plugin_properties};
 
     my $plugin_index = $plugin_register_state->{index};
 
@@ -366,6 +367,25 @@ my sub do_pre_register_check_plugin : prototype(%) {
 	$plugin_types->{$type} = $plugin_name;
     };
 
+    my $check_plugin_properties = sub {
+	# so that we may call methods on $plugin_name
+	no strict 'refs'; ## no critic
+
+	# method is inherited and doesn't throw
+	my $properties = $plugin_name->properties();
+
+	my @errs = ();
+	for my $property (keys $properties->%*) {
+	    my $source_plugin = $plugin_properties->{$property};
+	    push(@errs, "property \"$property\" already exists - defined by $source_plugin")
+		if defined($source_plugin);
+
+	    $plugin_properties->{$property} = $plugin_name;
+	}
+
+	die(join("\n", @errs), "\n") if scalar(@errs);
+    };
+
     my $check_symbols = sub {
 	my $prohibited_symbols = PVE::Storage::Plugin::get_prohibited_symbols();
 
@@ -386,6 +406,7 @@ my sub do_pre_register_check_plugin : prototype(%) {
 	$check_derives_base->();
 	$check_plugin_api->() if $is_custom;
 	$check_plugin_type->();
+	$check_plugin_properties->();
 	$check_symbols->();
     };
 
@@ -414,10 +435,17 @@ my sub pre_register_check_plugins : prototype() {
     # { type() => plugin_name }
     my $plugin_types = {};
 
+    # { property_name => plugin_name }
+    my $plugin_properties = {};
+
     my @standard_plugins = get_indexed_plugins(is_custom => 0);
     for my $plugin (@standard_plugins) {
 	eval {
-	    do_pre_register_check_plugin(name => $plugin, plugin_types => $plugin_types);
+	    do_pre_register_check_plugin(
+		name => $plugin,
+		plugin_types => $plugin_types,
+		plugin_properties => $plugin_properties,
+	    );
 	};
 
 	push(@errs, $@) if $@;
@@ -433,7 +461,11 @@ my sub pre_register_check_plugins : prototype() {
     my @custom_plugins = get_indexed_plugins(is_custom => 1);
     for my $plugin (@custom_plugins) {
 	eval {
-	    do_pre_register_check_plugin(name => $plugin, plugin_types => $plugin_types);
+	    do_pre_register_check_plugin(
+		name => $plugin,
+		plugin_types => $plugin_types,
+		plugin_properties => $plugin_properties,
+	    );
 	};
 
 	if ($@) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel