public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH pve-storage, pve-manager v1 00/54] Fix #2884: Implement Subdirectory Scanning for Dir-Based Storage Types
@ 2026-04-22 11:12 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
                   ` (53 more replies)
  0 siblings, 54 replies; 55+ messages in thread
From: Max R. Carrara @ 2026-04-22 11:12 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 55+ messages in thread

end of thread, other threads:[~2026-04-22 11:21 UTC | newest]

Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-22 11:12 ` [PATCH pve-storage v1 07/54] common: prevent autovivification in plugin_get_vtype_subdir helper Max R. Carrara
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-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-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-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-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-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-22 11:13 ` [PATCH pve-manager v1 54/54] fix #2884: ui: storage: add field for 'max-scan-depth' property Max R. Carrara

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