[PATCH pve-storage v1 28/54] tree-wide: replace remaining usages of regexes for 'import' vtype

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

Implement the remaining pieces that were missing from fully supporting
the 'import' volume type in `PVE::Storage::Common::Parse`. In
particular, parsing volume names for '.ova' files that refer to
contents inside the '.ova' file is now fully supported.

Also add corresponding test cases to 'parser_tests.pl'.

Replace the remaining usages of the `PVE::Storage::IMPORT_EXT_RE_1`
and `PVE::Storage::OVA_CONTENT_RE_1` with parsing functions from
`::Common::Parse` across the repository.

Note that the logic in `PVE::Storage::Plugin::parse_volname` can now
be simplified a little further, since the `parse_volname_as_parts()`
parsing function also handles extracting the inner and outer file
extensions, amongst other parts.

Since the replaced regexes are now completely unused inside
`PVE::Storage` and family, they should be phased out with a future
APIVER + APIAGE bump, like the regexes in previous commits.

Finally, update the test cases in 'guest_import_test.pl' that match
for specific parsing-related exception messages.

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/GuestImport.pm | 28 +-
src/PVE/Storage/Common/Parse.pm | 45 ++++
src/PVE/Storage/Common/test/parser_tests.pl | 281 ++++++++++++++++++++
src/PVE/Storage/Plugin.pm | 20 +-
src/test/guest_import_test.pl | 6 +-
5 files changed, 350 insertions(+), 30 deletions(-)

diff --git a/src/PVE/GuestImport.pm b/src/PVE/GuestImport.pm
index 3d59dcd7..3be84fc9 100644
--- a/src/PVE/GuestImport.pm
+++ b/src/PVE/GuestImport.pm
@@ -6,6 +6,9 @@ use warnings;
use File::Path;

use PVE::Storage;
+use PVE::Storage::Common::Parse qw(
+ parse_volname_as_parts
+);
use PVE::Tools qw(run_command);

