[PATCH pve-storage v2 25/50] tree-wide: replace usages of BACKUP_EXT_RE_2 with parsing functions

["Max R. Carrara" <m.carrara@proxmox.com>]

Add support for the 'backup' volume type in
`PVE::Storage::Common::Parse`.

Also add corresponding test cases.

Use the module's parsing functions to replace usages of the
`PVE::Storage::BACKUP_EXT_RE_2` regex across the repository.

Note that the new regex for parsing 'backup' vtype file paths is also
able to optionally extract the VMID and the type of guest from the
file name. This makes it possible to further simplify some parts of
the original logic.

As done with the `ISO_EXT_RE_0` and `VZTMPL_EXT_RE_1` regexes, note
the removal of the `BACKUP_EXT_RE_2` regex in `ApiChangeLog`, since
it's now completely unused. Add a FIXME to note that it should be
removed on the next APIAGE reset.

Additionally, the `COMPRESSOR_RE` constant in `PVE::Storage::Plugin`
is now unused. Together with its related `KNOWN_COMPRESSION_FORMATS`
constant, it should be phased out and replaced eventually. Add a TODO
comment for that.

In the meantime, keep track of the compression formats for specific
vtypes in the `::Common::Parse` module until we move them to a more
adequate place.

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
 ApiChangeLog                                |   1 +
 src/PVE/Storage.pm                          |   5 +-
 src/PVE/Storage/Common/Parse.pm             |  63 +++++++
 src/PVE/Storage/Common/test/parser_tests.pl | 181 ++++++++++++++++++++
 src/PVE/Storage/Plugin.pm                   |  42 +++--
 5 files changed, 271 insertions(+), 21 deletions(-)

diff --git a/ApiChangeLog b/ApiChangeLog
index ffe3725c..dc77bf41 100644
--- a/ApiChangeLog
+++ b/ApiChangeLog
@@ -31,6 +31,7 @@ Future changes should be documented in here.
 * Remove the following regular expressions:
   * `$PVE::Storage::ISO_EXT_RE_0` (`iso` volume type)
   * `$PVE::Storage::VZTMPL_EXT_RE_1` (`vztmpl` volume type)
+  * `$PVE::Storage::BACKUP_EXT_RE_2` (`backup` volume type)
 
   These regular expressions were mostly used for parsing file paths and volume
   names corresponding to their volume types, noted in parentheses above.
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index afe29178..9b12ed8d 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -126,6 +126,7 @@ our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i;
 # FIXME: remove this regex on the next APIAGE reset.
 our $VZTMPL_EXT_RE_1 = qr/\.(?|(tar)(?!\.)|tar\.(gz|xz|zst|bz2))/i;
 
+# FIXME: remove this regex on the next APIAGE reset.
 our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/;
 
 our $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/;
