[pve-devel] [PATCH v3] Add support for custom storage plugins

[Wolfgang Bumiller]

Looks good. Two hopefully last changes:
1) Since git takes the commit message from the mail's subject line &
text, it's better to put the version changes after the '---' lines so
that the commit message isn't filled with the patch's change history.

2) See the inline comment below.

On Thu, Aug 25, 2016 at 03:44:06PM +0300, Dmitry Petuhov wrote:
> New in v3:
> - Moved constant APIVER definion after other use statements.
> - Added check for api() method and PVE::Storage::Plugin inheritabce.
> - Added warning message on api version mismatch.
> - Moved custom plugins loading after standard plugins.
> 
> New in v2: add API version check.
> 
> Custom plugins MUST have api() method returning version for which
> it was designed. If API changes from PVE side, module is just not
> being registered, so user have to update module. Until module update,
> corresponding storage will just disappear from PVE, so it shall
> not impose any data damage because of API change.
> 
> Original message:
> 
> PVE team cannot support specialized vendor-specific storage
> plugins because of lack of hardware. But we can allow users to
> add own plugins for their storages without need to rewrite any
> PVE code and thus ease PVE updates to them.
> 
> Idea of this patch is to add folder /usr/share/perl5/PVE/Storage/Custom
> where user can place his plugins and PVE will automatically load
> them on start or warn if it could not and continue. Maybe we could
> even load all plugins (except PVE::Storage::Plugin itself) this way,
> because current storage plugins are not really plugins, if they
> need to be explicitly loaded in PVE code :-).
> 
> This approach works (with some limitations) if plugin supports
> generic PVE way: full control of volumes lifecycle. And will not
> currently work for custom plugins like iSCSI, which needs to select
> pre-existing volumes. Maybe someone will add more flexible way to
> pve-manager to select input elements for storage plugins to target
> this.
> 
> Currently tested with my NetApp plugin.
> 
> Signed-off-by: Dmitry Petuhov <mityapetuhov at gmail.com>
> ---
>  PVE/Storage.pm | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 25ff545..f90cf7b 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -12,7 +12,7 @@ use File::Path;
>  use Cwd 'abs_path';
>  use Socket;
>  
> -use PVE::Tools qw(run_command file_read_firstline $IPV6RE);
> +use PVE::Tools qw(run_command file_read_firstline dir_glob_foreach $IPV6RE);
>  use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
>  use PVE::Exception qw(raise_param_exc);
>  use PVE::JSONSchema;
> @@ -33,7 +33,10 @@ use PVE::Storage::ZFSPoolPlugin;
>  use PVE::Storage::ZFSPlugin;
>  use PVE::Storage::DRBDPlugin;
>  
> -# load and initialize all plugins
> +# Storage API version. Icrement it on changes in storage API interface.
> +use constant APIVER => 1;
> +
> +# load standard plugins
>  PVE::Storage::DirPlugin->register();
>  PVE::Storage::LVMPlugin->register();
>  PVE::Storage::LvmThinPlugin->register();
> @@ -46,6 +49,31 @@ PVE::Storage::GlusterfsPlugin->register();
>  PVE::Storage::ZFSPoolPlugin->register();
>  PVE::Storage::ZFSPlugin->register();
>  PVE::Storage::DRBDPlugin->register();
> +
> +# load custom plugins
> +if ( -d '/usr/share/perl5/PVE/Storage/Custom' ) {
> +    dir_glob_foreach('/usr/share/perl5/PVE/Storage/Custom', '.*\.pm$', sub {
> +	my ($file) = @_;
> +	my $modname = 'PVE::Storage::Custom::' . $file;
> +	$modname =~ s!\.pm$!!;
> +	$file = 'PVE/Storage/Custom/' . $file;
> +
> +	eval {
> +	    require $file;
> +	};
> +	if ($@) {
> +	    warn $@;
> +	# Check storage API version and that file is really storage plugin.
> +	} elsif ($modname->isa('PVE::Storage::Plugin') && $modname->can('api') && $modname->api() == APIVER) {
> +	    import $file;
> +	    $modname->register();

Since register() on already-existing types also causes a startup failure
this also needs to be eval{}ed, simply:
  eval { $modname->register() };
  warn $@ if $@;

> +	} else {
> +	    warn "Error loading storage plugin \"$modname\" because of API version mismatch. Please, update it.\n"
> +	}
> +    });
> +}
> +
> +# initialize all plugins
>  PVE::Storage::Plugin->init();
>  
>  my $UDEVADM = '/sbin/udevadm';
> -- 
> 2.1.4