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

[Alexandre DERUMIER]

Hi,

I thing that should be improved, is in Storage/Plugin.pm

        if ($type eq 'iscsi' || $type eq 'nfs' || $type eq 'rbd' || $type eq 'sheepdog' || $type eq 'iscsidirect' || $type eq 'glusterfs' || $type eq 'zfs' || $type eq 'drbd') {
            $d->{shared} = 1;
        }

Could be great to define it in plugins directly.


----- Mail original -----
De: "Wolfgang Bumiller" <w.bumiller at proxmox.com>
À: "Dmitry Petuhov" <mityapetuhov at gmail.com>
Cc: "pve-devel" <pve-devel at pve.proxmox.com>
Envoyé: Vendredi 26 Août 2016 11:14:16
Objet: Re: [pve-devel] [PATCH v3] Add support for custom storage plugins

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 

_______________________________________________ 
pve-devel mailing list 
pve-devel at pve.proxmox.com 
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel