Re: [PATCH pve-storage v1 25/54] tree-wide: replace usages of BACKUP_EXT_RE_2 with parsing functions

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

On Wed, Apr 22, 2026 at 01:12:51PM +0200, Max R. Carrara wrote:
> 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, keep
> the `BACKUP_EXT_RE_2` regex around for now, since it's public and
> therefore also part of the storage API. On an APIVER + APIAGE bump,
> this regex should be marked for removal as well, like the others.
> 
> 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>
> ---
>  src/PVE/Storage.pm                          |   4 +-
>  src/PVE/Storage/Common/Parse.pm             |  39 +++++
>  src/PVE/Storage/Common/test/parser_tests.pl | 181 ++++++++++++++++++++
>  src/PVE/Storage/Plugin.pm                   |  37 ++--
>  4 files changed, 240 insertions(+), 21 deletions(-)
> 
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index d5b0b637..ce6d2154 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -765,9 +765,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_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 b18ef576..950f6b18 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) {
> diff --git a/src/PVE/Storage/Common/test/parser_tests.pl b/src/PVE/Storage/Common/test/parser_tests.pl
> index 31575b66..96159608 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 6bfa5c11..4d63e0a5 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -28,6 +28,7 @@ use JSON;
> 
>  use base qw(PVE::SectionConfig);
> 
> +# TODO: Phase out these two constants
>  use constant KNOWN_COMPRESSION_FORMATS => ('gz', 'lzo', 'zst', 'bz2');
>  use constant COMPRESSOR_RE => join('|', KNOWN_COMPRESSION_FORMATS);
> 
> @@ -830,14 +831,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/([^/]+)$!) {
> @@ -1721,37 +1718,41 @@ my sub get_subdir_files {
>          }
> 
>          if ($vtype eq 'backup') {
> -            return if $filename !~ m!/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!;
> +            my $parts = parse_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+/;

↑ This strictly requires the `$filename` to contain a VMID…

> +            # Check if parsed VMID matched provided VMID in order to avoid
> +            # false positives (VMID in parent directory name)
> +            my $parsed_vmid = $parts->{vmid};

… the regex used above has a generic non-guest case in its `(?<file>)`
part matching `([^/]+)`, so `vmid` can indeed be undefined…

> +            if (defined($vmid) && defined($parsed_vmid)) {
> +                return if $vmid ne $parsed_vmid;

… but this ↑, now, only fires if there *was* a vmid.

So to match the previous behavior we we should probably add an

    } elif (defined($vmid)) {
        return;
    }

(or use a better if/else structure)

here, otherwise those non-guest files will now be listed for guests as
well, whereas before they were filtered out.

> +            }

> 
>              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
> 
> 
> 
> 
> 

--