public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Max R. Carrara" <m.carrara@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-storage v2 28/50] tree-wide: replace remaining usages of regexes for 'import' vtype
Date: Wed, 23 Sep 2026 17:05:42 +0200	[thread overview]
Message-ID: <20260923150606.531239-29-m.carrara@proxmox.com> (raw)
In-Reply-To: <20260923150606.531239-1-m.carrara@proxmox.com>

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. As before with other regexes,
mark their removal in `ApiChangeLog` and add corresponding FIXME
comments for a future APIAGE reset.

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 only remaining usage of the
`PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE` regex is now in
`PVE::GuestImport::OVF`, handle it the same way as the removed
`SAFE_CHAR_CLASS_RE`. This means that we move it into
`::Common::Parse`, make it private, and only expose it through a
helper subroutine. Also note this in `ApiChangeLog` and add a
corresponding FIXME comment once again.

This allows us to stop using it in `PVE::GuestImport::OVF` as well,
where we instead add a small helper sub to aid us in parsing the
obtained file path.

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>
---
 ApiChangeLog                                |   3 +
 src/PVE/GuestImport.pm                      |  28 +-
 src/PVE/GuestImport/OVF.pm                  |  24 +-
 src/PVE/Storage.pm                          |   7 +-
 src/PVE/Storage/Common/Parse.pm             |  81 ++++++
 src/PVE/Storage/Common/test/parser_tests.pl | 282 ++++++++++++++++++++
 src/PVE/Storage/Plugin.pm                   |  20 +-
 src/test/guest_import_test.pl               |   6 +-
 8 files changed, 417 insertions(+), 34 deletions(-)

