[pve-devel] [PATCH v10 storage 1/3] factoring out regex'es for backup and vztmpl

[Lorenz Stechauner]

uniformly stores these regex definitions in PVE::Storage.

One test had to be adapted because it tested obsolete code. Namely:
it expects vztmpl to only end with .tar.gz, but the new regex also
includes .tar.xz, there is nothing against allowing .tar.xz files as
vztmpl files.

Signed-off-by: Lorenz Stechauner <l.stechauner at proxmox.com>
---
 PVE/API2/Storage/Status.pm     |  6 +++---
 PVE/Storage.pm                 | 11 ++++++++---
 PVE/Storage/Plugin.pm          | 15 +++++++++------
 test/path_to_volume_id_test.pm |  7 ++++---
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index 8a36aef..7069244 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -423,12 +423,12 @@ __PACKAGE__->register_method ({
 
 	if ($content eq 'iso') {
 	    if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
-		raise_param_exc({ filename => "missing '.iso' or '.img' extension" });
+		raise_param_exc({ filename => "wrong file extension" });
 	    }
 	    $path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
 	} elsif ($content eq 'vztmpl') {
-	    if ($filename !~ m![^/]+\.tar\.[gx]z$!) {
-		raise_param_exc({ filename => "missing '.tar.gz' or '.tar.xz' extension" });
+	    if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
+		raise_param_exc({ filename => "wrong file extension" });
 	    }
 	    $path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
 	} else {
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index ea887a4..519431c 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -101,6 +101,11 @@ PVE::Storage::Plugin->init();
 
 our $iso_extension_re = qr/\.(?:iso|img)/i;
 
+our $vztmpl_extension_re = qr/\.tar\.([gx]z)/i;
+
+# Caution: because of the '$' inside, this regex may only used to match at the end of strings
+our $backup_extension_re = qr/\.(tgz$|tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?/i;
+
 #  PVE::Storage utility functions
 
 sub config {
@@ -573,13 +578,13 @@ sub path_to_volume_id {
 	} elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
 	    my $name = $1;
 	    return ('iso', "$sid:iso/$name");
-	} elsif ($path =~ m!^$tmpldir/([^/]+\.tar\.gz)$!) {
+	} elsif ($path =~ m!^$tmpldir/([^/]+$vztmpl_extension_re)$!) {
 	    my $name = $1;
 	    return ('vztmpl', "$sid:vztmpl/$name");
 	} elsif ($path =~ m!^$privatedir/(\d+)$!) {
 	    my $vmid = $1;
 	    return ('rootdir', "$sid:rootdir/$vmid");
-	} elsif ($path =~ m!^$backupdir/([^/]+\.(?:tgz|(?:(?:tar|vma)(?:\.(?:${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)))$!) {
+	} elsif ($path =~ m!^$backupdir/([^/]+$backup_extension_re)$!) {
 	    my $name = $1;
 	    return ('backup', "$sid:backup/$name");
 	} elsif ($path =~ m!^$snippetsdir/([^/]+)$!) {
@@ -1463,7 +1468,7 @@ sub archive_info {
     my $info;
 
     my $volid = basename($archive);
-    if ($volid =~ /^(vzdump-(lxc|openvz|qemu)-.+\.(tgz$|tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)$/) {
+    if ($volid =~ /^(vzdump-(lxc|openvz|qemu)-.+$backup_extension_re)$/) {
 	my $filename = "$1"; # untaint
 	my ($type, $format, $comp) = ($2, $3, $4);
 	my $format_re = defined($comp) ? "$format.$comp" : "$format";
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index d330845..afb05ed 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -518,11 +518,11 @@ sub parse_volname {
 	return ('images', $name, $vmid, undef, undef, $isBase, $format);
     } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
 	return ('iso', $1);
-    } elsif ($volname =~ m!^vztmpl/([^/]+\.tar\.[gx]z)$!) {
+    } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::vztmpl_extension_re)$!) {
 	return ('vztmpl', $1);
     } elsif ($volname =~ m!^rootdir/(\d+)$!) {
 	return ('rootdir', $1, $1);
-    } elsif ($volname =~ m!^backup/([^/]+(?:\.(?:tgz|(?:(?:tar|vma)(?:\.(?:${\COMPRESSOR_RE}))?))))$!) {
+    } elsif ($volname =~ m!^backup/([^/]+$PVE::Storage::backup_extension_re)$!) {
 	my $fn = $1;
 	if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) {
 	    return ('backup', $fn, $2);
@@ -1041,15 +1041,18 @@ my $get_subdir_files = sub {
 	    $info = { volid => "$sid:iso/$1", format => 'iso' };
 
 	} elsif ($tt eq 'vztmpl') {
-	    next if $fn !~ m!/([^/]+\.tar\.([gx]z))$!;
+	    next if $fn !~ m!/([^/]+$PVE::Storage::vztmpl_extension_re)$!;
 
 	    $info = { volid => "$sid:vztmpl/$1", format => "t$2" };
 
 	} elsif ($tt eq 'backup') {
-	    next if $fn !~ m!/([^/]+\.(tgz|(?:(?:tar|vma)(?:\.(${\COMPRESSOR_RE}))?)))$!;
+	    next if $fn !~ m!/([^/]+$PVE::Storage::backup_extension_re)$!;
+
 	    my $original = $fn;
-	    my $format = $2;
-	    $fn = $1;
+	    my ($format, $comp);
+
+	    ($fn, $format, $comp) = ($1, $2, $3);
+	    $format = defined($comp) ? "$format.$comp" : $format;
 
 	    # only match for VMID now, to avoid false positives (VMID in parent directory name)
 	    next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
diff --git a/test/path_to_volume_id_test.pm b/test/path_to_volume_id_test.pm
index 5eee2f6..e0c46d9 100644
--- a/test/path_to_volume_id_test.pm
+++ b/test/path_to_volume_id_test.pm
@@ -166,12 +166,13 @@ my @tests = (
 	    'local:snippets/hookscript.pl',
 	],
     },
-
-    # no matches
     {
 	description => 'CT template, tar.xz',
 	volname     => "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.xz",
-	expected    => [''],
+	expected    => [
+	    'vztmpl',
+	    'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.xz'
+	],
     },
 
     # no matches, path or files with failures
-- 
2.30.2