[pve-devel] applied: [PATCH storage] plugin loader: add an APIAGE

[Thomas Lamprecht]

On 11/16/18 1:54 PM, Wolfgang Bumiller wrote:
> With the addition of the map/unmap_volume() methods we made
> an (actually unnecessary) API version bump.
> All current users of these methods fall back to path() when
> they return undef, so plugins implementing version 1 are
> in fact compatible currently. (In fact, the default
> Plugin::map_volume() could fall back to it on its own, but
> doesn't currently).
> 
> For now let's just allow plugins older plugins to also be
> loaded by introducing an API age variable. With it, if we
> have a reason to break older plugins, we can have a
> deprecation period during which older plugins cause a
> warning instead of refusing to load altogether.


applied, thanks!

> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> ---
>  PVE/Storage.pm | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index a5f7fdb..64b3fc9 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -38,6 +38,10 @@ use PVE::Storage::DRBDPlugin;
>  
>  # Storage API version. Icrement it on changes in storage API interface.
>  use constant APIVER => 2;
> +# Age is the number of versions we're backward compatible with.
> +# This is like having 'current=APIVER' and age='APIAGE' in libtool,
> +# see https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
> +use constant APIAGE => 1;
>  
>  # load standard plugins
>  PVE::Storage::DirPlugin->register();
> @@ -65,18 +69,28 @@ if ( -d '/usr/share/perl5/PVE/Storage/Custom' ) {
>  
>  	eval {
>  	    require $file;
> +
> +	    # Check perl interface:
> +	    die "not derived from PVE::Storage::Plugin\n"
> +		if !$modname->isa('PVE::Storage::Plugin');
> +	    die "does not provide an api() method\n"
> +		if !$modname->can('api');
> +	    # Check storage API version and that file is really storage plugin.
> +	    my $version = $modname->api();
> +	    die "implements an API version newer than current\n"
> +		if $version > APIVER;
> +	    die "API version too old, pluse update the plugin\n"
> +		if $version < (APIVER-APIAGE);
> +	    import $file;
> +	    $modname->register();
> +
> +	    # If we got this far and the API version is not the same, make some
> +	    # noise:
> +	    warn "Plugin \"$modname\" is implementing an older storage API, an upgrade is recommended\n"
> +		if $version != APIVER;
>  	};
>  	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) {
> -            eval {
> -	        import $file;
> -	        $modname->register();
> -            };
> -            warn $@ if $@;
> -	} else {
> -	    warn "Error loading storage plugin \"$modname\" because of API version mismatch. Please, update it.\n"
> +	    warn "Error loading storage plugin \"$modname\": $@";
>  	}
>      });
>  }
>