diff --git a/ApiChangeLog b/ApiChangeLog
index b1b6b88e..30dd0374 100644
--- a/ApiChangeLog
+++ b/ApiChangeLog
@@ -32,7 +32,9 @@ Future changes should be documented in here.
   * `$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)
+  * `$PVE::Storage::IMPORT_EXT_RE_1` (`import` volume type)
   * `$PVE::Storage::UPLOAD_IMPORT_EXT_RE_1` (`import` volume type)
+  * `$PVE::Storage::OVA_CONTENT_RE_1` (`import` volume type)
 
   These regular expressions were mostly used for parsing file paths and volume
   names corresponding to their volume types, noted in parentheses above.
@@ -42,6 +44,7 @@ Future changes should be documented in here.
 
 * Migrate the following regular expressions:
   * `$PVE::Storage::SAFE_CHAR_CLASS_RE`
+  * `$PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE`
 
   These regular expressions are now defined as private constants in the
   `PVE::Storage::Common::Parse` module and can be obtained by using the
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/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 8999cab4..ed76b8ff 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -12,6 +12,9 @@ use Cwd 'realpath';
 
 use PVE::Tools;
 use PVE::Storage;
+use PVE::Storage::Common::Parse qw(
+    get_re_safe_char_with_whitespace_class
+);
 use PVE::Seccomp qw(stmt /^BPF_/);
 
 # map OVF resources types to descriptive strings
@@ -184,6 +187,22 @@ my sub read_ovf_file {
     return $raw;
 }
 
+my $RE_SAFE_FILEPATH = qr/
+    ^ (?<filepath> (${get_re_safe_char_with_whitespace_class()}+) ) $
+/xn;
+
+my sub parse_filepath_from_xpath {
+    my ($filepath) = @_;
+
+    # untaint & check no sub/parent dirs
+    $filepath =~ $RE_SAFE_FILEPATH;
+    $filepath = $+{filepath};
+
+    return if !$filepath || $filepath eq '.' || $filepath eq '..';
+
+    return $filepath;
+}
+
 # returns two references, $qm which holds qm.conf style key/values, and \@disks
 my sub parse_ovf_do {
     my ($ovf, $ovf_data, $isOva, $debug) = @_;
@@ -357,9 +376,8 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id,
         }
         print "file path: $filepath\n" if $debug;
         my $original_filepath = $filepath;
-        ($filepath) = $filepath =~ m|^(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+)$|; # untaint & check no sub/parent dirs
-        die "referenced path '$original_filepath' is invalid\n"
-            if !$filepath || $filepath eq "." || $filepath eq "..";
+        $filepath = parse_filepath_from_xpath($filepath);
+        die "referenced path '$original_filepath' is invalid\n" if !$filepath;
 
         push @disks,
             {
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index ce90f942..98d5f64e 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -28,6 +28,7 @@ use PVE::Storage::Common qw(
 );
 use PVE::Storage::Common::Parse qw(
     get_re_safe_char_class
+    get_re_safe_char_with_whitespace_class
     parse_abs_path_as_volid
 );
 use PVE::RESTEnvironment qw(log_warn);
@@ -130,6 +131,7 @@ 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}))?)/;
 
+# FIXME: remove this regex on the next APIAGE reset.
 our $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/;
 
 # FIXME: remove this regex on the next APIAGE reset.
@@ -137,8 +139,11 @@ our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova|qcow2|raw|vmdk)/;
 
 # FIXME: remove this regex on the next APIAGE reset.
 our $SAFE_CHAR_CLASS_RE = get_re_safe_char_class();
-our $SAFE_CHAR_WITH_WHITESPACE_CLASS_RE = qr/[ a-zA-Z0-9\-\.\+\=\_]/;
 
+# FIXME: remove this regex on the next APIAGE reset.
+our $SAFE_CHAR_WITH_WHITESPACE_CLASS_RE = get_re_safe_char_with_whitespace_class();
+
+# FIXME: remove this regex on the next APIAGE reset.
 our $OVA_CONTENT_RE_1 = qr/${SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+\.(qcow2|raw|vmdk)/;
 
 # FIXME remove with PVE 9.0, add versioned breaks for pve-manager
diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm
index 2193aa65..296a6c4d 100644
--- a/src/PVE/Storage/Common/Parse.pm
+++ b/src/PVE/Storage/Common/Parse.pm
@@ -10,6 +10,7 @@ use Exporter qw(import);
 
 our @EXPORT_OK = qw(
     get_re_safe_char_class
+    get_re_safe_char_with_whitespace_class
 
     parse_rel_path_as_volname_parts
     parse_rel_path_as_volname
@@ -41,6 +42,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);
 }
@@ -51,8 +54,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
@@ -126,6 +133,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$/,
@@ -139,6 +182,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) {
@@ -223,6 +269,17 @@ sub get_re_safe_char_class() {
     return $RE_SAFE_CHAR_CLASS;
 }
 
+=head3 get_re_safe_char_with_whitespace_class
+
+Like C<L<< get_re_safe_char_class()|/"get_re_safe_char_class" >>>, but the
+returned character class also allows a space character (U+0020).
+
+=cut
+
+sub get_re_safe_char_with_whitespace_class() {
+    return $RE_SAFE_CHAR_WITH_WHITESPACE_CLASS;
+}
+
 =head1 PARSERS RELATED TO VOLUMES
 
 The parsing functions in this section primarily deal with parsing data related
@@ -302,6 +359,30 @@ by Proxmox VE, as well as C<ext-archive>, C<ext-compression> (optional):
         volname => 'backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
     }
 
+=item C<import>
+
+B<NOTE:> Only for C<L<< parse_volname_as_parts()|/"parse_volname_as_parts" >>>!
+
+C<import> volnames may additionally refer to the C<content> inside an OVA file,
+which is further differentiated into C<content-file> and C<content-ext>:
+
+    {
+        file => 'import.ova',
+        ext => 'ova',
+        'disk-path' => 'import.ova',
+        path => 'import.ova/disk.raw',
+
+        # Exclusive to 'import' volume names
+        content => 'disk.raw',
+
+        # Used to further differentiate 'content'
+        'content-file' => 'disk.raw',
+        'content-ext' => 'raw',
+
+        vtype => 'import',
+        volname => 'import/import.ova/disk.raw',
+    }
+
 =back
 
 =cut
diff --git a/src/PVE/Storage/Common/test/parser_tests.pl b/src/PVE/Storage/Common/test/parser_tests.pl
index 137b91b8..2869d3f0 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,
 ];
 
 {
@@ -475,6 +562,179 @@ my sub run_volname_parsing_tests : prototype($) ($volname_tests) {
     return;
 }
 
+# NOTE: These run for parse_rel_path_as_volname_parts() and parse_rel_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_rel_path_as_volname_parts($path, $vtype);
+            my $got_volname = parse_rel_path_as_volname($path, $vtype);
+
+            if (defined($case->{expected})) {
+                eq_or_diff(
+                    $got_volname_parts,
+                    $case->{expected},
+                    'parse_rel_path_as_volname_parts() returns expected hashref',
+                    { context => 50000 },
+                );
+
+                is(
+                    $got_volname,
+                    $case->{expected}->{volname},
+                    'parse_rel_path_as_volname() returns expected volname',
+                );
+            } else {
+                is($got_volname_parts, undef,
+                    'parse_rel_path_as_volname_parts() returns undef');
+                is($got_volname, undef, 'parse_rel_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 = {
@@ -547,6 +807,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) {
@@ -606,6 +886,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 afddca73..45c0b838 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -845,20 +845,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





  parent reply	other threads:[~2026-09-23 15:11 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-23 15:05 [PATCH manager/storage v2 00/50] Fix #2884: Implement Subdirectory Scanning for Dir-Based Storage Types Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 01/50] test: plugin tests: run tests with at most 4 jobs Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 02/50] plugin, common: remove superfluous use of =pod command paragraph Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 03/50] common: add POD headings for groups of helpers Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 04/50] common: use Exporter module for PVE::Storage::Common Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 05/50] plugin: make get_subdir_files a proper subroutine and update style Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 06/50] plugin api: replace helpers w/ standalone subs, bump API version & age Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 07/50] common: prevent autovivification in plugin_get_vtype_subdir helper Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 08/50] plugin: break up if-elsif chain into separate if-blocks Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 09/50] plugin: adapt get_subdir_files helper of list_volumes API method Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 10/50] plugin: update code style of list_volumes plugin " Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 11/50] plugin: use closure for obtaining raw volume data in list_volumes Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 12/50] plugin: use closure for inner loop logic " Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 13/50] storage: update code style in function path_to_volume_id Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 14/50] storage: break up if-elsif chain in path_to_volume_id Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 15/50] storage: heave vtype file path parsing logic inside loop into helper Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 16/50] storage: clean up code that was moved into helper in path_to_volume_id Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 17/50] api: status: move content type assert for up-/downloads into helper Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 18/50] api: status: use helper from common module to get content directory Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 19/50] api: status: move up-/download file path parsing code into helper Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 20/50] api: status: simplify file content assertion logic for up-/download Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 21/50] test: guest import: add tests for PVE::GuestImport Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 22/50] tree-wide: introduce parsing module and replace usages of ISO_EXT_RE_0 Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 23/50] common: test: set up parser testing code, add tests for 'iso' vtype Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 24/50] tree-wide: replace usages of VZTMPL_EXT_RE_1 with parsing functions Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 25/50] tree-wide: replace usages of BACKUP_EXT_RE_2 " Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 26/50] tree-wide: replace usages of inline regexes for snippets with parsers Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 27/50] tree-wide: partially replace usages of regexes for 'import' vtype Max R. Carrara
2026-09-23 15:05 ` Max R. Carrara [this message]
2026-09-23 15:05 ` [PATCH pve-storage v2 29/50] tree-wide: simplify recently refactored parsing logic Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 30/50] test: list volumes: reorganize and modernize test running code Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 31/50] test: list volumes: fix broken test checking for vmlist modifications Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 32/50] test: list volumes: introduce new format for test cases Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 33/50] test: list volumes: remove legacy code and migrate cases to new format Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 34/50] plugin: do not return volumes of undeclared content types anymore Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 35/50] plugin: correct comment in get_subdir_files helper Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 36/50] test: parse volname: modernize code Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 37/50] test: parse volname: adapt tests regarding 'import' volume type Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 38/50] test: parse volname: move VM disk test creation into separate block Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 39/50] test: parse volname: move backup file test creation into sep. block Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 40/50] test: parse volname: parameterize test case creation for some vtypes Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 41/50] test: volume id: modernize code Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 42/50] test: volume id: rename 'volname' test case parameter to 'file' Max R. Carrara
2026-09-23 15:05 ` [PATCH pve-storage v2 43/50] test: filesystem path: modernize code Max R. Carrara
2026-09-23 15:16 ` [PATCH pve-storage v2 44/50] fix #2884: implement nested subdir scanning and support 'iso' vtype Max R. Carrara
2026-09-23 15:16   ` [PATCH pve-storage v2 45/50] fix #2884: support nested subdir scanning for 'vztmpl' volume type Max R. Carrara
2026-09-23 15:16   ` [PATCH pve-storage v2 46/50] fix #2884: support nested subdir scanning for 'snippets' vtype Max R. Carrara
2026-09-23 15:16   ` [PATCH pve-storage v2 47/50] test: add more tests for 'import' vtype & guard against nested subdirs Max R. Carrara
2026-09-23 15:16   ` [PATCH pve-storage v2 48/50] test: add tests guarding against subdir scanning for vtypes Max R. Carrara
2026-09-23 15:16   ` [PATCH pve-manager v2 49/50] fix #2884: ui: storage: add field for 'max-scan-depth' property Max R. Carrara
2026-09-23 15:16   ` [PATCH pve-manager v2 50/50] pve8to9: use helper from common storage module to get vtype subdir Max R. Carrara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260923150606.531239-29-m.carrara@proxmox.com \
    --to=m.carrara@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal