[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-22T11:16:53Z

In order to make the parsing logic in `path_to_volume_id()` both
simpler and more manageable for future changes, do the following:

1. Iterate over a list of vtypes that store their contents in
subdirectories and fetch said subdirectories dynamically instead of
hardcoding them.

2. Explicitly exclude the `rootdir` vtype in this list of vtypes, as
its subdirectory is currently not used.

3. Strip the vtype subdirectory from the beginning of the path in
order to make matching on the file name easier. Return early as a
safeguard if this fails.

4. Adapt the original regexes in the parsing helper to not include the
vtype subdir anymore, since this is now being stripped.

5. Instead of returning a 2-element array, return `undef` if the path
fails to parse, and the volume ID (volid) alone if parsing
succeeds. Since the caller is now passing along the vtype, it does
not need to be returned anymore.

Note that the parsing logic of the `parse_volid_from_file_path()`
helper inside `path_to_volume_id()` now is very similar to the parsing
logic of the `get_subdir_file_info()` closure in the
`get_subdir_files()` helper inside `Plugin.pm`. This opens up further
simplification / code de-duplication in the future, so that our
parsing logic can eventually be kept in one place only.

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/Storage.pm | 73 +++++++++++++++++++++++++++++++---------------
1 file changed, 50 insertions(+), 23 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 2886ab9e..1b0569dd 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -23,6 +23,7 @@ use PVE::INotify;
use PVE::RPCEnvironment;
use PVE::SSHInfo;
use PVE::Storage::Common qw(
+ plugin_get_default_vtype_subdirs
plugin_get_vtype_subdir
);
use PVE::RESTEnvironment qw(log_warn);
@@ -712,17 +713,31 @@ sub path_to_volume_id {
# for example when nfs storage is not mounted
$path = abs_path($path) || $path;

+ my $get_vtypes_to_check = sub {
+ my $default_vtype_subdirs = plugin_get_default_vtype_subdirs();
+
+ # TODO: vtype split: handle directory for 'rootdir' type.
+ # CTs currently use the same dir as VMs --> the dir for the 'images' type.
+ # Directory is therefore unused right now, so exclude it.
+ delete $default_vtype_subdirs->{rootdir};
+
+ return [sort keys $default_vtype_subdirs->%*];
+ };
+
my $parse_volid_from_file_path = sub {
- my ($plugin, $sid, $scfg) = @_;
+ my ($plugin, $sid, $scfg, $vtype) = @_;

- 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');
+ my $vtype_subdir = plugin_get_vtype_subdir($scfg, $vtype);

- if ($path =~ m!^\Q$imagedir\E/(\d+)/([^/\s]+)$!) {
+ # Strip the vtype subdir from beginning of the current path
+ # so that we don't have to take it into account when parsing the file name
+ my $filename = $path;
+ if ($filename !~ s!^\Q$vtype_subdir\E!!) {
+ return;
+ }
+
+ if ($vtype eq 'images') {
+ return if $filename !~ m!/(\d+)/([^/\s]+)$!;
my $vmid = $1;
my $name = $2;

@@ -731,48 +746,60 @@ sub path_to_volume_id {
my ($storeid, $volname) = parse_volume_id($info->{volid});
my $volpath = $plugin->path($scfg, $volname, $storeid);
if ($volpath eq $path) {
- return ('images', $info->{volid});
+ return $info->{volid};
}
}

- return ('');
+ return;
}

- if ($path =~ m!^\Q$isodir\E/([^/]+$ISO_EXT_RE_0)$!) {
+ if ($vtype eq 'iso') {
+ return if $filename !~ m!/([^/]+$ISO_EXT_RE_0)$!;
my $name = $1;
- return ('iso', "$sid:iso/$name");
+ return "$sid:iso/$name";
}

- if ($path =~ m!^\Q$tmpldir\E/([^/]+$VZTMPL_EXT_RE_1)$!) {
+ if ($vtype eq 'vztmpl') {
+ return if $filename !~ m!/([^/]+$VZTMPL_EXT_RE_1)$!;
my $name = $1;
- return ('vztmpl', "$sid:vztmpl/$name");
+ return "$sid:vztmpl/$name";
}

- if ($path =~ m!^\Q$backupdir\E/([^/]+$BACKUP_EXT_RE_2)$!) {
+ if ($vtype eq 'backup') {
+ return if $filename !~ m!/([^/]+$BACKUP_EXT_RE_2)$!;
my $name = $1;
- return ('backup', "$sid:backup/$name");
+ return "$sid:backup/$name";
}

- if ($path =~ m!^\Q$snippetsdir\E/([^/]+)$!) {
+ if ($vtype eq 'snippets') {
+ return if $filename !~ m!/([^/]+)$!;
my $name = $1;
- return ('snippets', "$sid:snippets/$name");
+ return "$sid:snippets/$name";
}

- if ($path =~ m!^\Q$importdir\E/(${SAFE_CHAR_CLASS_RE}+${IMPORT_EXT_RE_1})$!) {
+ if ($vtype eq 'import') {
+ return if $filename !~ m!/(${SAFE_CHAR_CLASS_RE}+${IMPORT_EXT_RE_1})$!;
my $name = $1;
- return ('import', "$sid:import/$name");
+ return "$sid:import/$name";
}

- return ('');
+ return;
};

+ my $vtypes_to_check = $get_vtypes_to_check->();
+
for my $sid (keys $ids->%*) {
my $scfg = $ids->{$sid};
next if !$scfg->{path};
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});

- my ($vtype, $volid) = $parse_volid_from_file_path->($plugin, $sid, $scfg);
- return ($vtype, $volid) if $vtype;
+ for my $vtype ($vtypes_to_check->@*) {
+ my $volid = $parse_volid_from_file_path->($plugin, $sid, $scfg, $vtype);
+
+ if (defined($volid)) {
+ return ($vtype, $volid);
+ }
+ }
}

# can't map path to volume id
--
2.47.3