From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Roland Kammerer <roland.kammerer@linbit.com>,
pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH storage 1/1] storage/plugins: pass scfg to parse_volname
Date: Tue, 5 Mar 2024 12:13:05 +0100 [thread overview]
Message-ID: <4fd06374-5985-4036-aed0-edf3a4c1ac40@proxmox.com> (raw)
In-Reply-To: <20240223092436.202277-2-roland.kammerer@linbit.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.
next prev parent reply other threads:[~2024-03-05 11:13 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240223092436.202277-1-roland.kammerer@linbit.com>
[not found] ` <20240223092436.202277-2-roland.kammerer@linbit.com>
2024-02-29 13:29 ` Fiona Ebner
[not found] ` <mailman.311.1709219402.434.pve-devel@lists.proxmox.com>
2024-03-01 9:45 ` Dietmar Maurer
2024-03-05 12:13 ` Fabian Grünbichler
2024-03-05 11:13 ` Thomas Lamprecht [this message]
2024-03-13 15:38 ` Wolfgang Bumiller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4fd06374-5985-4036-aed0-edf3a4c1ac40@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=roland.kammerer@linbit.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox