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 4EA8A1FF0AB for ; Wed, 23 Sep 2026 17:07:35 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 75EBB216B9; Wed, 23 Sep 2026 17:07:00 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v2 09/50] plugin: adapt get_subdir_files helper of list_volumes API method Date: Wed, 23 Sep 2026 17:05:23 +0200 Message-ID: <20260923150606.531239-10-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: 1790175985629 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.465 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: KIDDNUM4YR7BRX67B47UZ3VG5BZTL7XB X-Message-ID-Hash: KIDDNUM4YR7BRX67B47UZ3VG5BZTL7XB 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: Pull the body of the loop inside `get_subdir_files()` out into a closure and break up the if-elsif chain into more readable if-clauses. Strip the vtype subdirectory from the beginning of the file paths being worked on inside that closure. This does not have an effect on how any of the subsequent regexes match on the path / file name, because they match at the end of a given path. Additionally, take take the storage config hash `$scfg` instead of the volume type subdir `$path` as parameter. Passing `$scfg` along makes it possible to obtain the subdir for the given `$vtype` inside the helper directly instead of passing both the vtype and its associated subdirectory along. All of these things make future changes easier to manage. Signed-off-by: Max R. Carrara --- src/PVE/Storage/Plugin.pm | 95 ++++++++++++++++++++++++++------------- 1 file changed, 64 insertions(+), 31 deletions(-) diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 475214ec..cbcb96b0 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -1699,37 +1699,54 @@ sub list_images { # $vtype = my sub get_subdir_files { - my ($storeid, $path, $vtype, $vmid) = @_; + my ($storeid, $scfg, $vtype, $vmid) = @_; + + my $vtype_subdir = plugin_get_vtype_subdir($scfg, $vtype); my $res = []; - for my $filename (<$path/*>) { - my $st = File::stat::stat($filename); + my $get_subdir_file_info = sub { + my ($path, $st) = @_; - next if (!$st || S_ISDIR($st->mode)); - - my $info; + # 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 'iso') { - next if $filename !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i; + return if $filename !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i; - $info = { volid => "$storeid:iso/$1", format => 'iso' }; + return { + volid => "$storeid:iso/$1", + format => 'iso', + }; + } - } elsif ($vtype eq 'vztmpl') { - next if $filename !~ m!/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!; + if ($vtype eq 'vztmpl') { + return if $filename !~ m!/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!; - $info = { volid => "$storeid:vztmpl/$1", format => $2 eq 'tar' ? $2 : "t$2" }; + return { + volid => "$storeid:vztmpl/$1", + format => $2 eq 'tar' ? $2 : "t$2", + }; + } - } elsif ($vtype eq 'backup') { - next if $filename !~ m!/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!; - my $original = $filename; + if ($vtype eq 'backup') { + return if $filename !~ m!/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!; + + my $original = $path; my $format = $2; $filename = $1; # only match for VMID now, to avoid false positives (VMID in parent directory name) - next if defined($vmid) && $filename !~ m/\S+-$vmid-\S+/; + return if defined($vmid) && $filename !~ m/\S+-$vmid-\S+/; - $info = { volid => "$storeid:backup/$filename", format => $format }; + my $info = { + volid => "$storeid:backup/$filename", + format => $format, + }; my $archive_info = eval { PVE::Storage::archive_info($filename) } // {}; @@ -1748,24 +1765,42 @@ my sub get_subdir_files { } $info->{protected} = 1 if -e PVE::Storage::protection_file_path($original); - } elsif ($vtype eq 'snippets') { - $info = { + return $info; + } + + if ($vtype eq 'snippets') { + return { volid => "$storeid:snippets/" . basename($filename), format => 'snippet', }; - } elsif ($vtype eq 'import') { - next + } + + if ($vtype eq 'import') { + return if $filename !~ m!/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!i; - $info = { volid => "$storeid:import/$1", format => "$2" }; + return { + volid => "$storeid:import/$1", + format => "$2", + }; } - $info->{size} = $st->size; - $info->{ctime} //= $st->ctime; + return; + }; - push @$res, $info; + for my $path (<$vtype_subdir/*>) { + my $st = File::stat::stat($path); + + next if (!$st || S_ISDIR($st->mode)); + + if (defined(my $info = $get_subdir_file_info->($path, $st))) { + $info->{size} = $st->size; + $info->{ctime} //= $st->ctime; + + push $res->@*, $info; + } } return $res; @@ -1784,18 +1819,16 @@ sub list_volumes { if ($type eq 'images' || $type eq 'rootdir') { $data = $class->list_images($storeid, $scfg, $vmid); } elsif ($scfg->{path}) { - my $path = plugin_get_vtype_subdir($scfg, $type); - if ($type eq 'iso' && !defined($vmid)) { - $data = get_subdir_files($storeid, $path, 'iso'); + $data = get_subdir_files($storeid, $scfg, 'iso', undef); } elsif ($type eq 'vztmpl' && !defined($vmid)) { - $data = get_subdir_files($storeid, $path, 'vztmpl'); + $data = get_subdir_files($storeid, $scfg, 'vztmpl', undef); } elsif ($type eq 'backup') { - $data = get_subdir_files($storeid, $path, 'backup', $vmid); + $data = get_subdir_files($storeid, $scfg, 'backup', $vmid); } elsif ($type eq 'snippets') { - $data = get_subdir_files($storeid, $path, 'snippets'); + $data = get_subdir_files($storeid, $scfg, 'snippets', undef); } elsif ($type eq 'import') { - $data = get_subdir_files($storeid, $path, 'import'); + $data = get_subdir_files($storeid, $scfg, 'import', undef); } } -- 2.47.3