From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 4E0FE1FF0AB for ; Wed, 23 Sep 2026 17:06:17 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 02C80215F1; Wed, 23 Sep 2026 17:06:14 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH manager/storage v2 00/50] Fix #2884: Implement Subdirectory Scanning for Dir-Based Storage Types Date: Wed, 23 Sep 2026 17:05:14 +0200 Message-ID: <20260923150606.531239-1-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790175966804 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.492 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: 5DHUQJGBCSVJU7MTBUDSAMYUJGRRKPSF X-Message-ID-Hash: 5DHUQJGBCSVJU7MTBUDSAMYUJGRRKPSF 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: Fix #2884: Implement Subdirectory Scanning for Dir-Based Storage Types - v2 =========================================================================== Basically what the title says. Implement subdirectory scanning for directory-based storage types, which includes directories (duh), NFS, CIFS, CephFS, and BTRFS. This fixes #2884 [2884] and the more narrowly-scoped #623 [623]. See the initial series's cover letter [init-cover] for the entire context. *Huge* thanks to @Wolfgang for the thorough review on v1, and also for the off-list correspondence regarding this! :) This refresh incorporates all of his feedback. Changes Since v1 ---------------- Since this series is quite massive, I decided to: * track all changes here, not just the notable ones * track changes to patches' index numbers * add references to each suggestion relevant for the change, if applicable / mentioned over the mailing list While this took a bit of extra time, it should hopefully alleviate some tracking work on the maintainers' end. - Clarify that `PVE::Storage::Common::plugin_get_default_vtype_subdirs()` returns an absolute path in its docstring (patch #6). - Do not call if-elsif chains "needless" in the patches that break them up anymore (patch #8 and #14). Ref.: https://lore.proxmox.com/pve-devel/w44tdwzbsu3bh4xggcss6b3ukrv7imddbzackyf3qhsfv2j5mn@bogediza5n5g/ - Use a lookahead for a slash (`(?=/)`) after the `\E` in `get_subdir_files() in patch #9. Ref.: https://lore.proxmox.com/pve-devel/ex6gdtlicrwrxpw6bmz5awhw5bnnt4g5uokuajq4fn3b2kkkd6@mjkvz3oppvzi/ - Use a lookahead for a slash (`(?=/)`) after the `\E` in `path_to_volume_id()` in patch #16. Ref.: https://lore.proxmox.com/pve-devel/j5ygsulsykpaevszwpuv2726r7ogzz6wd2346telm7szlqmbwl@opm2banmchnp/ - Fix filtering using `grep` in patch #17 that made the entire check a no-op, which could have affected security. Ref.: https://lore.proxmox.com/pve-devel/hwb7vmxog4p3v3iqe74ptettlev3nk4vcgaqdrs67nnoohmh72@2wncblbtmcij/ - Do not use a hash in the `split_leading_dir_from_path()` helper in patch #22 anymore. Ref.: https://lore.proxmox.com/pve-devel/jfa5fql3svnils2dlklyz4vrgyo4jmn24tuv2wa7uu7lf2wgzb@7arsjxlzdivs/ - Rename the subs added in patch #22 to better distinguish between those that take a relative path and those take an absolute path. Ref.: https://lore.proxmox.com/pve-devel/jfa5fql3svnils2dlklyz4vrgyo4jmn24tuv2wa7uu7lf2wgzb@7arsjxlzdivs/ - Clarify in the docstrings of certain subs added in patch #22 that we return `undef` in scalar context, and an empty list in list context. Ref.: https://lore.proxmox.com/pve-devel/jfa5fql3svnils2dlklyz4vrgyo4jmn24tuv2wa7uu7lf2wgzb@7arsjxlzdivs/ - Improve the docstrings of the parsing subs in patch #22 by using examples for expected outputs and by making them a little less verbose. - Document further special cases in every patch that introduces them -- patches #24, #25, #28, and also #44-46 (formerly patches #48-50 in v1). - Fold patches #29 - #32 of v1 into patch #29 of this series, since a separate patch for each site was a bit *too* granular. - Do not replace string interpolation with concatenation in patch #7 anymore. Ref.: https://lore.proxmox.com/pve-devel/ehmk4jjaekoqsxf2pqy6imbfouvaeuwdkzc2odvlskf72yl7yp@3lfm4ihnutiw/ - Do not return volumes of undeclared content types anymore instead of just documenting that behavior using test cases in patch #34 (formerly patch #38 in v1). Instead, return an empty list if the volumes of an undeclared content type are queried. Ref.: https://lore.proxmox.com/pve-devel/fgno5lzql7h2jrnyfcmn2usahi5e2chteiwfw543ak7mahglvq@ph2sigxrk2ty/ - Use greedy matching instead of non-greedy matching for the subdirectory component regex in patch #44 (formerly patch #48 in v1). Ref.: https://lore.proxmox.com/pve-devel/cvjtmro3odmdgh574wnfql3nlaezezukokbsuxdnmycadfcuah@4ckwm5hn2bci/ - Add API deprecations within the commits that actually deprecate things instead of bumping the APIVER + APIAGE twice throughout the series. This means that if e.g. one commit makes a regex in `PVE::Storage` obsolete, the deprecation notice in `ApiChangeLog` and TODO comment for that regex are added right away. Ref.: https://lore.proxmox.com/pve-devel/xapbgpwdqk7h6uebzkgxei7v7gamzbhyin2blbohd2bt4kyrr4@ogniag55fdai/ - Add patch #50 that replaces the usage of `$plugin->get_subdir()` with `PVE::Storage::Common::plugin_get_vtype_subdir()`, since the latter is now deprecated. Ref.: https://lore.proxmox.com/pve-devel/dzlh5oyu73idd2s6e3ovzrivdhhvxdmkiumwjvzq6nbjotidst@2i25ysrie7xt/ Additional Notes ---------------- Since this series also touches the compression extensions and moves them to the new parsing module it introduces, I considered expanding upon that, cleaning up our compression-related code as well. However, given that this series is already large enough, I consider this out of scope for now and will probably follow up on this in a separate series. Testing ------- If anyone could give this series a spin, I'd be most grateful! Here are some interesting things you could check out (non-exhaustive): - Configuring the new 'max-scan-depth' property in the UI - Adding subdirectories in your ISO, LXC template and snippets dirs (and populating those dirs afterwards) - Checking whether the depth limit is honored --> 0 is the default, which retains the current behavior of not scanning through any subdirs - Checking whether imports (.ova files etc) still work as expected - ISO / CT template upload / deletion - ... Also note that I ran the tests in the repository for every single patch that I added; if you want to do this for yourself as a sanity check, try the following: git rebase -i --autostash --autosquash origin/master -x 'cd src && make test' References ---------- [2884]: https://bugzilla.proxmox.com/show_bug.cgi?id=2884 [623]: https://bugzilla.proxmox.com/show_bug.cgi?id=623 [init-cover]: https://lore.proxmox.com/pve-devel/20260422111322.257380-1-m.carrara@proxmox.com/ Summary of Changes ------------------ pve-storage: Max R. Carrara (48): test: plugin tests: run tests with at most 4 jobs plugin, common: remove superfluous use of =pod command paragraph common: add POD headings for groups of helpers common: use Exporter module for PVE::Storage::Common plugin: make get_subdir_files a proper subroutine and update style plugin api: replace helpers w/ standalone subs, bump API version & age common: prevent autovivification in plugin_get_vtype_subdir helper plugin: break up if-elsif chain into separate if-blocks plugin: adapt get_subdir_files helper of list_volumes API method plugin: update code style of list_volumes plugin API method plugin: use closure for obtaining raw volume data in list_volumes plugin: use closure for inner loop logic in list_volumes storage: update code style in function path_to_volume_id storage: break up if-elsif chain in path_to_volume_id storage: heave vtype file path parsing logic inside loop into helper storage: clean up code that was moved into helper in path_to_volume_id api: status: move content type assert for up-/downloads into helper api: status: use helper from common module to get content directory api: status: move up-/download file path parsing code into helper api: status: simplify file content assertion logic for up-/download test: guest import: add tests for PVE::GuestImport tree-wide: introduce parsing module and replace usages of ISO_EXT_RE_0 common: test: set up parser testing code, add tests for 'iso' vtype tree-wide: replace usages of VZTMPL_EXT_RE_1 with parsing functions tree-wide: replace usages of BACKUP_EXT_RE_2 with parsing functions tree-wide: replace usages of inline regexes for snippets with parsers tree-wide: partially replace usages of regexes for 'import' vtype tree-wide: replace remaining usages of regexes for 'import' vtype tree-wide: simplify recently refactored parsing logic test: list volumes: reorganize and modernize test running code test: list volumes: fix broken test checking for vmlist modifications test: list volumes: introduce new format for test cases test: list volumes: remove legacy code and migrate cases to new format plugin: do not return volumes of undeclared content types anymore plugin: correct comment in get_subdir_files helper test: parse volname: modernize code test: parse volname: adapt tests regarding 'import' volume type test: parse volname: move VM disk test creation into separate block test: parse volname: move backup file test creation into sep. block test: parse volname: parameterize test case creation for some vtypes test: volume id: modernize code test: volume id: rename 'volname' test case parameter to 'file' test: filesystem path: modernize code fix #2884: implement nested subdir scanning and support 'iso' vtype fix #2884: support nested subdir scanning for 'vztmpl' volume type fix #2884: support nested subdir scanning for 'snippets' vtype test: add more tests for 'import' vtype & guard against nested subdirs test: add tests guarding against subdir scanning for vtypes ApiChangeLog | 45 + debian/control | 1 + src/PVE/API2/Storage/Status.pm | 198 +- src/PVE/BackupProvider/Plugin/Base.pm | 6 +- src/PVE/GuestImport.pm | 28 +- src/PVE/GuestImport/OVF.pm | 24 +- src/PVE/Makefile | 1 + src/PVE/Storage.pm | 118 +- src/PVE/Storage/BTRFSPlugin.pm | 16 +- src/PVE/Storage/CephFSPlugin.pm | 1 + src/PVE/Storage/Common.pm | 91 +- src/PVE/Storage/Common/Makefile | 5 + src/PVE/Storage/Common/Parse.pm | 674 ++++++ src/PVE/Storage/Common/test/Makefile | 6 + src/PVE/Storage/Common/test/parser_tests.pl | 1134 ++++++++++ src/PVE/Storage/Common/test/run_tests.pl | 25 + src/PVE/Storage/DirPlugin.pm | 1 + src/PVE/Storage/ESXiPlugin.pm | 6 - src/PVE/Storage/Makefile | 4 + src/PVE/Storage/Plugin.pm | 368 ++-- src/test/filesystem_path_test.pm | 109 +- src/test/get_subdir_test.pm | 12 +- src/test/guest_import_test.pl | 948 +++++++++ src/test/list_volumes_test.pm | 2095 +++++++++++++++---- src/test/parse_volname_test.pm | 696 ++++-- src/test/path_to_volume_id_test.pm | 201 +- src/test/run_plugin_tests.pl | 18 +- src/test/run_volume_access_tests.pl | 5 +- 28 files changed, 5833 insertions(+), 1003 deletions(-) create mode 100644 src/PVE/Storage/Common/Parse.pm create mode 100644 src/PVE/Storage/Common/test/Makefile create mode 100755 src/PVE/Storage/Common/test/parser_tests.pl create mode 100755 src/PVE/Storage/Common/test/run_tests.pl create mode 100755 src/test/guest_import_test.pl pve-manager: Max R. Carrara (2): fix #2884: ui: storage: add field for 'max-scan-depth' property pve8to9: use helper from common storage module to get vtype subdir PVE/CLI/pve8to9.pm | 5 +++-- www/manager6/storage/Base.js | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) Summary over all repositories: 30 files changed, 5850 insertions(+), 1005 deletions(-) -- Generated by murpp 0.12.0