on_add_hook allows to encapsulate storage specific add steps, like
copying a keyring (RDB) or creating a volume group (LVM) in a clean
manner.
The same for deletion with on_delete_hook, here all should be cleaned
up, as much as possible.

Until now, this was done directly in the api config CREATE and DELETE
code, respectively, with a series of

if ($storage_type eq 'foo) {
    ...
} elsif ($storage_type eq 'bar') {
    ...
}

which isn't really that nice...

Another nice result of this approach is that also external plugins
can use those hooks and to their setup/cleanup steps sanely.

Signed-off-by: Thomas Lamprecht <t.lampre...@proxmox.com>
---
 PVE/API2/Storage/Config.pm |  8 ++++++++
 PVE/Storage/Plugin.pm      | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm
index 95ca9b8..0d56115 100755
--- a/PVE/API2/Storage/Config.pm
+++ b/PVE/API2/Storage/Config.pm
@@ -161,6 +161,8 @@ __PACKAGE__->register_method ({
 
                $cfg->{ids}->{$storeid} = $opts;
 
+               $plugin->on_add_hook($storeid, $opts, password => $password);
+
                my $cred_file = undef;
 
                if ($type eq 'lvm' && $opts->{base}) {
@@ -209,6 +211,8 @@ __PACKAGE__->register_method ({
                    }
                };
                if(my $err = $@) {
+                   eval { $plugin->on_delete_hook($storeid, $opts) };
+                   warn "$@\n" if $@;
                    unlink $cred_file if defined($cred_file);
                    die $err;
                }
@@ -293,6 +297,10 @@ __PACKAGE__->register_method ({
                die "can't remove storage - storage is used as base of another 
storage\n"
                    if PVE::Storage::storage_is_used($cfg, $storeid);
 
+               my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+               $plugin->on_delete_hook($storeid, $scfg);
+
                if ($scfg->{type} eq 'cifs')  {
                    my $cred_file = 
PVE::Storage::CIFSPlugin::cifs_cred_file_name($storeid);
                    if (-f $cred_file) {
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 163871d..88faa47 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -352,6 +352,26 @@ sub parse_config {
 
 # Storage implementation
 
+# called during addition of storage (before the new storage config got written)
+# die to abort additon if there are (grave) problems
+# NOTE: runs in a storage config *locked* context
+sub on_add_hook {
+    my ($class, $storeid, $scfg, %param) = @_;
+
+    # do nothing by default
+}
+
+# called during deletion of storage (before the new storage config got written)
+# and if the activate check on addition fails, to cleanup all storage traces
+# which on_add_hook may have created.
+# die to abort deletion if there are (very grave) problems
+# NOTE: runs in a storage config *locked* context
+sub on_delete_hook {
+    my ($class, $storeid, $scfg) = @_;
+
+    # do nothing by default
+}
+
 sub cluster_lock_storage {
     my ($class, $storeid, $shared, $timeout, $func, @param) = @_;
 
-- 
2.17.1


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

Reply via email to