Re: [PATCH pve-storage v2 03/17] api: plugins/storage: add initial routes and endpoints

[Jakob Klocker <j.klocker@proxmox.com>]

comments inline

On 7/17/26 5:50 PM, Max R. Carrara wrote:
> Add the following routes / endpoints:
> 
> * `GET plugins/storage`
>   Subdir index--lists the direct child path components (so, just
>   'plugin' currently).
> 
> * `GET plugins/storage/plugin`
>   Lists all installed storage plugins and their metadata.
>   Currently, this is just an array of objects, with each object
>   containing the plugin's `type()` and Perl module path.
> 
> * `GET plugins/storage/plugin/{type}`
>   Returns the metadata for a single plugin given by `{type}`.
> 
> Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
> ---
>  src/PVE/API2/Makefile                  |   1 +
>  src/PVE/API2/Plugins/Makefile          |  18 +++++
>  src/PVE/API2/Plugins/Storage.pm        |  54 +++++++++++++
>  src/PVE/API2/Plugins/Storage/Makefile  |  17 ++++
>  src/PVE/API2/Plugins/Storage/Plugin.pm | 104 +++++++++++++++++++++++++
>  5 files changed, 194 insertions(+)
>  create mode 100644 src/PVE/API2/Plugins/Makefile
>  create mode 100644 src/PVE/API2/Plugins/Storage.pm
>  create mode 100644 src/PVE/API2/Plugins/Storage/Makefile
>  create mode 100644 src/PVE/API2/Plugins/Storage/Plugin.pm
> 
> diff --git a/src/PVE/API2/Makefile b/src/PVE/API2/Makefile
> index fe316c52..01b7b28f 100644
> --- a/src/PVE/API2/Makefile
> +++ b/src/PVE/API2/Makefile
> @@ -5,3 +5,4 @@ install:
>  	install -D -m 0644 Disks.pm ${DESTDIR}${PERLDIR}/PVE/API2/Disks.pm
>  	make -C Storage install
>  	make -C Disks install
> +	make -C Plugins install
> diff --git a/src/PVE/API2/Plugins/Makefile b/src/PVE/API2/Plugins/Makefile
> new file mode 100644
> index 00000000..f59e75b6
> --- /dev/null
> +++ b/src/PVE/API2/Plugins/Makefile
> @@ -0,0 +1,18 @@
> +SOURCES = Storage.pm		\
> +
> +
> +SUBDIRS = Storage		\
> +
> +
> +INSTALL_PATH = ${DESTDIR}${PERLDIR}/PVE/API2/Plugins
> +
> +
> +.PHONY: install
> +install:
> +	set -e && for SOURCE in ${SOURCES}; \
> +		do install -D -m 0644 $$SOURCE ${INSTALL_PATH}/$$SOURCE; \
> +	done
> +	set -e && for SUBDIR in ${SUBDIRS}; \
> +		do make -C $$SUBDIR install; \
> +	done
> +
> diff --git a/src/PVE/API2/Plugins/Storage.pm b/src/PVE/API2/Plugins/Storage.pm
> new file mode 100644
> index 00000000..53b883fd
> --- /dev/null
> +++ b/src/PVE/API2/Plugins/Storage.pm
> @@ -0,0 +1,54 @@
> +package PVE::API2::Plugins::Storage;
> +
> +use v5.36;
> +
> +use PVE::Storage;
> +use PVE::Storage::Plugin;
> +
> +use PVE::API2::Plugins::Storage::Plugin;
> +
> +use PVE::RESTHandler;
> +use base qw(PVE::RESTHandler);

style-nit: the modules should be split into 4 groups and ordered
alphabetically per the perl style guide [0]. `PVE::RESTHandler;` is a
common module, so it belongs in the same group as `PVE::Storage*`.

I think it should look as following:

use PVE::RESTHandler;
use PVE::Storage;
use PVE::Storage::Plugin;

use PVE::API2::Plugins::Storage::Plugin;

use base qw(PVE::RESTHandler);

[0] https://pve.proxmox.com/wiki/Perl_Style_Guide#Module_Dependencies

> +
> +__PACKAGE__->register_method({
> +    subclass => 'PVE::API2::Plugins::Storage::Plugin',
> +    path => 'plugin',
> +});
> +
> +# plugins/storage
> +
> +__PACKAGE__->register_method({
> +    name => 'index',
> +    path => '',
> +    method => 'GET',
> +    description => 'Directory index.',
> +    permissions => {
> +        user => 'all',
> +    },
> +    parameters => {
> +        additionalProperties => 0,
> +        properties => {},
> +    },
> +    returns => {
> +        type => 'array',
> +        items => {
> +            type => "object",
> +            additionalProperties => 0,
> +            properties => {
> +                subdir => {
> +                    type => 'string',
> +                },
> +            },
> +        },
> +        links => [{ rel => 'child', href => "{subdir}" }],
> +    },
> +    code => sub($param) {
> +        my $result = [
> +            { subdir => 'plugin' },
> +        ];
> +
> +        return $result;
> +    },
> +});
> +
> +1;
> diff --git a/src/PVE/API2/Plugins/Storage/Makefile b/src/PVE/API2/Plugins/Storage/Makefile
> new file mode 100644
> index 00000000..06473a2f
> --- /dev/null
> +++ b/src/PVE/API2/Plugins/Storage/Makefile
> @@ -0,0 +1,17 @@
> +SOURCES = Plugin.pm		\
> +
> +
> +SUBDIRS =
> +
> +
> +INSTALL_PATH = ${DESTDIR}${PERLDIR}/PVE/API2/Plugins/Storage
> +
> +.PHONY: install
> +install:
> +	set -e && for SOURCE in ${SOURCES}; \
> +		do install -D -m 0644 $$SOURCE ${INSTALL_PATH}/$$SOURCE; \
> +	done
> +	set -e && for SUBDIR in ${SUBDIRS}; \
> +		do make -C $$SUBDIR install; \
> +	done
> +
> diff --git a/src/PVE/API2/Plugins/Storage/Plugin.pm b/src/PVE/API2/Plugins/Storage/Plugin.pm
> new file mode 100644
> index 00000000..fd0f734b
> --- /dev/null
> +++ b/src/PVE/API2/Plugins/Storage/Plugin.pm
> @@ -0,0 +1,104 @@
> +package PVE::API2::Plugins::Storage::Plugin;
> +
> +use v5.36;
> +
> +use HTTP::Status qw(:constants);
> +
> +use PVE::Exception qw(raise);
> +use PVE::Storage;
> +use PVE::Storage::Plugin;
> +use PVE::Tools qw(extract_param);
> +
> +use PVE::RESTHandler;
> +use base qw(PVE::RESTHandler);
> +

style-nit: same as above

> +my $ALL_PLUGIN_TYPES = PVE::Storage::Plugin->lookup_types();
> +[snip]