public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: "Max R. Carrara" <m.carrara@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH pve-storage v1 06/54] plugin api: replace helpers w/ standalone subs, bump API version & age
Date: Tue, 28 Apr 2026 17:01:45 +0200	[thread overview]
Message-ID: <dzlh5oyu73idd2s6e3ovzrivdhhvxdmkiumwjvzq6nbjotidst@2i25ysrie7xt> (raw)
In-Reply-To: <20260422111322.257380-7-m.carrara@proxmox.com>

On Wed, Apr 22, 2026 at 01:12:32PM +0200, Max R. Carrara wrote:
> 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.

Reminder (I menetioned this off-list).

It would be good to also update pve8to9 since it makes use of
$plugin->get_subdir()
> 
> 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
> 
> 
> 
> 
> 

-- 




  reply	other threads:[~2026-04-28 15:02 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-22 11:12 [PATCH pve-storage, pve-manager v1 00/54] Fix #2884: Implement Subdirectory Scanning for Dir-Based Storage Types Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 01/54] test: plugin tests: run tests with at most 4 jobs Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 02/54] plugin, common: remove superfluous use of =pod command paragraph Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 03/54] common: add POD headings for groups of helpers Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 04/54] common: use Exporter module for PVE::Storage::Common Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 05/54] plugin: make get_subdir_files a proper subroutine and update style Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 06/54] plugin api: replace helpers w/ standalone subs, bump API version & age Max R. Carrara
2026-04-28 15:01   ` Wolfgang Bumiller [this message]
2026-04-22 11:12 ` [PATCH pve-storage v1 07/54] common: prevent autovivification in plugin_get_vtype_subdir helper Max R. Carrara
2026-04-28 15:02   ` Wolfgang Bumiller
2026-04-22 11:12 ` [PATCH pve-storage v1 08/54] plugin: break up needless if-elsif chain into separate if-blocks Max R. Carrara
2026-04-28 15:04   ` Wolfgang Bumiller
2026-04-22 11:12 ` [PATCH pve-storage v1 09/54] plugin: adapt get_subdir_files helper of list_volumes API method Max R. Carrara
2026-04-28 15:07   ` Wolfgang Bumiller
2026-04-22 11:12 ` [PATCH pve-storage v1 10/54] plugin: update code style of list_volumes plugin " Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 11/54] plugin: use closure for obtaining raw volume data in list_volumes Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 12/54] plugin: use closure for inner loop logic " Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 13/54] storage: update code style in function path_to_volume_id Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 14/54] storage: break up needless if-elsif chain in path_to_volume_id Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 15/54] storage: heave vtype file path parsing logic inside loop into helper Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 16/54] storage: clean up code that was moved into helper in path_to_volume_id Max R. Carrara
2026-04-28 15:08   ` Wolfgang Bumiller
2026-04-22 11:12 ` [PATCH pve-storage v1 17/54] api: status: move content type assert for up-/downloads into helper Max R. Carrara
2026-04-28 15:10   ` Wolfgang Bumiller
2026-04-22 11:12 ` [PATCH pve-storage v1 18/54] api: status: use helper from common module to get content directory Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 19/54] api: status: move up-/download file path parsing code into helper Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 20/54] api: status: simplify file content assertion logic for up-/download Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 21/54] test: guest import: add tests for PVE::GuestImport Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 22/54] tree-wide: introduce parsing module and replace usages of ISO_EXT_RE_0 Max R. Carrara
2026-04-28 15:20   ` Wolfgang Bumiller
2026-04-22 11:12 ` [PATCH pve-storage v1 23/54] common: test: set up parser testing code, add tests for 'iso' vtype Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 24/54] tree-wide: replace usages of VZTMPL_EXT_RE_1 with parsing functions Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 25/54] tree-wide: replace usages of BACKUP_EXT_RE_2 " Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 26/54] tree-wide: replace usages of inline regexes for snippets with parsers Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 27/54] tree-wide: partially replace usages of regexes for 'import' vtype Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 28/54] tree-wide: replace remaining " Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 29/54] plugin: simplify recently refactored logic in parse_volname method Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 30/54] plugin: simplify recently refactored logic in get_subdir_files helper Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 31/54] storage: simplify recently refactored logic in path_to_volume_id sub Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 32/54] api: status: simplify recently added parsing helper for file transfers Max R. Carrara
2026-04-22 11:12 ` [PATCH pve-storage v1 33/54] plugin: use parsing helper in parse_volume_id sub Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 34/54] test: list volumes: reorganize and modernize test running code Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 35/54] test: list volumes: fix broken test checking for vmlist modifications Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 36/54] test: list volumes: introduce new format for test cases Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 37/54] test: list volumes: remove legacy code and migrate cases to new format Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 38/54] test: list volumes: document behavior wrt. undeclared content types Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 39/54] plugin: correct comment in get_subdir_files helper Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 40/54] test: parse volname: modernize code Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 41/54] test: parse volname: adapt tests regarding 'import' volume type Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 42/54] test: parse volname: move VM disk test creation into separate block Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 43/54] test: parse volname: move backup file test creation into sep. block Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 44/54] test: parse volname: parameterize test case creation for some vtypes Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 45/54] test: volume id: modernize code Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 46/54] test: volume id: rename 'volname' test case parameter to 'file' Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 47/54] test: filesystem path: modernize code Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 48/54] fix #2884: implement nested subdir scanning and support 'iso' vtype Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 49/54] fix #2884: support nested subdir scanning for 'vztmpl' volume type Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 50/54] fix #2884: support nested subdir scanning for 'snippets' vtype Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 51/54] test: add more tests for 'import' vtype & guard against nested subdirs Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 52/54] test: add tests guarding against subdir scanning for vtypes Max R. Carrara
2026-04-22 11:13 ` [PATCH pve-storage v1 53/54] storage api: mark old public regexes for removal, bump APIVER & APIAGE Max R. Carrara
2026-04-28 15:22   ` Wolfgang Bumiller
2026-04-22 11:13 ` [PATCH pve-manager v1 54/54] fix #2884: ui: storage: add field for 'max-scan-depth' property Max R. Carrara

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=dzlh5oyu73idd2s6e3ovzrivdhhvxdmkiumwjvzq6nbjotidst@2i25ysrie7xt \
    --to=w.bumiller@proxmox.com \
    --cc=m.carrara@proxmox.com \
    --cc=pve-devel@lists.proxmox.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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal