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

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Jun 30, 2026 at 04:19:21PM +0200, Wolfgang Bumiller wrote:
> On Wed, Apr 22, 2026 at 01:12:53PM +0200, Max R. Carrara wrote:
> > Add partial support for the 'import' volume type in
> > `PVE::Storage::Common::Parse`. What remains unsupported right now is
> > parsing volume names for that type, as those require a little more
> > care.
> > 
> > Replace most usages of the `PVE::Storage::IMPORT_EXT_RE_1` regex with
> > parsing functions from `::Common::Parse`.
> > 
> > Replace the one remaining spot where we use the
> > `PVE::Storage::UPLOAD_IMPORT_EXT_RE_1` regex with a parsing function
> > as well. This regex should now be phased out in a future APIVER +
> > APIAGE bump, like the others in previous commits.
> > 
> > Note that the `UPLOAD_IMPORT_EXT_RE_1` regex only exists to handle the
> > special case of excluding .ovf files from the upload / download_url
> > API methods. Instead of adding a separate parser (or a flag etc.) to
> > handle this case, just parse the path of the up-/downloaded file
> > regardless. Then, raise a parameter exception when the `ovf` file
> > extension is encountered.
> > 
> > Also add a bunch of test cases that target the `import` volume type
> > for the `list_volumes()` plugin API method. For all of these tests,
> > sort the expected and resulting list items by their volume ID in order
> > to make the output comparison deterministic. Iterate over the declared
> > content / volume types of the mocked storage config instead of using a
> > fixed list of vtypes as well.
> > 
> > Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
> > ---
> >  src/PVE/API2/Storage/Status.pm  |  13 ++-
> >  src/PVE/Storage.pm              |   4 +-
> >  src/PVE/Storage/Common/Parse.pm |  13 +++
> >  src/PVE/Storage/Plugin.pm       |   9 +-
> >  src/test/list_volumes_test.pm   | 180 +++++++++++++++++++++++++++++++-
> >  5 files changed, 203 insertions(+), 16 deletions(-)
> > 
> > diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
> > index 03929d26..b578a1ed 100644
> > --- a/src/PVE/API2/Storage/Status.pm
> > +++ b/src/PVE/API2/Storage/Status.pm
> > @@ -96,13 +96,18 @@ my sub parse_transferred_file_path_extension : prototype($$) {
> >      }
> > 
> >      if ($vtype eq 'import') {
> > -        if (
> > -            $path !~ m!${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::UPLOAD_IMPORT_EXT_RE_1$!
> > -        ) {
> > +        my $parts = parse_path_as_volname_parts($path, $vtype);
> > +
> > +        if (!defined($parts)) {
> >              raise_param_exc({ filename => "invalid filename or wrong extension" });
> >          }
> > 
> > -        my $ext = $1;
> > +        my $ext = $parts->{ext};
> > +
> > +        if ($ext eq 'ovf') {
> > +            raise_param_exc({ filename => "wrong file extension" });
> > +        }
> > +
> >          return $ext;
> >      }
> > 
> > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> > index 20b7bafa..3a716894 100755
> > --- a/src/PVE/Storage.pm
> > +++ b/src/PVE/Storage.pm
> > @@ -773,9 +773,7 @@ sub path_to_volume_id {
> >          }
> > 
> >          if ($vtype eq 'import') {
> > -            return if $filename !~ m!/(${SAFE_CHAR_CLASS_RE}+${IMPORT_EXT_RE_1})$!;
> > -            my $name = $1;
> > -            return "$sid:import/$name";
> > +            return parse_path_as_volid($sid, $scfg, $path, $vtype);
> >          }
> > 
> >          return;
> > diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm
> > index 33220758..bd7cec13 100644
> > --- a/src/PVE/Storage/Common/Parse.pm
> > +++ b/src/PVE/Storage/Common/Parse.pm
> > @@ -37,6 +37,8 @@ my @VZTMPL_COMPRESSION_EXTENSIONS = ('gz', 'xz', 'zst', 'bz2');
> > 
> >  my @BACKUP_COMPRESSION_EXTENSIONS = ('gz', 'lzo', 'zst', 'bz2');
> > 
> > +my @IMPORT_EXTENSIONS = ('ova', 'ovf', 'qcow2', 'raw', 'vmdk');
> > +
> >  my sub join_to_re_alternations(@list) {
> >      return join('|', map { quotemeta } @list);
> >  }
> > @@ -45,6 +47,10 @@ my $RE_VZTMPL_COMPRESSION_EXTENSIONS = join_to_re_alternations(@VZTMPL_COMPRESSI
> > 
> >  my $RE_BACKUP_COMPRESSION_EXTENSIONS = join_to_re_alternations(@BACKUP_COMPRESSION_EXTENSIONS);
> > 
> > +my $RE_IMPORT_EXTENSIONS = join_to_re_alternations(@IMPORT_EXTENSIONS);
> > +
> > +my $RE_SAFE_CHAR_CLASS = qr/[a-zA-Z0-9\-\.\+\=\_]/;
> > +
> >  my $RE_PARENT_DIR = quotemeta('..');
> >  my $RE_CONTAINS_PARENT_DIR = qr!
> >      ( ^$RE_PARENT_DIR/ )  #  ../ --> Beginning of path
> > @@ -112,11 +118,18 @@ my $RE_SNIPPETS_FILE_PATH = qr!
> >      )
> >  !xn;
> > 
> > +my $RE_IMPORT_FILE_PATH = qr!
> > +    (?<path>
> > +        (?<file> ($RE_SAFE_CHAR_CLASS)+ \. (?<ext> $RE_IMPORT_EXTENSIONS) )
> 
> This misses an `(?i)`.
> 
> After more checking, so does `ext_compression` from the previous `BACKUP_EXT_RE_2` patch.
> 
> Perhaps there should be a `join_to_insensitive_alterations()` helper
> which generally wraps the alterations inside `(?i:…)` used for all the
> file extension regexes?
> 
> > +    )
> > +!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$/,
> >      snippets => qr/^$RE_SNIPPETS_FILE_PATH$/,
> > +    import => qr/^$RE_IMPORT_FILE_PATH$/,
> >  };
> > 
> >  my $RE_VOLNAME_FOR_VTYPE = {
> > diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> > index b08e038b..39e5cb0d 100644
> > --- a/src/PVE/Storage/Plugin.pm
> > +++ b/src/PVE/Storage/Plugin.pm
> > @@ -1768,13 +1768,12 @@ my sub get_subdir_files {
> >          }
> > 
> >          if ($vtype eq 'import') {
> > -            return
> > -                if $filename !~
> > -                m!/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!i;
> 
> (to-the-above-relevant /i (`m!…!i`) for context)

Actually, nevermind, both of these used `qr//`, so the `!i` at the end
here should not have any effect whatsoever here, if I'm not mistaken.

> 
> > +            my $parts = parse_path_as_volid_parts($storeid, $scfg, $path, $vtype);
> > +            return if !defined($parts);

--