Re: [pve-devel] [PATCH storage 1/1] storage/plugins: pass scfg to parse_volname

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 23/02/2024 um 10:24 schrieb Roland Kammerer:
> This passes the well known $scfg to parse_volname and bumps the API
> versions accordingly. This allows plugins to access their configuration
> if necessary.

We discussed this another time here and effectively it can be fine, while
the need for it seems like a slight smell from our architecture POV, as
it basically always means that the VMID -> volume mapping is not encoded
in the name any more (worse UX), it still does not hurt our, or other
external, existing plug-ins.

So fine to add, but please also the parameter also to the base
"parse_volname" method including a comment that mentions that this
is in general not used and only required if the storage cannot
provide all required information in the volume name.
 > Signed-off-by: Roland Kammerer <roland.kammerer@linbit.com>
> ---
>  ApiChangeLog       |  7 +++++++
>  src/PVE/Storage.pm | 18 +++++++++---------
>  2 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/ApiChangeLog b/ApiChangeLog
> index 98b5893..042e13b 100644
> --- a/ApiChangeLog
> +++ b/ApiChangeLog
> @@ -6,6 +6,13 @@ without breaking anything unaware of it.)
>  
>  Future changes should be documented in here.
>  
> +##  Version 11:
> +
> +* `parse_volname` got additional `$scfg` parameter
> +
> +  This allows plugins to use information from their configuration.
> +  As this is the last, additional parameter APIAGE is not reset.
> +
>  ##  Version 10:
>  
>  * a new `rename_volume` method has been added
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index 0beabbc..3ac4f1e 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -41,11 +41,11 @@ use PVE::Storage::PBSPlugin;
>  use PVE::Storage::BTRFSPlugin;
>  
>  # Storage API version. Increment it on changes in storage API interface.
> -use constant APIVER => 10;
> +use constant APIVER => 11;
>  # 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;
> +use constant APIAGE => 2;
>  

unrelated, but wondering if we could rework the approach how we warn about
to old plugins somewhat, as currently this is IMO rather to noisy, especially
for a change like this one.