From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id D85211FF0AB for ; Wed, 23 Sep 2026 17:08:42 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5F07A217D4; Wed, 23 Sep 2026 17:07:03 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v2 16/50] storage: clean up code that was moved into helper in path_to_volume_id Date: Wed, 23 Sep 2026 17:05:30 +0200 Message-ID: <20260923150606.531239-17-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260923150606.531239-1-m.carrara@proxmox.com> References: <20260923150606.531239-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: 1790176000289 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.443 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: 566UZK5J2FR3OJ6DCNRYSAJ2TDIO2FPZ X-Message-ID-Hash: 566UZK5J2FR3OJ6DCNRYSAJ2TDIO2FPZ 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 bb3e565d..485bd822 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