[PATCH pve-storage v1 06/54] plugin api: replace helpers w/ standalone subs, bump API version & age

Max R. Carrara
2026-04-22T11:14:50Z

Replace the `get_vtype_subdirs()` and `get_subdir()` helpers in
PVE::Storage::Plugin with standalone helper subroutines
`plugin_get_default_vtype_subdirs()` and `plugin_get_vtype_subdir()`
in PVE::Storage::Common.

This is mostly to prevent third-party plugins from relying on these
helpers as if they were part of the storage plugin API; the signature
of `get_subdir()` in particular looks as if it belongs to an API
method, even though it is only used as a helper in PVE::Storage,
PVE::Storage::Plugin, and PVE::Storage::BTRFSPlugin.

Therefore, make it explicit that these subroutines are really just
helpers by replacing them with slightly altered versions in
PVE::Storage::Common. Add docstrings for these replacements as well.

Additionally, note that PVE::Storage::ESXiPlugin is the only plugin
that explicitly overrides `get_subdir()`, treating it as if it were an
API method, but because that plugin is not filesystem-based (in the
sense that it does not declare the `path` property in its options),
calling `get_subdir()` would throw an exception anyhow. Also, it
should be mentioned that all methods that could call into
`get_subdir()` have been overridden inside ESXiPlugin anyway.

Therefore, the override of `get_subdir()` in PVE::Storage::ESXiPlugin
is superfluous, so remove it.

Additionally, replace all occurrences of `get_vtype_subdirs()` and
`get_subdir()` across the repository with their newly introduced
counterparts `plugin_get_default_vtype_subdirs()` and
`plugin_get_vtype_subdir()`. Adapt any comments and test names as well
in accordance with these changes.

Finally, increment APIAGE and APIVER in PVE::Storage and add a
corresponding entry to the API changelog.

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
ApiChangeLog | 22 ++++++++++++
src/PVE/Storage.pm | 31 +++++++++--------
src/PVE/Storage/BTRFSPlugin.pm | 15 +++++----
src/PVE/Storage/Common.pm | 52 +++++++++++++++++++++++++++++
src/PVE/Storage/ESXiPlugin.pm | 6 ----
src/PVE/Storage/Plugin.pm | 47 ++++++++++----------------
src/test/get_subdir_test.pm | 12 ++++---
src/test/parse_volname_test.pm | 6 +++-
src/test/path_to_volume_id_test.pm | 6 +++-
src/test/run_volume_access_tests.pl | 5 ++-
10 files changed, 140 insertions(+), 62 deletions(-)

diff --git a/ApiChangeLog b/ApiChangeLog
index de411927..d1d3e1c3 100644
--- a/ApiChangeLog
+++ b/ApiChangeLog
@@ -6,6 +6,28 @@ without breaking anything unaware of it.)

Future changes should be documented in here.

+## Version 14:
+
+* Replace plugin helper `get_vtype_subdirs()` with standalone helper subroutine
+ `PVE::Storage::Common::plugin_get_default_vtype_subdirs()`
+
+ The `get_vtype_subdirs()` subroutine in `PVE::Storage::Plugin` was an
+ internal helper with a public signature. In order to make it clear that this
+ is a helper and not meant to be part of the storage plugin API, it is
+ replaced with the more explicit
+ `PVE::Storage::Common::plugin_get_default_vtype_subdirs()` subroutine.
+
+* Replace plugin method `get_subdir()` with standalone helper subroutine
+ `PVE::Storage::Common::plugin_get_vtype_subdir()`
+
+ The `get_subdir()` method in `PVE::Storage::Plugin` was an internal helper
+ method with a public signature. Moreover, its signature could mistakenly
+ suggest that this method was part of the storage plugin API, which it was not
+ originally meant to be.
+
+ In order to make it clear that this is just a helper, it is replaced with the
+ more explicit `PVE::Storage::Common::plugin_get_vtype_subdir()` subroutine.
+
## Version 13:

* Add new parameter $hints to the `activate_volume()` and `map_volume()` plugin methods
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 6e87bac3..ef2e7887 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -22,6 +22,9 @@ use PVE::JSONSchema;
use PVE::INotify;
use PVE::RPCEnvironment;
use PVE::SSHInfo;
+use PVE::Storage::Common qw(
+ plugin_get_vtype_subdir
+);
use PVE::RESTEnvironment qw(log_warn);

use PVE::Storage::Plugin;
@@ -41,11 +44,11 @@ use PVE::Storage::BTRFSPlugin;
use PVE::Storage::ESXiPlugin;