sub extract_disk_from_import_file {
@@ -13,31 +16,26 @@ sub extract_disk_from_import_file {

my ($source_storeid, $volname) = PVE::Storage::parse_volume_id($volid);
$target_storeid //= $source_storeid;
- my $cfg = PVE::Storage::config();

- my ($vtype, $name, undef, undef, undef, undef, $fmt) =
- PVE::Storage::parse_volname($cfg, $volid);
+ my $parts = parse_volname_as_parts($volname);
+ die "cannot extract $volid - invalid volname $volname\n"
+ if !defined($parts);
+
+ my ($vtype, $outer_fmt) = $parts->@{qw(vtype ext)};

die "only files with content type 'import' can be extracted\n"
if $vtype ne 'import';

die "only files from 'ova' format can be extracted\n"
- if $fmt !~ m/^ova\+/;
+ if $outer_fmt ne 'ova';

- # extract the inner file from the name
- my $archive_volid;
- my $inner_file;
- my $inner_fmt;
- if ($name =~ m!^(.*\.ova)/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+)$!) {
- $archive_volid = "$source_storeid:import/$1";
- $inner_file = $2;
- ($inner_fmt) = $fmt =~ /^ova\+(.*)$/;
- } else {
- die "cannot extract $volid - invalid volname $volname\n";
- }
+ my $archive_volid = $source_storeid . ':' . $vtype . '/' . $parts->{'disk-path'};
+ my $inner_file = $parts->{'content-file'};
+ my $inner_fmt = $parts->{'content-ext'};

die "cannot determine format of '$volid'\n" if !$inner_fmt;

+ my $cfg = PVE::Storage::config();
my $ova_path = PVE::Storage::path($cfg, $archive_volid);

my $tmpdir = PVE::Storage::get_image_dir($cfg, $target_storeid, $vmid);
diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm
index bd7cec13..9fb45b0c 100644
--- a/src/PVE/Storage/Common/Parse.pm
+++ b/src/PVE/Storage/Common/Parse.pm
@@ -39,6 +39,8 @@ my @BACKUP_COMPRESSION_EXTENSIONS = ('gz', 'lzo', 'zst', 'bz2');

my @IMPORT_EXTENSIONS = ('ova', 'ovf', 'qcow2', 'raw', 'vmdk');

+my @OVA_CONTENT_EXTENSIONS = ('qcow2', 'raw', 'vmdk');
+
my sub join_to_re_alternations(@list) {
return join('|', map { quotemeta } @list);
}
@@ -49,8 +51,12 @@ my $RE_BACKUP_COMPRESSION_EXTENSIONS = join_to_re_alternations(@BACKUP_COMPRESSI

my $RE_IMPORT_EXTENSIONS = join_to_re_alternations(@IMPORT_EXTENSIONS);

+my $RE_OVA_CONTENT_EXTENSIONS = join_to_re_alternations(@OVA_CONTENT_EXTENSIONS);
+
my $RE_SAFE_CHAR_CLASS = qr/[a-zA-Z0-9\-\.\+\=\_]/;

+my $RE_SAFE_CHAR_WITH_WHITESPACE_CLASS = qr/[ a-zA-Z0-9\-\.\+\=\_]/;
+
my $RE_PARENT_DIR = quotemeta('..');
my $RE_CONTAINS_PARENT_DIR = qr!
( ^$RE_PARENT_DIR/ ) # ../ --> Beginning of path
@@ -124,6 +130,42 @@ my $RE_IMPORT_FILE_PATH = qr!
)
!xn;

+my $RE_OVA_CONTENT = qr!
+ (?<content_file>
+ ($RE_SAFE_CHAR_WITH_WHITESPACE_CLASS)+ \. (?<content_ext> $RE_OVA_CONTENT_EXTENSIONS)
+ )
+!xn;
+
+# NOTE: Volume names with the 'import' vtype are treated differently when
+# they do not stem from a file path directly - see comments inline.
+my $RE_IMPORT_VOLNAME_OVA_FILE_WITH_CONTENT = qr!
+ (?<file>
+ # NOTE: Unlike in RE_IMPORT_FILE_PATH, we allow whitespace here
+ ($RE_SAFE_CHAR_WITH_WHITESPACE_CLASS)+
+ \.
+ (?<ext> ova)
+ ) / (?<content> $RE_OVA_CONTENT)
+!xn;
+
+my $RE_IMPORT_VOLNAME_REGULAR_FILE = qr!
+ (?<file>
+ # NOTE: Unlike in RE_IMPORT_FILE_PATH, we allow whitespace here
+ ($RE_SAFE_CHAR_WITH_WHITESPACE_CLASS)+
+ \.
+ (?<ext> $RE_IMPORT_EXTENSIONS)
+ )
+!xn;
+
+my $RE_IMPORT_VOLNAME = qr!
+ (?<path>
+ # NOTE: Order here matters - the ova+content regex is stricter,
+ # so try this branch here first
+ ($RE_IMPORT_VOLNAME_OVA_FILE_WITH_CONTENT)
+ |
+ ($RE_IMPORT_VOLNAME_REGULAR_FILE)
+ )
+!xn;
+
my $RE_FILE_PATH_FOR_VTYPE = {
iso => qr/^$RE_ISO_FILE_PATH$/,
vztmpl => qr/^$RE_VZTMPL_FILE_PATH$/,
@@ -137,6 +179,9 @@ my $RE_VOLNAME_FOR_VTYPE = {
vztmpl => qr/^$RE_VZTMPL_FILE_PATH$/,
backup => qr/^$RE_BACKUP_FILE_PATH$/,
snippets => qr/^$RE_SNIPPETS_FILE_PATH$/,
+
+ # special cases - not reusing file path regexes:
+ import => qr/^$RE_IMPORT_VOLNAME$/,
};

my sub contains_parent_dir($path) {
diff --git a/src/PVE/Storage/Common/test/parser_tests.pl b/src/PVE/Storage/Common/test/parser_tests.pl
index e0029fc6..b6e42307 100755
--- a/src/PVE/Storage/Common/test/parser_tests.pl
+++ b/src/PVE/Storage/Common/test/parser_tests.pl
@@ -396,17 +396,104 @@ my $volname_cases_snippets_valid = [
},
];

+my $volname_cases_import_valid = [
+ {
+ path => 'import.ova',
+ expected => {
+ file => 'import.ova',
+ ext => 'ova',
+ 'disk-path' => 'import.ova',
+ path => 'import.ova',
+ vtype => 'import',
+ volname => 'import/import.ova',
+ },
+ },
+ {
+ path => 'import.ovf',
+ expected => {
+ file => 'import.ovf',
+ ext => 'ovf',
+ 'disk-path' => 'import.ovf',
+ path => 'import.ovf',
+ vtype => 'import',
+ volname => 'import/import.ovf',
+ },
+ },
+ {
+ path => 'disk-0.qcow2',
+ expected => {
+ file => 'disk-0.qcow2',
+ ext => 'qcow2',
+ 'disk-path' => 'disk-0.qcow2',
+ path => 'disk-0.qcow2',
+ vtype => 'import',
+ volname => 'import/disk-0.qcow2',
+ },
+ },
+ {
+ path => 'raw_disk_v2.5.raw',
+ expected => {
+ file => 'raw_disk_v2.5.raw',
+ ext => 'raw',
+ 'disk-path' => 'raw_disk_v2.5.raw',
+ path => 'raw_disk_v2.5.raw',
+ vtype => 'import',
+ volname => 'import/raw_disk_v2.5.raw',
+ },
+ },
+ {
+ path => 'disk-1+data.vmdk',
+ expected => {
+ file => 'disk-1+data.vmdk',
+ ext => 'vmdk',
+ 'disk-path' => 'disk-1+data.vmdk',
+ path => 'disk-1+data.vmdk',
+ vtype => 'import',
+ volname => 'import/disk-1+data.vmdk',
+ },
+ },
+];
+
+my $volname_cases_import_invalid = [
+ {
+ description => "Invalid file extension (import)",
+ args => {
+ path => 'import.zip',
+ vtype => 'import',
+ },
+ expected => undef,
+ },
+ {
+ description => "Uppercase letter in file extension (import)",
+ args => {
+ path => 'import.Ova',
+ vtype => 'import',
+ },
+ expected => undef,
+ },
+ {
+ description => "Unsafe characters in file name (import)",
+ args => {
+ path => '🐪perl-playground🐪.ova',
+ vtype => 'import',
+ },
+ expected => undef,
+ },
+];
+
my $cases_valid_all = [
$volname_cases_iso_valid,
$volname_cases_vztmpl_valid,
$volname_cases_backup_valid,
$volname_cases_snippets_valid,
+ $volname_cases_import_valid,
];

my $cases_invalid_all = [
$volname_cases_iso_invalid,
$volname_cases_vztmpl_invalid,
$volname_cases_backup_invalid,
+ $volname_cases_import_invalid,
];

{
@@ -474,6 +561,178 @@ my sub run_volname_parsing_tests : prototype($) ($volname_tests) {
return;
}

+# NOTE: These run for parse_path_as_volname_parts() and parse_path_as_volname() only!
+my $cases_import_special_file_path = [
+ {
+ description => "Whitespace not allowed in volume file paths (import)",
+ args => {
+ path => 'Some Disk.qcow2',
+ vtype => 'import',
+ },
+ expected => undef,
+ },
+ {
+ description => "Volume file path with disallowed OVA content (raw) (import)",
+ args => {
+ path => 'import.ova/disk.raw',
+ vtype => 'import',
+ },
+ expected => undef,
+ },
+ {
+ description => "Volume file path with disallowed OVA content (qcow2) (import)",
+ args => {
+ path => 'import.ova/disk.qcow2',
+ vtype => 'import',
+ },
+ expected => undef,
+ },
+ {
+ description => "Volume file path with disallowed OVA content (vmdk) (import)",
+ args => {
+ path => 'import.ova/disk.vmdk',
+ vtype => 'import',
+ },
+ expected => undef,
+ },
+];
+
+# NOTE: These run for parse_volname_as_parts() only!
+my $cases_import_special_volname = [
+ {
+ description => "Whitespace allowed in volume names (import)",
+ args => {
+ volname => 'import/Some Disk.qcow2',
+ },
+ expected => {
+ file => 'Some Disk.qcow2',
+ ext => 'qcow2',
+ 'disk-path' => 'Some Disk.qcow2',
+ path => 'Some Disk.qcow2',
+ vtype => 'import',
+ volname => 'import/Some Disk.qcow2',
+ },
+ },
+ {
+ description => "Volume name with OVA content (raw) (import)",
+ args => {
+ volname => 'import/import.ova/disk.raw',
+ },
+ expected => {
+ file => 'import.ova',
+ ext => 'ova',
+ 'disk-path' => 'import.ova',
+ path => 'import.ova/disk.raw',
+ content => 'disk.raw',
+ 'content-file' => 'disk.raw',
+ 'content-ext' => 'raw',
+ vtype => 'import',
+ volname => 'import/import.ova/disk.raw',
+ },
+ },
+ {
+ description => "Volume name with OVA content (qcow2) (import)",
+ args => {
+ volname => 'import/import.ova/disk.qcow2',
+ },
+ expected => {
+ file => 'import.ova',
+ ext => 'ova',
+ 'disk-path' => 'import.ova',
+ path => 'import.ova/disk.qcow2',
+ content => 'disk.qcow2',
+ 'content-file' => 'disk.qcow2',
+ 'content-ext' => 'qcow2',
+ vtype => 'import',
+ volname => 'import/import.ova/disk.qcow2',
+ },
+ },
+ {
+ description => "Volume name with OVA content (vmdk) (import)",
+ args => {
+ volname => 'import/import.ova/disk.vmdk',
+ },
+ expected => {
+ file => 'import.ova',
+ ext => 'ova',
+ 'disk-path' => 'import.ova',
+ path => 'import.ova/disk.vmdk',
+ content => 'disk.vmdk',
+ 'content-file' => 'disk.vmdk',
+ 'content-ext' => 'vmdk',
+ vtype => 'import',
+ volname => 'import/import.ova/disk.vmdk',
+ },
+ },
+ {
+ description => "Whitespace in volume name + OVA content (import)",
+ args => {
+ volname => 'import/Some Import.ova/disk.qcow2',
+ },
+ expected => {
+ file => 'Some Import.ova',
+ ext => 'ova',
+ 'disk-path' => 'Some Import.ova',
+ path => 'Some Import.ova/disk.qcow2',
+ content => 'disk.qcow2',
+ 'content-file' => 'disk.qcow2',
+ 'content-ext' => 'qcow2',
+ vtype => 'import',
+ volname => 'import/Some Import.ova/disk.qcow2',
+ },
+ },
+];
+
+my sub run_special_import_volname_parsing_tests : prototype() () {
+ for my $case ($cases_import_special_file_path->@*) {
+ subtest $case->{description} => sub () {
+ my ($path, $vtype) = $case->{args}->@{qw(path vtype)};
+
+ my $got_volname_parts = parse_path_as_volname_parts($path, $vtype);
+ my $got_volname = parse_path_as_volname($path, $vtype);
+
+ if (defined($case->{expected})) {
+ eq_or_diff(
+ $got_volname_parts,
+ $case->{expected},
+ 'parse_path_as_volname_parts() returns expected hashref',
+ { context => 50000 },
+ );
+
+ is(
+ $got_volname,
+ $case->{expected}->{volname},
+ 'parse_path_as_volname() returns expected volname',
+ );
+ } else {
+ is($got_volname_parts, undef, 'parse_path_as_volname_parts() returns undef');
+ is($got_volname, undef, 'parse_path_as_volname() returns undef');
+ }
+ };
+ }
+
+ for my $case ($cases_import_special_volname->@*) {
+ subtest $case->{description} => sub () {
+ my ($volname) = $case->{args}->@{qw(volname)};
+
+ my $got_volname = parse_volname_as_parts($volname);
+
+ if (defined($case->{expected})) {
+ eq_or_diff(
+ $got_volname,
+ $case->{expected},
+ 'parse_volname_as_parts() returns expected hashref',
+ { context => 50000 },
+ );
+ } else {
+ is($got_volname, undef, 'parse_volname_as_parts() returns undef');
+ }
+ };
+ }
+
+ return;
+}
+
my $DEFAULT_STOREID = 'local';
my $DEFAULT_STORAGE_PATH = File::Temp->newdir();
my $DEFAULT_SCFG = {
@@ -546,6 +805,26 @@ my $volid_tests = [
$volname_tests->@*
],
},
+ {
+ description =>
+ "volid parsers build on volname parsers' behaviors - 'import' vtype file paths (1)",
+ storedid => $DEFAULT_STOREID,
+ scfg => $DEFAULT_SCFG,
+ cases => [
+ map { format_volname_case_to_volid_case($DEFAULT_STOREID, $DEFAULT_SCFG, $_) }
+ $cases_import_special_file_path->@*
+ ],
+ },
+ {
+ description =>
+ "volid parsers build on volname parsers' behaviors - 'import' vtype file paths (2)",
+ storedid => $ALT_STOREID,
+ scfg => $ALT_SCFG,
+ cases => [
+ map { format_volname_case_to_volid_case($DEFAULT_STOREID, $DEFAULT_SCFG, $_) }
+ $cases_import_special_file_path->@*
+ ],
+ },
];

my sub run_volid_parsing_tests : prototype($) ($volid_tests) {
@@ -603,6 +882,8 @@ my sub main() {
unified_diff();

run_volname_parsing_tests($volname_tests);
+ run_special_import_volname_parsing_tests();
+
run_volid_parsing_tests($volid_tests);

done_testing();
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 39e5cb0d..a3cb1343 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -839,20 +839,16 @@ sub parse_volname {
if ($vtype eq 'snippets') {
return ($vtype, $volume_path, undef, undef, undef, undef, 'raw');
}
- }

- if ($volname =~
- m!^import/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+\.ova\/${PVE::Storage::OVA_CONTENT_RE_1})$!
- ) {
- my $packed_image = $1;
- my $format = $2;
- return ('import', $packed_image, undef, undef, undef, undef, "ova+$format");
- }
+ if ($vtype eq 'import') {
+ my $format = $parts->{ext};

- if ($volname =~
- m!^import/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!
- ) {
- return ('import', $1, undef, undef, undef, undef, $2);
+ if (defined(my $content_ext = $parts->{'content-ext'})) {
+ $format .= "+$content_ext";
+ }
+
+ return ($vtype, $volume_path, undef, undef, undef, undef, $format);
+ }
}

die "unable to parse directory volume name '$volname'\n";
diff --git a/src/test/guest_import_test.pl b/src/test/guest_import_test.pl
index 04eec24d..234013e3 100755
--- a/src/test/guest_import_test.pl
+++ b/src/test/guest_import_test.pl
@@ -357,7 +357,7 @@ my $tests = [
{
volname => 'import/some-import.ova/disk.txt',
vmid => 1337,
- 'expect-fail' => qr/unable to parse directory volume name/,
+ 'expect-fail' => qr/invalid volname/,
},
],
},
@@ -532,7 +532,7 @@ my $tests = [
{
volname => 'import/some-🐧-import.ova/disk.qcow2',
vmid => 1337,
- 'expect-fail' => qr/unable to parse directory volume name/,
+ 'expect-fail' => qr/invalid volname/,
},
],
},
@@ -619,7 +619,7 @@ my $tests = [
{
volname => 'import/some-import.ova',
vmid => 1337,
- 'expect-fail' => qr/only files from 'ova' format can be extracted/,
+ 'expect-fail' => qr/cannot determine format of/,
},
],
},
--
2.47.3