@@ -767,9 +768,7 @@ sub path_to_volume_id {
         }
 
         if ($vtype eq 'backup') {
-            return if $filename !~ m!/([^/]+$BACKUP_EXT_RE_2)$!;
-            my $name = $1;
-            return "$sid:backup/$name";
+            return parse_abs_path_as_volid($sid, $scfg, $path, $vtype);
         }
 
         if ($vtype eq 'snippets') {
diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm
index 207f287b..d3667117 100644
--- a/src/PVE/Storage/Common/Parse.pm
+++ b/src/PVE/Storage/Common/Parse.pm
@@ -35,12 +35,16 @@ primarily related to.
 
 my @VZTMPL_COMPRESSION_EXTENSIONS = ('gz', 'xz', 'zst', 'bz2');
 
+my @BACKUP_COMPRESSION_EXTENSIONS = ('gz', 'lzo', 'zst', 'bz2');
+
 my sub join_to_re_alternations(@list) {
     return join('|', map { quotemeta } @list);
 }
 
 my $RE_VZTMPL_COMPRESSION_EXTENSIONS = join_to_re_alternations(@VZTMPL_COMPRESSION_EXTENSIONS);
 
+my $RE_BACKUP_COMPRESSION_EXTENSIONS = join_to_re_alternations(@BACKUP_COMPRESSION_EXTENSIONS);
+
 my $RE_PARENT_DIR = quotemeta('..');
 my $RE_CONTAINS_PARENT_DIR = qr!
     ( ^$RE_PARENT_DIR/ )  #  ../ --> Beginning of path
@@ -50,6 +54,11 @@ my $RE_CONTAINS_PARENT_DIR = qr!
     ( /$RE_PARENT_DIR$ )  # /..  --> End of path
 !xn;
 
+# A vmid is an integer between 100 and 999_999_999
+# See: https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/JSONSchema.pm;h=17e7126a6c42f8326a1da890680af10b6106d906;hb=refs/heads/master#l62
+# See also: https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/JSONSchema.pm;h=17e7126a6c42f8326a1da890680af10b6106d906;hb=refs/heads/master#l304
+my $RE_VMID = qr![1-9][0-9]{2,8}!;
+
 my $RE_ISO_FILE_PATH = qr!
     (?<path>
         (?<file> [^/]+ \. (?<ext> (?i: iso|img) ) )
@@ -69,14 +78,44 @@ my $RE_VZTMPL_FILE_PATH = qr!
     )
 !xn;
 
+my $RE_GUEST_VZDUMP_BACKUP_FILE_NAME = qr!
+    vzdump
+    - (?<type> openvz|lxc|qemu)
+    - (?<vmid> $RE_VMID)
+    - [^/]+
+!xn;
+
+my $RE_BACKUP_FILE_PATH = qr!
+    (?<path>
+        (?<file>
+            (
+                ($RE_GUEST_VZDUMP_BACKUP_FILE_NAME)
+                |
+                ([^/]+)
+            )
+            \.
+            (?<ext>
+                (?<ext_archive> tgz)
+                |
+                (
+                    (?<ext_archive> tar|vma)
+                    ( \. (?<ext_compression> $RE_BACKUP_COMPRESSION_EXTENSIONS) )?
+                )
+            )
+        )
+    )
+!xn;
+
 my $RE_FILE_PATH_FOR_VTYPE = {
     iso => qr/^$RE_ISO_FILE_PATH$/,
     vztmpl => qr/^$RE_VZTMPL_FILE_PATH$/,
+    backup => qr/^$RE_BACKUP_FILE_PATH$/,
 };
 
 my $RE_VOLNAME_FOR_VTYPE = {
     iso => qr/^$RE_ISO_FILE_PATH$/,
     vztmpl => qr/^$RE_VZTMPL_FILE_PATH$/,
+    backup => qr/^$RE_BACKUP_FILE_PATH$/,
 };
 
 my sub contains_parent_dir($path) {
@@ -178,6 +217,30 @@ C<ext-archive>, C<ext-compression> (optional):
         volname => 'vztmpl/archlinux-base_20190924-1_amd64.tar.gz',
     }
 
+=item C<backup>
+
+C<type> (optional), C<vmid> (optional), which are included in backups created
+by Proxmox VE, as well as C<ext-archive>, C<ext-compression> (optional):
+
+    {
+        file => 'vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+
+        # Used to further differentiate 'file'
+        type => 'qemu',
+        vmid => '16110',
+
+        ext => 'vma.lzo',
+
+        # Used to further differentiate 'ext'
+        'ext-archive' => 'vma',
+        'ext-compression' => 'lzo',
+
+        'disk-path' => 'vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+        path => 'vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+        vtype => 'backup',
+        volname => 'backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+    }
+
 =back
 
 =cut
diff --git a/src/PVE/Storage/Common/test/parser_tests.pl b/src/PVE/Storage/Common/test/parser_tests.pl
index 1ea8df1a..979758b5 100755
--- a/src/PVE/Storage/Common/test/parser_tests.pl
+++ b/src/PVE/Storage/Common/test/parser_tests.pl
@@ -193,14 +193,195 @@ my $volname_cases_vztmpl_invalid = [
     },
 ];
 
+my $volname_cases_backup_valid = [
+    {
+        path => 'vzdump-qemu-16110-2020_03_30-21_13_55.vma',
+        expected => {
+            type => 'qemu',
+            vmid => '16110',
+            file => 'vzdump-qemu-16110-2020_03_30-21_13_55.vma',
+            ext => 'vma',
+            'ext-archive' => 'vma',
+            'disk-path' => 'vzdump-qemu-16110-2020_03_30-21_13_55.vma',
+            path => 'vzdump-qemu-16110-2020_03_30-21_13_55.vma',
+            vtype => 'backup',
+            volname => 'backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma',
+        },
+    },
+    {
+        path => 'vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz',
+        expected => {
+            type => 'qemu',
+            vmid => '16110',
+            file => 'vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz',
+            ext => 'vma.gz',
+            'ext-archive' => 'vma',
+            'ext-compression' => 'gz',
+            'disk-path' => 'vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz',
+            path => 'vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz',
+            vtype => 'backup',
+            volname => 'backup/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz',
+        },
+    },
+    {
+        path => 'vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+        expected => {
+            type => 'qemu',
+            vmid => '16110',
+            file => 'vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+            ext => 'vma.lzo',
+            'ext-archive' => 'vma',
+            'ext-compression' => 'lzo',
+            'disk-path' => 'vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+            path => 'vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+            vtype => 'backup',
+            volname => 'backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
+        },
+    },
+    {
+        path => 'vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst',
+        expected => {
+            type => 'qemu',
+            vmid => '16110',
+            file => 'vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst',
+            ext => 'vma.zst',
+            'ext-archive' => 'vma',
+            'ext-compression' => 'zst',
+            'disk-path' => 'vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst',
+            path => 'vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst',
+            vtype => 'backup',
+            volname => 'backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst',
+        },
+    },
+    {
+        path => 'vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo',
+        expected => {
+            type => 'lxc',
+            vmid => '16112',
+            file => 'vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo',
+            ext => 'tar.lzo',
+            'ext-archive' => 'tar',
+            'ext-compression' => 'lzo',
+            'disk-path' => 'vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo',
+            path => 'vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo',
+            vtype => 'backup',
+            volname => 'backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo',
+        },
+    },
+    {
+        path => 'vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz',
+        expected => {
+            type => 'lxc',
+            vmid => '16112',
+            file => 'vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz',
+            ext => 'tar.gz',
+            'ext-archive' => 'tar',
+            'ext-compression' => 'gz',
+            'disk-path' => 'vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz',
+            path => 'vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz',
+            vtype => 'backup',
+            volname => 'backup/vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz',
+        },
+    },
+    {
+        path => 'vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst',
+        expected => {
+            type => 'lxc',
+            vmid => '16112',
+            file => 'vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst',
+            ext => 'tar.zst',
+            'ext-archive' => 'tar',
+            'ext-compression' => 'zst',
+            'disk-path' => 'vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst',
+            path => 'vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst',
+            vtype => 'backup',
+            volname => 'backup/vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst',
+        },
+    },
+    {
+        path => 'vzdump-lxc-16112-2020_03_30-21_59_30.tgz',
+        expected => {
+            type => 'lxc',
+            vmid => '16112',
+            file => 'vzdump-lxc-16112-2020_03_30-21_59_30.tgz',
+            ext => 'tgz',
+            'ext-archive' => 'tgz',
+            'disk-path' => 'vzdump-lxc-16112-2020_03_30-21_59_30.tgz',
+            path => 'vzdump-lxc-16112-2020_03_30-21_59_30.tgz',
+            vtype => 'backup',
+            volname => 'backup/vzdump-lxc-16112-2020_03_30-21_59_30.tgz',
+        },
+    },
+    {
+        path => 'vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
+        expected => {
+            type => 'openvz',
+            vmid => '16112',
+            file => 'vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
+            ext => 'tar.bz2',
+            'ext-archive' => 'tar',
+            'ext-compression' => 'bz2',
+            'disk-path' => 'vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
+            path => 'vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
+            vtype => 'backup',
+            volname => 'backup/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
+        },
+    },
+];
+
+my $volname_cases_backup_invalid = [
+    {
+        description => "Invalid file extension (1) (backup)",
+        args => {
+            path => 'vzdump-openvz-16112-2020_03_30-21_39_30.tar.zip',
+            vtype => 'backup',
+        },
+        expected => undef,
+    },
+    {
+        description => "Invalid file extension (2) (backup)",
+        args => {
+            path => 'vzdump-openvz-16112-2020_03_30-21_39_30.zip.gz',
+            vtype => 'backup',
+        },
+        expected => undef,
+    },
+    {
+        description => "Invalid file extension (3) (backup)",
+        args => {
+            path => 'vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz',
+            vtype => 'backup',
+        },
+        expected => undef,
+    },
+    {
+        description => "Uppercase letter in file extension (1) (backup)",
+        args => {
+            path => 'vzdump-qemu-16110-2020_03_30-21_12_40.Tar.gz',
+            vtype => 'backup',
+        },
+        expected => undef,
+    },
+    {
+        description => "Uppercase letter in file extension (2) (backup)",
+        args => {
+            path => 'vzdump-qemu-16110-2020_03_30-21_12_40.tar.Gz',
+            vtype => 'backup',
+        },
+        expected => undef,
+    },
+];
+
 my $cases_valid_all = [
     $volname_cases_iso_valid,
     $volname_cases_vztmpl_valid,
+    $volname_cases_backup_valid,
 ];
 
 my $cases_invalid_all = [
     $volname_cases_iso_invalid,
     $volname_cases_vztmpl_invalid,
+    $volname_cases_backup_invalid,
 ];
 
 {
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index b539ef78..3f1c436c 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -28,7 +28,9 @@ use JSON;
 
 use base qw(PVE::SectionConfig);
 
+# TODO: Phase out these two constants
 use constant KNOWN_COMPRESSION_FORMATS => ('gz', 'lzo', 'zst', 'bz2');
+# FIXME: remove this regex on the next APIAGE reset.
 use constant COMPRESSOR_RE => join('|', KNOWN_COMPRESSION_FORMATS);
 
 use constant LOG_EXT => ".log";
@@ -835,14 +837,10 @@ sub parse_volname {
         if ($vtype eq 'vztmpl') {
             return ($vtype, $volume_path, undef, undef, undef, undef, 'raw');
         }
-    }
 
-    if ($volname =~ m!^backup/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!) {
-        my $fn = $1;
-        if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) {
-            return ('backup', $fn, $2, undef, undef, undef, 'raw');
+        if ($vtype eq 'backup') {
+            return ($vtype, $volume_path, $parts->{vmid}, undef, undef, undef, 'raw');
         }
-        return ('backup', $fn, undef, undef, undef, undef, 'raw');
     }
 
     if ($volname =~ m!^snippets/([^/]+)$!) {
@@ -1747,37 +1745,45 @@ my sub get_subdir_files {
         }
 
         if ($vtype eq 'backup') {
-            return if $filename !~ m!/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!;
+            my $parts = parse_abs_path_as_volid_parts($storeid, $scfg, $path, $vtype);
+            return if !defined($parts);
 
-            my $original = $path;
-            my $format = $2;
-            $filename = $1;
+            my $format = $parts->{ext};
+            my $volume_path = $parts->{path};
 
-            # only match for VMID now, to avoid false positives (VMID in parent directory name)
-            return if defined($vmid) && $filename !~ m/\S+-$vmid-\S+/;
+            # Check if parsed VMID matched provided VMID in order to avoid
+            # false positives (VMID in parent directory name)
+            my $parsed_vmid = $parts->{vmid};
+            if (defined($vmid)) {
+                if (defined($parsed_vmid)) {
+                    return if $vmid ne $parsed_vmid;
+                } else {
+                    return;
+                }
+            }
 
             my $info = {
-                volid => "$storeid:backup/$filename",
+                volid => $parts->{volid},
                 format => $format,
             };
 
-            my $archive_info = eval { PVE::Storage::archive_info($filename) } // {};
+            my $archive_info = eval { PVE::Storage::archive_info($volume_path) } // {};
 
             $info->{ctime} = $archive_info->{ctime} if defined($archive_info->{ctime});
             $info->{subtype} = $archive_info->{type} // 'unknown';
 
-            if (defined($vmid) || $filename =~ m!\-([1-9][0-9]{2,8})\-[^/]+\.${format}$!) {
-                $info->{vmid} = $vmid // $1;
+            if (defined($vmid) || defined($parsed_vmid)) {
+                $info->{vmid} = $vmid // $parsed_vmid;
             }
 
-            my $notes_filename = $original . NOTES_EXT;
+            my $notes_filename = $path . NOTES_EXT;
             if (-f $notes_filename) {
                 my $notes = PVE::Tools::file_read_firstline($notes_filename);
                 $info->{notes} = eval { decode('UTF-8', $notes, 1) } // $notes
                     if defined($notes);
             }
 
-            $info->{protected} = 1 if -e PVE::Storage::protection_file_path($original);
+            $info->{protected} = 1 if -e PVE::Storage::protection_file_path($path);
 
             return $info;
         }
-- 
2.47.3