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 0FFA51FF13B for ; Wed, 22 Apr 2026 13:16:53 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BD53118534; Wed, 22 Apr 2026 13:14:37 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v1 16/54] storage: clean up code that was moved into helper in path_to_volume_id Date: Wed, 22 Apr 2026 13:12:42 +0200 Message-ID: <20260422111322.257380-17-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: 1776856332259 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.085 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: SS6UHSN4XAOXL2Z6PO5CBVMSG52SOYBK X-Message-ID-Hash: SS6UHSN4XAOXL2Z6PO5CBVMSG52SOYBK 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: 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 --- 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