[PATCH pve-storage v1 39/54] plugin: correct comment in get_subdir_files helper

Max R. Carrara
2026-04-22T11:17:52Z

The comment in the branch for 'backup' volume types in the
`get_subdir_files()` helper states that the condition below checks for
"false positives", apparently meaning that the VMID in the parent
directory might have been matched.

Challenge this claim by adding several new test cases that try to
cause the parser to return a wrong result.

As it turns out, the check the comment refers to is not (anymore)
there to check for "false positives" or VMIDs appearing in the parent
directory, but instead simply filters out backups that do not belong
to the provided VMID.

Note that if a backup has an arbitrary file name, that is, it's not
named something like "vzdump-qemu-1337-$TIMESTAMP.vma", that backup is
still returned in all cases.

Therefore, also include test cases for a plain "some-backup.tar.gz"
file and ensure that it is included in the expected output.

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/Storage/Plugin.pm | 4 +-
src/test/list_volumes_test.pm | 466 ++++++++++++++++++++++++++++++++++
2 files changed, 468 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 3853682b..101e0b6d 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1702,8 +1702,8 @@ my sub get_subdir_files {
my $format = $parts->{ext};
my $volume_path = $parts->{path};

- # Check if parsed VMID matched provided VMID in order to avoid
- # false positives (VMID in parent directory name)
+ # Check if parsed VMID matches provided VMID in order to avoid
+ # returning backups of other guests
my $parsed_vmid = $parts->{vmid};
if (defined($vmid) && defined($parsed_vmid)) {
return if $vmid ne $parsed_vmid;
diff --git a/src/test/list_volumes_test.pm b/src/test/list_volumes_test.pm
index 455fb227..ce35c782 100644
--- a/src/test/list_volumes_test.pm
+++ b/src/test/list_volumes_test.pm
@@ -566,6 +566,284 @@ my $test_param_list = [
},
],
},
+ {
+ description => 'VMID: none, backups of all guests',
+ storeid => $DEFAULT_STOREID,
+ scfg => $DEFAULT_SCFG,
+ vmid => undef,
+ vtypes => ['backup'],
+ cases => [
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz",
+ expected => {
+ content => 'backup',
+ ctime => 1585602700,
+ format => 'vma.gz',
+ size => $DEFAULT_SIZE,
+ subtype => 'qemu',
+ vmid => '16110',
+ volid => 'local:backup/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo",
+ expected => {
+ content => 'backup',
+ ctime => 1585602765,
+ format => 'vma.lzo',
+ size => $DEFAULT_SIZE,
+ subtype => 'qemu',
+ vmid => '16110',
+ volid => 'local:backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+ },
+ },
+ {
+ file => "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma",
+ expected => {
+ content => 'backup',
+ ctime => 1585602835,
+ format => 'vma',
+ size => $DEFAULT_SIZE,
+ subtype => 'qemu',
+ vmid => '16110',
+ volid => 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo",
+ expected => {
+ content => 'backup',
+ ctime => 1585604370,
+ format => 'tar.lzo',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz",
+ expected => {
+ content => 'backup',
+ ctime => 1585604970,
+ format => 'tar.gz',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst",
+ expected => {
+ content => 'backup',
+ ctime => 1585604970,
+ format => 'tar.zst',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst',
+ },
+ },
+ {
+ file => "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_59_30.tgz",
+ expected => {
+ content => 'backup',
+ ctime => 1585605570,
+ format => 'tgz',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_59_30.tgz',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",
+ expected => {
+ content => 'backup',
+ ctime => 1585604370,
+ format => 'tar.bz2',
+ size => $DEFAULT_SIZE,
+ subtype => 'openvz',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst",
+ expected => {
+ content => 'backup',
+ ctime => 1585602835,
+ format => 'vma.zst',
+ size => $DEFAULT_SIZE,
+ subtype => 'qemu',
+ vmid => '16110',
+ volid => 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-19253-2020_02_03-19_57_43.tar.gz",
+ expected => {
+ content => 'backup',
+ ctime => 1580759863,
+ format => 'tar.gz',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '19253',
+ volid => 'local:backup/vzdump-lxc-19253-2020_02_03-19_57_43.tar.gz',
+ },
+ },
+ {
+ file => "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-19254-2019_01_21-19_29_19.tar",
+ expected => {
+ content => 'backup',
+ ctime => 1548098959,
+ format => 'tar',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '19254',
+ volid => 'local:backup/vzdump-lxc-19254-2019_01_21-19_29_19.tar',
+ },
+ },
+ # Arbitrary backups are always included.
+ # Note that in this case, the 'vmid' key does not exist at all,
+ # instead of being set to undef.
+ {
+ file => "$DEFAULT_STORAGE_PATH/dump/some-backup.tar.gz",
+ expected => {
+ content => 'backup',
+ ctime => $DEFAULT_CTIME,
+ format => 'tar.gz',
+ size => $DEFAULT_SIZE,
+ subtype => 'unknown',
+ volid => 'local:backup/some-backup.tar.gz',
+ },
+ },
+ ],
+ },
+ {
+ description => 'VMID: 16112, backups of specific guest',
+ storeid => $DEFAULT_STOREID,
+ scfg => $DEFAULT_SCFG,
+ vmid => 16112,
+ vtypes => ['backup'],
+ cases => [
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz",
+ expected => undef,
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo",
+ expected => undef,
+ },
+ {
+ file => "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma",
+ expected => undef,
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo",
+ expected => {
+ content => 'backup',
+ ctime => 1585604370,
+ format => 'tar.lzo',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz",
+ expected => {
+ content => 'backup',
+ ctime => 1585604970,
+ format => 'tar.gz',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst",
+ expected => {
+ content => 'backup',
+ ctime => 1585604970,
+ format => 'tar.zst',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst',
+ },
+ },
+ {
+ file => "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_59_30.tgz",
+ expected => {
+ content => 'backup',
+ ctime => 1585605570,
+ format => 'tgz',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_59_30.tgz',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",
+ expected => {
+ content => 'backup',
+ ctime => 1585604370,
+ format => 'tar.bz2',
+ size => $DEFAULT_SIZE,
+ subtype => 'openvz',
+ vmid => '16112',
+ volid => 'local:backup/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
+ },
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst",
+ expected => undef,
+ },
+ {
+ file =>
+ "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-19253-2020_02_03-19_57_43.tar.gz",
+ expected => undef,
+ },
+ {
+ file => "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-19254-2019_01_21-19_29_19.tar",
+ expected => undef,
+ },
+ # Arbitrary backups are always included.
+ # In this case the 'vmid' also gets set to the provided one.
+ {
+ file => "$DEFAULT_STORAGE_PATH/dump/some-backup.tar.gz",
+ expected => {
+ content => 'backup',
+ ctime => $DEFAULT_CTIME,
+ format => 'tar.gz',
+ size => $DEFAULT_SIZE,
+ subtype => 'unknown',
+ vmid => 16112,
+ volid => 'local:backup/some-backup.tar.gz',
+ },
+ },
+ ],
+ },
{
description => 'VMID: none, parent, non-matching',
storeid => $DEFAULT_STOREID,
@@ -1020,6 +1298,194 @@ my $test_param_list = [
},
];

+# Additional test cases that cannot be constructed within the list above
+{
+ my $file_name = "vzdump-qemu-16110-2020_03_30-21_13_55.vma";
+ my $storage_path = File::Temp->newdir() . '/' . $file_name;
+
+ my $backup_vmid_test_params = {
+ description => "VMID: 16110, file name in path of storage",
+ storeid => $DEFAULT_STOREID,
+ scfg => {
+ type => 'dir',
+ path => $storage_path,
+ shared => 0,
+ content => {
+ backup => 1,
+ },
+ },
+ vmid => 16110,
+ vtypes => ['backup'],
+ cases => [
+ {
+ file => "$storage_path/dump/$file_name",
+ expected => {
+ content => 'backup',
+ ctime => 1585602835,
+ format => 'vma',
+ size => $DEFAULT_SIZE,
+ subtype => 'qemu',
+ vmid => '16110',
+ volid => "local:backup/$file_name",
+ },
+ },
+ ],
+ };
+
+ push($test_param_list->@*, $backup_vmid_test_params);
+}
+
+{
+ my $file_name = "vzdump-qemu-16110-2020_03_30-21_13_55.vma";
+ my $storage_path = File::Temp->newdir();
+
+ my $backup_vmid_test_params = {
+ description => "VMID: 16110, file name in vtype subdir of 'backup' vtype",
+ storeid => $DEFAULT_STOREID,
+ scfg => {
+ type => 'dir',
+ path => $storage_path,
+ shared => 0,
+ content => {
+ backup => 1,
+ },
+ 'content-dirs' => {
+ backup => $file_name,
+ },
+ },
+ vmid => 16110,
+ vtypes => ['backup'],
+ cases => [
+ {
+ file => "$storage_path/$file_name/$file_name",
+ expected => {
+ content => 'backup',
+ ctime => 1585602835,
+ format => 'vma',
+ size => $DEFAULT_SIZE,
+ subtype => 'qemu',
+ vmid => '16110',
+ volid => "local:backup/$file_name",
+ },
+ },
+ ],
+ };
+
+ push($test_param_list->@*, $backup_vmid_test_params);
+}
+
+{
+ my $file_name = "vzdump-qemu-16110-2020_03_30-21_13_55.vma";
+ my $storage_path = File::Temp->newdir() . '/' . $file_name;
+
+ my $backup_vmid_test_params = {
+ description => "VMID: 16110, file name in storage path and subdir of 'backup' vtype",
+ storeid => $DEFAULT_STOREID,
+ scfg => {
+ type => 'dir',
+ path => $storage_path,
+ shared => 0,
+ content => {
+ backup => 1,
+ },
+ 'content-dirs' => {
+ backup => $file_name,
+ },
+ },
+ vmid => 16110,
+ vtypes => ['backup'],
+ cases => [
+ {
+ file => "$storage_path/$file_name/$file_name",
+ expected => {
+ content => 'backup',
+ ctime => 1585602835,
+ format => 'vma',
+ size => $DEFAULT_SIZE,
+ subtype => 'qemu',
+ vmid => '16110',
+ volid => "local:backup/$file_name",
+ },
+ },
+ ],
+ };
+
+ push($test_param_list->@*, $backup_vmid_test_params);
+}
+
+{
+ my $file_name = "vzdump-qemu-16110-2020_03_30-21_13_55.vma";
+ my $storage_path = File::Temp->newdir() . '/' . $file_name;
+
+ my $backup_vmid_test_params = {
+ description =>
+ "VMID: 19253, file name with different VMID in storage path and subdir of 'backup' vtype",
+ storeid => $DEFAULT_STOREID,
+ scfg => {
+ type => 'dir',
+ path => $storage_path,
+ shared => 0,
+ content => {
+ backup => 1,
+ },
+ 'content-dirs' => {
+ backup => $file_name,
+ },
+ },
+ vmid => 19253,
+ vtypes => ['backup'],
+ cases => [
+ {
+ file => "$storage_path/$file_name/vzdump-lxc-19253-2020_02_03-19_57_43.tar.gz",
+ expected => {
+ content => 'backup',
+ ctime => 1580759863,
+ format => 'tar.gz',
+ size => $DEFAULT_SIZE,
+ subtype => 'lxc',
+ vmid => '19253',
+ volid => 'local:backup/vzdump-lxc-19253-2020_02_03-19_57_43.tar.gz',
+ },
+ },
+ ],
+ };
+
+ push($test_param_list->@*, $backup_vmid_test_params);
+}
+
+{
+ my $file_name = "vzdump-qemu-16110-2020_03_30-21_13_55.vma";
+ my $storage_path = File::Temp->newdir() . '/' . $file_name;
+
+ my $backup_vmid_test_params = {
+ description =>
+ "VMID: none, file name in storage path and subdir of 'backup' vtype, no backups",
+ storeid => $DEFAULT_STOREID,
+ scfg => {
+ type => 'dir',
+ path => $storage_path,
+ shared => 0,
+ content => {
+ backup => 1,
+ snippets => 1,
+ },
+ 'content-dirs' => {
+ backup => $file_name,
+ },
+ },
+ vmid => 19253,
+ vtypes => ['backup'],
+ cases => [
+ {
+ file => "$storage_path/snippets/hookscript.pl",
+ expected => undef,
+ },
+ ],
+ };
+
+ push($test_param_list->@*, $backup_vmid_test_params);
+}
+
# provide static vmlist for tests
my $mock_cluster = Test::MockModule->new('PVE::Cluster', no_auto => 1);
$mock_cluster->redefine(get_vmlist => sub { return $mocked_vmlist; });
--
2.47.3