From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id B3AEB1FF13C for ; Thu, 05 Mar 2026 15:51:39 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A044FC405; Thu, 5 Mar 2026 15:52:43 +0100 (CET) Message-ID: <2f390ef7-000c-4fc4-8997-de8af84711d3@proxmox.com> Date: Thu, 5 Mar 2026 15:52:07 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-storage 1/1] storage: add xz support To: mail@romanondracek.cz, pve-devel@lists.proxmox.com References: <20260305120830.4108248-1-mail@romanondracek.cz> <20260305120830.4108248-2-mail@romanondracek.cz> Content-Language: en-US From: David Riley In-Reply-To: <20260305120830.4108248-2-mail@romanondracek.cz> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1772722300660 X-SPAM-LEVEL: Spam detection results: 0 BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.018 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.703 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 1.386 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 7NRAZ6QKAPIDYF75KLPVYSLW2T5DOXAE X-Message-ID-Hash: 7NRAZ6QKAPIDYF75KLPVYSLW2T5DOXAE X-MailFrom: d.riley@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: no deep review I just noted a small thing, comment inline On 3/5/26 1:08 PM, mail@romanondracek.cz wrote: > From: Roman Ondráček > > Signed-off-by: Roman Ondráček > --- > debian/control | 1 + > src/PVE/Storage.pm | 5 ++++- > src/PVE/Storage/Plugin.pm | 2 +- > src/test/archive_info_test.pm | 6 ++++-- > src/test/list_volumes_test.pm | 11 ++++++++++- > src/test/parse_volname_test.pm | 8 ++++---- > src/test/path_to_volume_id_test.pm | 12 +++++++----- > 7 files changed, 31 insertions(+), 14 deletions(-) > > diff --git a/debian/control b/debian/control > index 6bd55a2..9ef10e6 100644 > --- a/debian/control > +++ b/debian/control > @@ -50,6 +50,7 @@ Depends: bzip2, > smbclient, > thin-provisioning-tools, > udev, > + xz-utils, > zstd, > ${misc:Depends}, > ${perl:Depends}, > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm > index 6e87bac..b9ae6d7 100755 > --- a/src/PVE/Storage.pm > +++ b/src/PVE/Storage.pm > @@ -115,7 +115,7 @@ PVE::Storage::Plugin->init(); > > our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i; > > -our $VZTMPL_EXT_RE_1 = qr/\.(?|(tar)(?!\.)|tar\.(gz|xz|zst|bz2))/i; > +our $VZTMPL_EXT_RE_1 = qr/\.(?|(tar)(?!\.)|tar\.(gz|xz|zst|bz2|xz))/i; seems like xz was already part of the regex. > > our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/; > > @@ -1761,18 +1761,21 @@ sub decompressor_info { > lzo => ['tar', '--lzop'], > zst => ['tar', '--zstd'], > bz2 => ['tar', '--bzip2'], > + xz => ['tar', '--xz'], > }, > vma => { > gz => ['zcat'], > lzo => ['lzop', '-d', '-c'], > zst => ['zstd', '-q', '-d', '-c'], > bz2 => ['bzcat', '-q'], > + xz => ['xz', '-q', '-d', '-c'], > }, > iso => { > gz => ['zcat'], > lzo => ['lzop', '-d', '-c'], > zst => ['zstd', '-q', '-d', '-c'], > bz2 => ['bzcat', '-q'], > + xz => ['xz', '-q', '-d', '-c'], > }, > }; > > diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm > index 6f3d691..5b0216e 100644 > --- a/src/PVE/Storage/Plugin.pm > +++ b/src/PVE/Storage/Plugin.pm > @@ -21,7 +21,7 @@ use JSON; > > use base qw(PVE::SectionConfig); > > -use constant KNOWN_COMPRESSION_FORMATS => ('gz', 'lzo', 'zst', 'bz2'); > +use constant KNOWN_COMPRESSION_FORMATS => ('gz', 'lzo', 'zst', 'bz2', 'xz'); > use constant COMPRESSOR_RE => join('|', KNOWN_COMPRESSION_FORMATS); > > use constant LOG_EXT => ".log"; > diff --git a/src/test/archive_info_test.pm b/src/test/archive_info_test.pm > index c5c3992..b8dfbff 100644 > --- a/src/test/archive_info_test.pm > +++ b/src/test/archive_info_test.pm > @@ -115,19 +115,21 @@ my $tests = [ > }, > ]; > > -# add new compression fromats to test > +# add new compression formats to test > my $decompressor = { > tar => { > gz => ['tar', '-z'], > lzo => ['tar', '--lzop'], > zst => ['tar', '--zstd'], > bz2 => ['tar', '--bzip2'], > + xz => ['tar', '--xz'], > }, > vma => { > gz => ['zcat'], > lzo => ['lzop', '-d', '-c'], > zst => ['zstd', '-q', '-d', '-c'], > bz2 => ['bzcat', '-q'], > + xz => ['xz', '-q', '-d', '-c'], > }, > }; > > @@ -167,7 +169,7 @@ for my $virt (sort keys %$bkp_suffix) { > my $non_bkp_suffix = { > 'openvz' => ['zip', 'tgz.lzo', 'zip.gz', ''], > 'lxc' => ['zip', 'tgz.lzo', 'zip.gz', ''], > - 'qemu' => ['vma.xz', 'vms.gz', 'vmx.zst', ''], > + 'qemu' => ['vms.gz', 'vmx.zst', ''], > 'none' => ['tar.gz'], > }; > > diff --git a/src/test/list_volumes_test.pm b/src/test/list_volumes_test.pm > index 0876902..d79e991 100644 > --- a/src/test/list_volumes_test.pm > +++ b/src/test/list_volumes_test.pm > @@ -90,6 +90,7 @@ my @tests = ( > "$storage_dir/images/16110/vm-16110-disk-1.raw", > "$storage_dir/images/16110/vm-16110-disk-2.vmdk", > "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz", > + "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz", > "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo", > "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma", > "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst", > @@ -136,6 +137,15 @@ my @tests = ( > 'vmid' => '16110', > 'volid' => 'local:backup/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz', > }, > + { > + 'content' => 'backup', > + 'ctime' => 1585602760, > + 'format' => 'vma.xz', > + 'size' => DEFAULT_SIZE, > + 'subtype' => 'qemu', > + 'vmid' => '16110', > + 'volid' => 'local:backup/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz', > + }, > { > 'content' => 'backup', > 'ctime' => 1585602765, > @@ -457,7 +467,6 @@ my @tests = ( > "$storage_dir/private/subvol-19254-disk-0/19254", > "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.zip.gz", > "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tgz.lzo", > - "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz", > "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vms.gz", > ], > expected => [], # returns empty list > diff --git a/src/test/parse_volname_test.pm b/src/test/parse_volname_test.pm > index 5c9478a..035e3ee 100644 > --- a/src/test/parse_volname_test.pm > +++ b/src/test/parse_volname_test.pm > @@ -247,9 +247,9 @@ foreach my $s (@$disk_suffix) { > > # create more test cases for backup files matches > my $bkp_suffix = { > - qemu => ['vma', 'vma.gz', 'vma.lzo', 'vma.zst'], > - lxc => ['tar', 'tgz', 'tar.gz', 'tar.lzo', 'tar.zst', 'tar.bz2'], > - openvz => ['tar', 'tgz', 'tar.gz', 'tar.lzo', 'tar.zst'], > + qemu => ['vma', 'vma.gz', 'vma.lzo', 'vma.xz', 'vma.zst'], > + lxc => ['tar', 'tgz', 'tar.gz', 'tar.lzo', 'tar.xz', 'tar.zst', 'tar.bz2'], > + openvz => ['tar', 'tgz', 'tar.gz', 'tar.lzo', 'tar.xz', 'tar.zst'], > }; > > foreach my $virt (keys %$bkp_suffix) { > @@ -277,7 +277,7 @@ foreach my $virt (keys %$bkp_suffix) { > > # create more test cases for failed backup files matches > my $non_bkp_suffix = { > - qemu => ['vms.gz', 'vma.xz'], > + qemu => ['vms.gz'], > lxc => ['zip.gz', 'tgz.lzo'], > }; > foreach my $virt (keys %$non_bkp_suffix) { > diff --git a/src/test/path_to_volume_id_test.pm b/src/test/path_to_volume_id_test.pm > index dfa51e6..ad3f39a 100644 > --- a/src/test/path_to_volume_id_test.pm > +++ b/src/test/path_to_volume_id_test.pm > @@ -92,6 +92,13 @@ my @tests = ( > 'backup', 'local:backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo', > ], > }, > + { > + description => 'Backup, vma.xz', > + volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz", > + expected => [ > + 'backup', 'local:backup/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz', > + ], > + }, > { > description => 'Backup, vma.zst', > volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst", > @@ -212,11 +219,6 @@ my @tests = ( > volname => "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tgz.lzo", > expected => [''], > }, > - { > - description => 'Backup, wrong ending, qemu, vma.xz', > - volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz", > - expected => [''], > - }, > { > description => 'Backup, wrong format, qemu, vms.gz', > volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vms.gz",