# Storage API version. Increment it on changes in storage API interface.
-use constant APIVER => 13;
+use constant APIVER => 14;
# 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 => 4;
+use constant APIAGE => 5;

our $KNOWN_EXPORT_FORMATS = ['raw+size', 'tar+size', 'qcow2+size', 'vmdk+size', 'zfs', 'btrfs'];

@@ -530,7 +533,7 @@ sub get_image_dir {
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});

- my $path = $plugin->get_subdir($scfg, 'images');
+ my $path = plugin_get_vtype_subdir($scfg, 'images');

return $vmid ? "$path/$vmid" : $path;
}
@@ -541,7 +544,7 @@ sub get_private_dir {
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});

- my $path = $plugin->get_subdir($scfg, 'rootdir');
+ my $path = plugin_get_vtype_subdir($scfg, 'rootdir');

return $vmid ? "$path/$vmid" : $path;
}
@@ -552,7 +555,7 @@ sub get_iso_dir {
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});

- return $plugin->get_subdir($scfg, 'iso');
+ return plugin_get_vtype_subdir($scfg, 'iso');
}

sub get_import_dir {
@@ -561,7 +564,7 @@ sub get_import_dir {
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});

- return $plugin->get_subdir($scfg, 'import');
+ return plugin_get_vtype_subdir($scfg, 'import');
}

sub get_vztmpl_dir {
@@ -570,7 +573,7 @@ sub get_vztmpl_dir {
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});

- return $plugin->get_subdir($scfg, 'vztmpl');
+ return plugin_get_vtype_subdir($scfg, 'vztmpl');
}

sub get_backup_dir {
@@ -579,7 +582,7 @@ sub get_backup_dir {
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});

- return $plugin->get_subdir($scfg, 'backup');
+ return plugin_get_vtype_subdir($scfg, 'backup');
}

