From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id E69F31FF13B for ; Wed, 22 Apr 2026 13:13:33 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E07B6163D3; Wed, 22 Apr 2026 13:13:28 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage, pve-manager v1 00/54] Fix #2884: Implement Subdirectory Scanning for Dir-Based Storage Types Date: Wed, 22 Apr 2026 13:12:26 +0200 Message-ID: <20260422111322.257380-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: 1776856314760 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.087 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com,esxiplugin.pm,status.pm,common.pm,dirplugin.pm,plugin.pm,storage.pm,cephfsplugin.pm,parse.pm,guestimport.pm,btrfsplugin.pm] Message-ID-Hash: CHITGQVY4D6O7CJNOLLGRR6Q6U3HU44P X-Message-ID-Hash: CHITGQVY4D6O7CJNOLLGRR6Q6U3HU44P 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 - v1 =========================================================================== 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 [1] and the more narrowly-scoped #623 [2]. Note that this series is a complete refresh of N. Ullreich's original series [3]. Only the general idea of how to implement this was kept around [^1], but since a lot of the code the original series touched has changed in the last three or so years, a lot of cleaning up needed to be done before implementing subdir scanning was even sanely possible. In particular, all parsing related to disk files corresponding to certain volume types is done using a bunch of regexes [7] that are then interpolated ad hoc at various locations [4][5][6], more than I could list here. Since interpolating in yet another regex (or possibly more) into every single place where we do such parsing is brittle, error-prone, and also unmaintainable in the long term, I decided to clean up the relevant code in bite-sized pieces first. In other words, patches #01 - #47 are just preparation for the actual fixes #48 - #50. These prep patches are kept as contained as possible, which is why you see a large number of patches overall. For example, instead of refactoring everything related to one thing in one huge patch, the changes are split into things like "break up this if-elsif-chain", "move this block into this new helper", "adapt code style here", and so on. Otherwise, it would be incredibly hard to follow what's actually happening. Note that all this work ultimately results in the original parsing regexes [7] being phased out in patch #53. Along the way I also improve the existing tests, add new ones for existing code, and also introduce a new module named `PVE::Storage::Common:Parse`, whose purpose it is to collect all parsing-related functionality for things regarding storage. (Besides, parsing stuff should all be kept in one place anyways, so might as well introduce a proper module for that.) Finally, patch #54 makes the new 'max-scan-depth' available in the UI. Note that no other UI-related changes are added otherwise, so the ISO / CT template / snippets lists for storages have not changed. In the future, we might want to make those prettier in some way as well, so that it's easier to browse those lists (maybe something like a mini file browser or something). [^1]: That idea being to call the `get_subdir_files()` helper in `Plugin.pm` recursively. 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 ========== [1]: https://bugzilla.proxmox.com/show_bug.cgi?id=2884 [2]: https://bugzilla.proxmox.com/show_bug.cgi?id=623 [3]: https://lore.proxmox.com/pve-devel/20230721122314.80427-1-n.ullreich@proxmox.com/ [4]: https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/Storage/Plugin.pm;h=afd31414e94b17b4fc0582d0ab17db36fba19d8e;hb=refs/heads/master#l799 [5]: https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/API2/Storage/Status.pm;hb=e26583a0fe15742fc22de2dddc85baf0feb4809c#l593 [6]: https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/Storage.pm;hb=e26583a0fe15742fc22de2dddc85baf0feb4809c#l712 [7]: https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/Storage.pm;hb=e26583a0fe15742fc22de2dddc85baf0feb4809c#l116 Summary of Changes ================== pve-storage: Max R. Carrara (53): 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 needless 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 needless 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 plugin: simplify recently refactored logic in parse_volname method plugin: simplify recently refactored logic in get_subdir_files helper storage: simplify recently refactored logic in path_to_volume_id sub api: status: simplify recently added parsing helper for file transfers plugin: use parsing helper in parse_volume_id sub 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 test: list volumes: document behavior wrt. undeclared content types 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 storage api: mark old public regexes for removal, bump APIVER & APIAGE ApiChangeLog | 38 + debian/control | 1 + src/PVE/API2/Storage/Status.pm | 198 +- src/PVE/GuestImport.pm | 28 +- src/PVE/Makefile | 1 + src/PVE/Storage.pm | 109 +- 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 | 482 +++++ src/PVE/Storage/Common/test/Makefile | 6 + src/PVE/Storage/Common/test/parser_tests.pl | 1130 ++++++++++ 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 | 362 ++-- 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 | 2163 +++++++++++++++---- 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 +- 26 files changed, 5661 insertions(+), 995 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 (1): fix #2884: ui: storage: add field for 'max-scan-depth' property www/manager6/storage/Base.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) -- 2.47.3