From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id AB3991FF13B for ; Wed, 22 Apr 2026 13:21:19 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0FCD61B0CA; Wed, 22 Apr 2026 13:16:22 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v1 48/54] fix #2884: implement nested subdir scanning and support 'iso' vtype Date: Wed, 22 Apr 2026 13:13:14 +0200 Message-ID: <20260422111322.257380-49-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260422111322.257380-1-m.carrara@proxmox.com> References: <20260422111322.257380-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776856367387 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.082 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ETTHVPZFVHRI3BOBUUS2KCR6KC3TJADF X-Message-ID-Hash: ETTHVPZFVHRI3BOBUUS2KCR6KC3TJADF X-MailFrom: m.carrara@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Introduce a new 'max-scan-depth' property for directory-based storages. This property makes it so that files for specific volume types may also be put into (nested) subdirectories inside the volume type's directory. The maximum allowed depth is between 0 and 50, with 0 being the default, which corresponds to the current behavior. Support nested subdirectories for ISOs (the 'iso' vtype) only for now. Achieve all of this by calling the `get_subdir_files()` helper in `PVE::Storage::Plugin` recursively. Note that the default recursion limit in Perl appears to be exactly 100 [0], way above the maximum value of 50 for the 'max-scan-depth' property. Should we need deeper nesting, we can always make the subroutine iterative instead of recursive later. Add additional test cases wherever applicable to account for nested subdirectories, including cases that check whether the limit set by the property is upheld, and also cases that check for the existence of parent directory references ('..') in volume names. [0]: https://perldoc.perl.org/perl5101delta#Deep-recursion-on-subroutine-%22%25s%22 Originally-by: Noel Ullreich Signed-off-by: Max R. Carrara --- src/PVE/Storage/BTRFSPlugin.pm | 1 + src/PVE/Storage/CephFSPlugin.pm | 1 + src/PVE/Storage/Common/Parse.pm | 14 +++ src/PVE/Storage/Common/test/parser_tests.pl | 50 ++++++++ src/PVE/Storage/DirPlugin.pm | 1 + src/PVE/Storage/Plugin.pm | 41 ++++++- src/test/filesystem_path_test.pm | 8 ++ src/test/list_volumes_test.pm | 127 ++++++++++++++++++++ src/test/parse_volname_test.pm | 56 +++++++++ src/test/path_to_volume_id_test.pm | 7 ++ 10 files changed, 302 insertions(+), 4 deletions(-) diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm index 67b442f6..0a29fd31 100644 --- a/src/PVE/Storage/BTRFSPlugin.pm +++ b/src/PVE/Storage/BTRFSPlugin.pm @@ -68,6 +68,7 @@ sub properties { sub options { return { path => { fixed => 1 }, + 'max-scan-depth' => { optional => 1 }, nodes => { optional => 1 }, shared => { optional => 1 }, disable => { optional => 1 }, diff --git a/src/PVE/Storage/CephFSPlugin.pm b/src/PVE/Storage/CephFSPlugin.pm index fbc97113..b31f53d9 100644 --- a/src/PVE/Storage/CephFSPlugin.pm +++ b/src/PVE/Storage/CephFSPlugin.pm @@ -140,6 +140,7 @@ sub options { return { path => { fixed => 1 }, 'content-dirs' => { optional => 1 }, + 'max-scan-depth' => { optional => 1 }, monhost => { optional => 1 }, nodes => { optional => 1 }, subdir => { optional => 1 }, diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm index 9fb45b0c..f98b702d 100644 --- a/src/PVE/Storage/Common/Parse.pm +++ b/src/PVE/Storage/Common/Parse.pm @@ -57,6 +57,10 @@ my $RE_SAFE_CHAR_CLASS = qr/[a-zA-Z0-9\-\.\+\=\_]/; my $RE_SAFE_CHAR_WITH_WHITESPACE_CLASS = qr/[ a-zA-Z0-9\-\.\+\=\_]/; +my $RE_DIRECTORY_COMPONENTS = qr! + ( ($RE_SAFE_CHAR_WITH_WHITESPACE_CLASS)+ / )*? # NOTE: non-greedy matching +!xn; + my $RE_PARENT_DIR = quotemeta('..'); my $RE_CONTAINS_PARENT_DIR = qr! ( ^$RE_PARENT_DIR/ ) # ../ --> Beginning of path @@ -73,6 +77,7 @@ my $RE_VMID = qr![1-9][0-9]{2,8}!; my $RE_ISO_FILE_PATH = qr! (? + (? $RE_DIRECTORY_COMPONENTS )? (? [^/]+ \. (? (?i: iso|img) ) ) ) !xn; @@ -206,6 +211,15 @@ my sub format_named_groups(%groups) { my @disk_path_components = (); + if (defined($result->{dir})) { + if ($result->{dir} eq '') { + delete $result->{dir}; + } else { + $result->{dir} = strip_trailing_path_separators($result->{dir}); + push(@disk_path_components, $result->{dir}); + } + } + if (defined($result->{file})) { $result->{file} = strip_leading_path_separators($result->{file}); push(@disk_path_components, $result->{file}); diff --git a/src/PVE/Storage/Common/test/parser_tests.pl b/src/PVE/Storage/Common/test/parser_tests.pl index b6e42307..ad122cd2 100755 --- a/src/PVE/Storage/Common/test/parser_tests.pl +++ b/src/PVE/Storage/Common/test/parser_tests.pl @@ -73,6 +73,32 @@ my $volname_cases_iso_valid = [ volname => 'iso/Fedora.Img', }, }, + + # subdirectories + { + path => 'subdir/custom-debian.iso', + expected => { + file => 'custom-debian.iso', + ext => 'iso', + dir => 'subdir', + 'disk-path' => 'subdir/custom-debian.iso', + path => 'subdir/custom-debian.iso', + vtype => 'iso', + volname => 'iso/subdir/custom-debian.iso', + }, + }, + { + path => 'deeply/nested/dir/hannah-montana-linux.IMG', + expected => { + file => 'hannah-montana-linux.IMG', + ext => 'IMG', + dir => 'deeply/nested/dir', + 'disk-path' => 'deeply/nested/dir/hannah-montana-linux.IMG', + path => 'deeply/nested/dir/hannah-montana-linux.IMG', + vtype => 'iso', + volname => 'iso/deeply/nested/dir/hannah-montana-linux.IMG', + }, + }, ]; my $volname_cases_iso_invalid = [ @@ -84,6 +110,30 @@ my $volname_cases_iso_invalid = [ }, expected => undef, }, + { + description => "Parent dir reference in path (beginning) (iso)", + args => { + path => '../custom-debian.iso', + vtype => 'iso', + }, + expected => undef, + }, + { + description => "Parent dir reference in path (middle) (iso)", + args => { + path => 'subdir/../custom-debian.iso', + vtype => 'iso', + }, + expected => undef, + }, + { + description => "Parent dir reference in path (end) (iso)", + args => { + path => 'subdir/custom-debian.iso/..', + vtype => 'iso', + }, + expected => undef, + }, ]; my $volname_cases_vztmpl_valid = [ diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm index 80c4a031..58b942e7 100644 --- a/src/PVE/Storage/DirPlugin.pm +++ b/src/PVE/Storage/DirPlugin.pm @@ -81,6 +81,7 @@ sub options { return { path => { fixed => 1 }, 'content-dirs' => { optional => 1 }, + 'max-scan-depth' => { optional => 1 }, nodes => { optional => 1 }, shared => { optional => 1 }, disable => { optional => 1 }, diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 101e0b6d..0fbcbd4b 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -233,6 +233,15 @@ my $defaultData = { format => "pve-dir-override-list", optional => 1, }, + 'max-scan-depth' => { + description => "Maximum depth of subdirectories to traverse when searching for" + . " ISOs in directories.", + type => 'integer', + default => 0, + minimum => 0, + maximum => 50, + optional => 1, + }, options => { description => "NFS/CIFS mount options (see 'man nfs' or 'man mount.cifs')", type => 'string', @@ -1669,10 +1678,16 @@ sub list_images { # $vtype = my sub get_subdir_files { - my ($storeid, $scfg, $vtype, $vmid) = @_; + use feature 'current_sub'; # Needed for the __SUB__ token further below + + my ($storeid, $scfg, $vtype, $vmid, $remaining_depth, $current_path) = @_; my $vtype_subdir = plugin_get_vtype_subdir($scfg, $vtype); + if (!defined($current_path)) { + $current_path = $vtype_subdir; + } + my $res = []; my $get_subdir_file_info = sub { @@ -1752,10 +1767,24 @@ my sub get_subdir_files { return; }; - for my $path (<$vtype_subdir/*>) { + for my $path (<$current_path/*>) { my $st = File::stat::stat($path); - next if (!$st || S_ISDIR($st->mode)); + next if !$st; + + if (S_ISDIR($st->mode)) { + if (defined($remaining_depth) && $remaining_depth > 0) { + # Note: Have to call the subroutine via __SUB__->(...) because + # lexical subroutines otherwise cannot reference themselves + my $inner_res = __SUB__->( + $storeid, $scfg, $vtype, $vmid, $remaining_depth - 1, $path, + ); + + push($res->@*, $inner_res->@*); + } + + next; + } if (defined(my $info = $get_subdir_file_info->($path, $st))) { $info->{size} = $st->size; @@ -1773,6 +1802,10 @@ my sub get_subdir_files { sub list_volumes { my ($class, $storeid, $scfg, $vmid, $content_types) = @_; + my $depth = $scfg->{'max-scan-depth'} // 0; + $depth = 0 if $depth < 0; + $depth = 50 if $depth > 50; + my $res = []; my $vmlist = PVE::Cluster::get_vmlist(); @@ -1786,7 +1819,7 @@ sub list_volumes { return if !$scfg->{path}; if ($type eq 'iso' && !defined($vmid)) { - return get_subdir_files($storeid, $scfg, 'iso', undef); + return get_subdir_files($storeid, $scfg, 'iso', undef, $depth); } if ($type eq 'vztmpl' && !defined($vmid)) { diff --git a/src/test/filesystem_path_test.pm b/src/test/filesystem_path_test.pm index 5a715ce9..26a74a0d 100644 --- a/src/test/filesystem_path_test.pm +++ b/src/test/filesystem_path_test.pm @@ -48,6 +48,14 @@ my $tests = [ "$DEFAULT_STORAGE_DIR/template/iso/my-awesome-proxmox.iso", undef, 'iso', ], }, + { + volname => 'iso/foo/bar/baz/my-awesome-proxmox.iso', + snapname => undef, + expected => [ + "$DEFAULT_STORAGE_DIR/template/iso/foo/bar/baz/my-awesome-proxmox.iso", undef, + 'iso', + ], + }, { volname => "backup/vzdump-qemu-1234-2020_03_30-21_12_40.vma", snapname => undef, diff --git a/src/test/list_volumes_test.pm b/src/test/list_volumes_test.pm index ce35c782..0daaba94 100644 --- a/src/test/list_volumes_test.pm +++ b/src/test/list_volumes_test.pm @@ -1296,6 +1296,133 @@ my $test_param_list = [ }, ], }, + { + description => "VMID: none, no nested subdirectories when using defaults", + storeid => $DEFAULT_STOREID, + scfg => $DEFAULT_SCFG, + vmid => undef, + vtypes => ['iso', 'vztmpl', 'snippets', 'import'], + cases => [ + { + file => "$DEFAULT_STORAGE_PATH/template/iso/some-installer.iso", + expected => { + content => 'iso', + ctime => $DEFAULT_CTIME, + format => 'iso', + size => $DEFAULT_SIZE, + volid => 'local:iso/some-installer.iso', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/template/iso/1/some-installer.iso", + expected => undef, + }, + { + file => "$DEFAULT_STORAGE_PATH/template/iso/1/2/3/4/5/some-installer.iso", + expected => undef, + }, + ], + }, + { + description => "VMID: none, nested subdirectories allowed, max-scan-depth = 1", + storeid => $DEFAULT_STOREID, + scfg => { + type => 'dir', + path => $DEFAULT_STORAGE_PATH, + shared => 0, + 'max-scan-depth' => 1, + content => { + iso => 1, + vztmpl => 1, + snippets => 1, + import => 1, + }, + }, + vmid => undef, + vtypes => ['iso', 'vztmpl', 'snippets', 'import'], + cases => [ + { + file => "$DEFAULT_STORAGE_PATH/template/iso/some-installer.iso", + expected => { + content => 'iso', + ctime => $DEFAULT_CTIME, + format => 'iso', + size => $DEFAULT_SIZE, + volid => 'local:iso/some-installer.iso', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/template/iso/1/some-installer.iso", + expected => { + content => 'iso', + ctime => $DEFAULT_CTIME, + format => 'iso', + size => $DEFAULT_SIZE, + volid => 'local:iso/1/some-installer.iso', + }, + }, + { + # Exceeds max-scan-depth + file => "$DEFAULT_STORAGE_PATH/template/iso/1/2/some-installer.iso", + expected => undef, + }, + ], + }, + { + description => "VMID: none, nested subdirectories allowed, max-scan-depth = 5", + storeid => $DEFAULT_STOREID, + scfg => { + type => 'dir', + path => $DEFAULT_STORAGE_PATH, + shared => 0, + 'max-scan-depth' => 5, + content => { + iso => 1, + vztmpl => 1, + snippets => 1, + import => 1, + }, + }, + vmid => undef, + vtypes => ['iso', 'vztmpl', 'snippets', 'import'], + cases => [ + { + file => "$DEFAULT_STORAGE_PATH/template/iso/some-installer.iso", + expected => { + content => 'iso', + ctime => $DEFAULT_CTIME, + format => 'iso', + size => $DEFAULT_SIZE, + volid => 'local:iso/some-installer.iso', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/template/iso/1/some-installer.iso", + expected => { + content => 'iso', + ctime => $DEFAULT_CTIME, + format => 'iso', + size => $DEFAULT_SIZE, + volid => 'local:iso/1/some-installer.iso', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/template/iso/1/2/3/4/5/some-installer.iso", + expected => { + content => 'iso', + ctime => $DEFAULT_CTIME, + format => 'iso', + size => $DEFAULT_SIZE, + volid => 'local:iso/1/2/3/4/5/some-installer.iso', + }, + }, + { + # Exceeds max-scan-depth + file => "$DEFAULT_STORAGE_PATH/template/iso/1/2/3/4/5/6/some-installer.iso", + expected => undef, + }, + ], + }, ]; # Additional test cases that cannot be constructed within the list above diff --git a/src/test/parse_volname_test.pm b/src/test/parse_volname_test.pm index 1dcec9d6..24d1ed0e 100644 --- a/src/test/parse_volname_test.pm +++ b/src/test/parse_volname_test.pm @@ -167,10 +167,66 @@ my $tests = [ volname => "iso/$file_name", expected => ['iso', "$file_name", undef, undef, undef, undef, 'raw'], }, + { + description => "ISO image, $suffix, subdirectory", + volname => "iso/foo/$file_name", + expected => [ + 'iso', "foo/$file_name", undef, undef, undef, undef, 'raw', + ], + }, + { + description => "ISO image, $suffix, nested subdirectories", + volname => "iso/foo/bar/baz/$file_name", + expected => [ + 'iso', "foo/bar/baz/$file_name", undef, undef, undef, undef, 'raw', + ], + }, + { + description => "ISO image, $suffix, subdirectory with same name as file", + volname => "iso/$file_name/$file_name", + expected => [ + 'iso', "$file_name/$file_name", undef, undef, undef, undef, 'raw', + ], + }, ); push($tests->@*, @extra_tests); } + + # Failed tests + { + my $file_name = "$prefix.iso"; + + my @extra_failed_tests = ( + { + description => + "ISO image, iso, parent directory reference before volume type prefix", + volname => "../iso/$file_name", + expected => "unable to parse directory volume name '../iso/$file_name'\n", + }, + { + description => + "ISO image, iso, parent directory reference at beginning of volume path", + volname => "iso/../$file_name", + expected => "unable to parse directory volume name 'iso/../$file_name'\n", + }, + { + description => + "ISO image, iso, parent directory reference at end of volume path", + volname => "iso/$file_name/..", + expected => "unable to parse directory volume name 'iso/$file_name/..'\n", + }, + { + description => + "ISO image, iso, parent directory reference between dir components of volume path", + volname => "iso/foo/../bar/$file_name", + expected => + "unable to parse directory volume name 'iso/foo/../bar/$file_name'\n", + }, + ); + + push($tests->@*, @extra_failed_tests); + } } # Test cases for container templates diff --git a/src/test/path_to_volume_id_test.pm b/src/test/path_to_volume_id_test.pm index 6c7d0d67..bc87d289 100644 --- a/src/test/path_to_volume_id_test.pm +++ b/src/test/path_to_volume_id_test.pm @@ -123,6 +123,13 @@ my $tests = [ 'iso', 'local:iso/yet-again-a-installation-disk.iso', ], }, + { + description => 'ISO file, nested subdirectories', + file => "$DEFAULT_STORAGE_DIR/template/iso/foo/bar/hannah-montana-linux-installer.iso", + expected => [ + 'iso', 'local:iso/foo/bar/hannah-montana-linux-installer.iso', + ], + }, { description => 'CT template, tar.gz', file => "$DEFAULT_STORAGE_DIR/template/cache/debian-10.0-standard_10.0-1_amd64.tar.gz", -- 2.47.3