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

[Max Carrara]

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.carr...@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