# library implementation
@@ -713,12 +716,12 @@ sub path_to_volume_id {
my $scfg = $ids->{$sid};
next if !$scfg->{path};
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
- my $imagedir = $plugin->get_subdir($scfg, 'images');
- my $isodir = $plugin->get_subdir($scfg, 'iso');
- my $tmpldir = $plugin->get_subdir($scfg, 'vztmpl');
- my $backupdir = $plugin->get_subdir($scfg, 'backup');
- my $snippetsdir = $plugin->get_subdir($scfg, 'snippets');
- my $importdir = $plugin->get_subdir($scfg, 'import');
+ my $imagedir = plugin_get_vtype_subdir($scfg, 'images');
+ my $isodir = plugin_get_vtype_subdir($scfg, 'iso');
+ my $tmpldir = plugin_get_vtype_subdir($scfg, 'vztmpl');
+ my $backupdir = plugin_get_vtype_subdir($scfg, 'backup');
+ my $snippetsdir = plugin_get_vtype_subdir($scfg, 'snippets');
+ my $importdir = plugin_get_vtype_subdir($scfg, 'import');

if ($path =~ m!^\Q$imagedir\E/(\d+)/([^/\s]+)$!) {
my $vmid = $1;
diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
index e68d2bf9..67b442f6 100644
--- a/src/PVE/Storage/BTRFSPlugin.pm
+++ b/src/PVE/Storage/BTRFSPlugin.pm
@@ -11,6 +11,9 @@ use File::Path qw(mkpath);
use IO::Dir;
use POSIX qw(EEXIST);

+use PVE::Storage::Common qw(
+ plugin_get_vtype_subdir
+);
use PVE::Tools qw(run_command dir_glob_foreach);

use PVE::Storage::DirPlugin;
@@ -188,7 +191,7 @@ sub filesystem_path {

my ($vtype, $name, $vmid, undef, undef, $isBase, $format) = $class->parse_volname($volname);

- my $path = $class->get_subdir($scfg, $vtype);
+ my $path = plugin_get_vtype_subdir($scfg, $vtype);

$path .= "/$vmid" if $vtype eq 'images';

@@ -294,7 +297,7 @@ sub clone_image {
return PVE::Storage::DirPlugin::clone_image(@_);
}

- my $imagedir = $class->get_subdir($scfg, 'images');
+ my $imagedir = plugin_get_vtype_subdir($scfg, 'images');
$imagedir .= "/$vmid";
mkpath $imagedir;

@@ -327,7 +330,7 @@ sub alloc_image {

# From Plugin.pm:

- my $imagedir = $class->get_subdir($scfg, 'images') . "/$vmid";
+ my $imagedir = plugin_get_vtype_subdir($scfg, 'images') . "/$vmid";

mkpath $imagedir;

@@ -643,7 +646,7 @@ sub volume_has_feature {

sub list_images {
my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
- my $imagedir = $class->get_subdir($scfg, 'images');
+ my $imagedir = plugin_get_vtype_subdir($scfg, 'images');

my $res = [];

@@ -844,7 +847,7 @@ sub volume_import {
$volname = $class->find_free_diskname($storeid, $scfg, $vmid, $volume_format, 1);
}

- my $imagedir = $class->get_subdir($scfg, $vtype);
+ my $imagedir = plugin_get_vtype_subdir($scfg, $vtype);
$imagedir .= "/$vmid" if $vtype eq 'images';

my $tmppath = "$imagedir/recv.$vmid.tmp";
@@ -975,7 +978,7 @@ sub rename_volume {
if !$target_volname;
$target_volname = "$target_vmid/$target_volname";

- my $basedir = $class->get_subdir($scfg, 'images');
+ my $basedir = plugin_get_vtype_subdir($scfg, 'images');

mkpath "${basedir}/${target_vmid}";
my $source_dir = raw_name_to_dir($source_volname);
diff --git a/src/PVE/Storage/Common.pm b/src/PVE/Storage/Common.pm
index 76784e9e..52bd627d 100644
--- a/src/PVE/Storage/Common.pm
+++ b/src/PVE/Storage/Common.pm
@@ -17,6 +17,9 @@ our @EXPORT_OK = qw(
qemu_img_info
qemu_img_measure
qemu_img_resize
+
+ plugin_get_default_vtype_subdirs
+ plugin_get_vtype_subdir
);

use constant {
@@ -24,6 +27,16 @@ use constant {
FALLOC_FL_PUNCH_HOLE => 0x02, # see linux/falloc.h
};

+my $DEFAULT_VTYPE_SUBDIRS = {
+ images => 'images',
+ rootdir => 'private',
+ iso => 'template/iso',
+ vztmpl => 'template/cache',
+ backup => 'dump',
+ snippets => 'snippets',
+ import => 'import',
+};
+
=head1 NAME

PVE::Storage::Common - Shared functions and utilities for storage plugins and storage operations
@@ -283,4 +296,43 @@ sub qemu_img_resize {
run_command($cmd, timeout => $timeout);
}

+=head2 FUNCTIONS FOR STORAGE PLUGINS AND CONFIGURATIONS
+
+=cut
+
+=head3 plugin_get_default_vtype_subdirs
+
+ my $vtype_subdirs = plugin_get_default_vtype_subdirs()
+
+Returns a hashref containing the default sub-directories for every volume type.
+
+=cut
+
+sub plugin_get_default_vtype_subdirs : prototype() () {
+ return { $DEFAULT_VTYPE_SUBDIRS->%* }; # shallow copy
+}
+
+=head3 plugin_get_vtype_subdir
+
+ my $subdir = plugin_get_vtype_subdir($scfg, $vtype)
+
+Returns the sub-directory for the volume type C<$vtype> of a given storage
+configuration C<$scfg>.
+
+Raises an exception if the storage config has no C<path> attribute or
+if the given C<$vtype> does not exist.
+
+=cut
+
+sub plugin_get_vtype_subdir : prototype($$) ($scfg, $vtype) {
+ my $path = $scfg->{path};
+
+ die "storage definition has no path\n" if !$path;
+ die "unknown vtype '$vtype'\n" if !exists($DEFAULT_VTYPE_SUBDIRS->{$vtype});
+
+ my $subdir = $scfg->{"content-dirs"}->{$vtype} // $DEFAULT_VTYPE_SUBDIRS->{$vtype};
+
+ return "$path/$subdir";
+}
+
1;
diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
index f89e427f..da5abb04 100644
--- a/src/PVE/Storage/ESXiPlugin.pm
+++ b/src/PVE/Storage/ESXiPlugin.pm
@@ -605,12 +605,6 @@ sub volume_has_feature {
return undef;
}

-sub get_subdir {
- my ($class, $scfg, $vtype) = @_;
-
- die "no subdirectories available for storage $class\n";
-}
-
package PVE::Storage::ESXiPlugin::Manifest;

use strict;
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 07233db3..55268b29 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -15,7 +15,10 @@ use PVE::Tools qw(run_command);
use PVE::JSONSchema qw(get_standard_option register_standard_option);
use PVE::Cluster qw(cfs_register_file);

-use PVE::Storage::Common;
+use PVE::Storage::Common qw(
+ plugin_get_default_vtype_subdirs
+ plugin_get_vtype_subdir
+);

use JSON;

@@ -836,31 +839,16 @@ sub parse_volname {
die "unable to parse directory volume name '$volname'\n";
}

-my $vtype_subdirs = {
- images => 'images',
- rootdir => 'private',
- iso => 'template/iso',
- vztmpl => 'template/cache',
- backup => 'dump',
- snippets => 'snippets',
- import => 'import',
-};
-
+# FIXME: remove on the next APIAGE reset.
sub get_vtype_subdirs {
- return $vtype_subdirs;
+ return plugin_get_default_vtype_subdirs();
}

+# FIXME: remove on the next APIAGE reset.
sub get_subdir {
my ($class, $scfg, $vtype) = @_;

- my $path = $scfg->{path};
-
- die "storage definition has no path\n" if !$path;
- die "unknown vtype '$vtype'\n" if !exists($vtype_subdirs->{$vtype});
-
- my $subdir = $scfg->{"content-dirs"}->{$vtype} // $vtype_subdirs->{$vtype};
-
- return "$path/$subdir";
+ return plugin_get_vtype_subdir($scfg, $vtype);
}

my sub get_snap_name {
@@ -885,7 +873,7 @@ sub filesystem_path {
die "can't snapshot this image format\n"
if defined($snapname) && $format !~ m/^(qcow2|qed)$/;

- my $dir = $class->get_subdir($scfg, $vtype);
+ my $dir = plugin_get_vtype_subdir($scfg, $vtype);

$dir .= "/$vmid" if $vtype eq 'images';

@@ -1016,7 +1004,7 @@ sub clone_image {

die "clone_image only works on base images\n" if !$isBase;

- my $imagedir = $class->get_subdir($scfg, 'images');
+ my $imagedir = plugin_get_vtype_subdir($scfg, 'images');
$imagedir .= "/$vmid";

mkpath $imagedir;
@@ -1049,7 +1037,7 @@ sub clone_image {
sub alloc_image {
my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;

- my $imagedir = $class->get_subdir($scfg, 'images');
+ my $imagedir = plugin_get_vtype_subdir($scfg, 'images');
$imagedir .= "/$vmid";

mkpath $imagedir;
@@ -1608,7 +1596,7 @@ sub volume_has_feature {
sub list_images {
my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;

- my $imagedir = $class->get_subdir($scfg, 'images');
+ my $imagedir = plugin_get_vtype_subdir($scfg, 'images');

my $format_info = $class->get_formats($scfg, $storeid);
my $fmts = join('|', sort keys $format_info->{valid}->%*);
@@ -1756,7 +1744,7 @@ sub list_volumes {
if ($type eq 'images' || $type eq 'rootdir') {
$data = $class->list_images($storeid, $scfg, $vmid);
} elsif ($scfg->{path}) {
- my $path = $class->get_subdir($scfg, $type);
+ my $path = plugin_get_vtype_subdir($scfg, $type);

if ($type eq 'iso' && !defined($vmid)) {
$data = get_subdir_files($storeid, $path, 'iso');
@@ -1909,13 +1897,14 @@ sub activate_storage {
# FIXME The mkdir option is deprecated. Remove with PVE 9?
&& (!defined($scfg->{mkdir}) || $scfg->{mkdir})
) {
- for my $vtype (sort keys %$vtype_subdirs) {
+ my $vtype_subdirs = plugin_get_default_vtype_subdirs();
+ for my $vtype (sort keys $vtype_subdirs->%*) {
# OpenVZMigrate uses backup (dump) dir
if (
defined($scfg->{content}->{$vtype})
|| ($vtype eq 'backup' && defined($scfg->{content}->{'rootdir'}))
) {
- my $subdir = $class->get_subdir($scfg, $vtype);
+ my $subdir = plugin_get_vtype_subdir($scfg, $vtype);
mkpath $subdir if $subdir ne $path;
}
}
@@ -1924,7 +1913,7 @@ sub activate_storage {
# check that content dirs are pairwise inequal
my $resolved_subdirs = {};
for my $vtype (sort keys $scfg->{content}->%*) {
- my $subdir = $class->get_subdir($scfg, $vtype);
+ my $subdir = plugin_get_vtype_subdir($scfg, $vtype);
my $abs_subdir = abs_path($subdir);
next if !defined($abs_subdir);

@@ -2294,7 +2283,7 @@ sub rename_volume {
$target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format, 1)
if !$target_volname;

- my $basedir = $class->get_subdir($scfg, 'images');
+ my $basedir = plugin_get_vtype_subdir($scfg, 'images');

mkpath "${basedir}/${target_vmid}";

diff --git a/src/test/get_subdir_test.pm b/src/test/get_subdir_test.pm
index 5fb54458..843c8c39 100644
--- a/src/test/get_subdir_test.pm
+++ b/src/test/get_subdir_test.pm
@@ -5,16 +5,20 @@ use warnings;

use lib qw(..);

+use PVE::Storage::Common qw(
+ plugin_get_default_vtype_subdirs
+ plugin_get_vtype_subdir
+);
use PVE::Storage::Plugin;
use Test::More;

my $scfg_with_path = { path => '/some/path' };
-my $vtype_subdirs = PVE::Storage::Plugin::get_vtype_subdirs();
+my $vtype_subdirs = plugin_get_default_vtype_subdirs();

# each test is comprised of the following array keys:
# [0] => storage config; positive with path key
# [1] => storage type; see $vtype_subdirs
-# [2] => expected return from get_subdir
+# [2] => expected return from plugin_get_vtype_subdir
my $tests = [
# failed matches
[$scfg_with_path, 'none', "unknown vtype 'none'\n"],
@@ -45,10 +49,10 @@ foreach my $tt (@$tests) {
my ($scfg, $type, $expected) = @$tt;

my $got;
- eval { $got = PVE::Storage::Plugin->get_subdir($scfg, $type) };
+ eval { $got = plugin_get_vtype_subdir($scfg, $type) };
$got = $@ if $@;

- is($got, $expected, "get_subdir for $type") || diag(explain($got));
+ is($got, $expected, "plugin_get_vtype_subdir for $type") || diag(explain($got));
}

done_testing();
diff --git a/src/test/parse_volname_test.pm b/src/test/parse_volname_test.pm
index 5c9478ac..2ff4c6d0 100644
--- a/src/test/parse_volname_test.pm
+++ b/src/test/parse_volname_test.pm
@@ -6,6 +6,9 @@ use warnings;
use lib qw(..);

use PVE::Storage;
+use PVE::Storage::Common qw(
+ plugin_get_default_vtype_subdirs
+);
use Test::More;

my $vmid = 1234;
@@ -302,7 +305,8 @@ foreach my $virt (keys %$non_bkp_suffix) {
plan tests => scalar @$tests + 1;

my $seen_vtype;
-my $vtype_subdirs = { map { $_ => 1 } keys %{ PVE::Storage::Plugin::get_vtype_subdirs() } };
+my $vtype_subdirs =
+ { map { $_ => 1 } keys %{ plugin_get_default_vtype_subdirs() } };

foreach my $t (@$tests) {
my $description = $t->{description};
diff --git a/src/test/path_to_volume_id_test.pm b/src/test/path_to_volume_id_test.pm
index dfa51e64..e7e36037 100644
--- a/src/test/path_to_volume_id_test.pm
+++ b/src/test/path_to_volume_id_test.pm
@@ -6,6 +6,9 @@ use warnings;
use lib qw(..);

use PVE::Storage;
+use PVE::Storage::Common qw(
+ plugin_get_default_vtype_subdirs
+);

use Test::More;

@@ -237,7 +240,8 @@ my @tests = (
plan tests => scalar @tests + 1;

my $seen_vtype;
-my $vtype_subdirs = { map { $_ => 1 } keys %{ PVE::Storage::Plugin::get_vtype_subdirs() } };
+my $vtype_subdirs =
+ { map { $_ => 1 } keys %{ plugin_get_default_vtype_subdirs() } };

foreach my $tt (@tests) {
my $file = $tt->{volname};
diff --git a/src/test/run_volume_access_tests.pl b/src/test/run_volume_access_tests.pl
index 34487083..31bacaa9 100755
--- a/src/test/run_volume_access_tests.pl
+++ b/src/test/run_volume_access_tests.pl
@@ -10,6 +10,9 @@ use lib ('.', '..');

use PVE::RPCEnvironment;
use PVE::Storage;
+use PVE::Storage::Common qw(
+ plugin_get_default_vtype_subdirs
+);
use PVE::Storage::Plugin;

my $storage_cfg = <<'EOF';
@@ -64,7 +67,7 @@ $pve_cluster_module->mock(
my $rpcenv = PVE::RPCEnvironment->init('pub');
$rpcenv->init_request();

-my @types = sort keys PVE::Storage::Plugin::get_vtype_subdirs()->%*;
+my @types = sort keys plugin_get_default_vtype_subdirs()->%*;
my $all_types = { map { $_ => 1 } @types };

my @tests = (
--
2.47.3