public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: [pve-devel] applied-series: [PATCH storage v4 1/2] fix #5267: storage: add bzip2 support
Date: Mon, 11 Nov 2024 10:41:19 +0100	[thread overview]
Message-ID: <1731317977.q2i514tlv6.astroid@yuna.none> (raw)
In-Reply-To: <20240913081352.76932-1-m.sandoval@proxmox.com>

with Aaron's T-b on the first patch, and the second patch's commit
message adapted.

On September 13, 2024 10:13 am, Maximiliano Sandoval wrote:
> A popular ISO compressed exclusively with bz2 is OPNsense [2].
> 
> Since this requires adding `bz2` to the list of known compression
> formats we add decompression methods for vmz and tar.
> 
> [2] https://opnsense.org/download/
> 
> Suggested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> 
> Differences from v3:
>  - Remove gzip dependency from d/control
>  - Fix typo in commit message
>  - Add bz2 to VZTMPL_EXT_RE_1 regex which allow us to:
>  - Fix tests
> 
> Differences from v2:
>  - Fix typo s/lzpo/lzop
>  - Add fix to commit message
> 
> Differences from v1:
>  - Add more context to commit message
>  - Add decompression utilities as explicit dependencies
> 
>  src/PVE/Storage.pm                 |  5 ++++-
>  src/PVE/Storage/Plugin.pm          |  2 +-
>  src/test/archive_info_test.pm      |  6 ++++--
>  src/test/list_volumes_test.pm      | 20 ++++++++++++++++++--
>  src/test/parse_volname_test.pm     | 14 +++++++-------
>  src/test/path_to_volume_id_test.pm | 26 ++++++++++++++++----------
>  6 files changed, 50 insertions(+), 23 deletions(-)
> 
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index 57b2038e..2586753b 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -110,7 +110,7 @@ PVE::Storage::Plugin->init();
>  
>  our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i;
>  
> -our $VZTMPL_EXT_RE_1 = qr/\.tar\.(gz|xz|zst)/i;
> +our $VZTMPL_EXT_RE_1 = qr/\.tar\.(gz|xz|zst|bz2)/i;
>  
>  our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/;
>  
> @@ -1550,16 +1550,19 @@ sub decompressor_info {
>  	    gz => ['tar', '-z'],
>  	    lzo => ['tar', '--lzop'],
>  	    zst => ['tar', '--zstd'],
> +	    bz2 => ['tar', '--bzip2'],
>  	},
>  	vma => {
>  	    gz => ['zcat'],
>  	    lzo => ['lzop', '-d', '-c'],
>  	    zst => ['zstd', '-q', '-d', '-c'],
> +	    bz2 => ['bzcat', '-q'],
>  	},
>  	iso => {
>  	    gz => ['zcat'],
>  	    lzo => ['lzop', '-d', '-c'],
>  	    zst => ['zstd', '-q', '-d', '-c'],
> +	    bz2 => ['bzcat', '-q'],
>  	},
>      };
>  
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 8cc693c7..445577bc 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -19,7 +19,7 @@ use JSON;
>  
>  use base qw(PVE::SectionConfig);
>  
> -use constant KNOWN_COMPRESSION_FORMATS =>  ('gz', 'lzo', 'zst');
> +use constant KNOWN_COMPRESSION_FORMATS =>  ('gz', 'lzo', 'zst', 'bz2');
>  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 cf03c3d8..53e37be1 100644
> --- a/src/test/archive_info_test.pm
> +++ b/src/test/archive_info_test.pm
> @@ -121,11 +121,13 @@ my $decompressor = {
>  	gz  => ['tar', '-z'],
>  	lzo => ['tar', '--lzop'],
>  	zst => ['tar', '--zstd'],
> +	bz2 => ['tar', '--bzip2'],
>      },
>      vma => {
>  	gz  => ['zcat'],
>  	lzo => ['lzop', '-d', '-c'],
>  	zst => ['zstd', '-q', '-d', '-c'],
> +	bz2 => ['bzcat', '-q'],
>      },
>  };
>  
> @@ -163,8 +165,8 @@ for my $virt (sort keys %$bkp_suffix) {
>  
>  # add compression formats to test failed matches
>  my $non_bkp_suffix = {
> -    'openvz' => [ 'zip', 'tgz.lzo', 'tar.bz2', 'zip.gz', '', ],
> -    'lxc'    => [ 'zip', 'tgz.lzo', 'tar.bz2', 'zip.gz', '', ],
> +    'openvz' => [ 'zip', 'tgz.lzo', 'zip.gz', '', ],
> +    'lxc'    => [ 'zip', 'tgz.lzo', 'zip.gz', '', ],
>      'qemu'   => [ 'vma.xz', 'vms.gz', 'vmx.zst', '', ],
>      'none'   => [ 'tar.gz', ],
>  };
> diff --git a/src/test/list_volumes_test.pm b/src/test/list_volumes_test.pm
> index d155cb9f..e2648029 100644
> --- a/src/test/list_volumes_test.pm
> +++ b/src/test/list_volumes_test.pm
> @@ -189,6 +189,7 @@ my @tests = (
>  	    "$storage_dir/dump/vzdump-lxc-16112-2020_03_30-21_49_30.tar.gz",
>  	    "$storage_dir/dump/vzdump-lxc-16112-2020_03_30-21_49_30.tar.zst",
>  	    "$storage_dir/dump/vzdump-lxc-16112-2020_03_30-21_59_30.tgz",
> +	    "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",
>  	],
>  	expected => [
>  	    {
> @@ -237,6 +238,15 @@ my @tests = (
>  		'vmid'    => '16112',
>  		'volid'   => 'local:backup/vzdump-lxc-16112-2020_03_30-21_59_30.tgz',
>  	    },
> +	    {
> +		'content' => 'backup',
> +		'ctime'   => 1585604370,
> +		'format'  => 'tar.bz2',
> +		'size'    => DEFAULT_SIZE,
> +		'subtype' => 'openvz',
> +		'vmid'    => '16112',
> +		'volid'   => 'local:backup/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
> +	    },
>  	],
>      },
>      {
> @@ -315,6 +325,7 @@ my @tests = (
>  	    "$storage_dir/template/iso/proxmox-ve_6.1-1.iso",
>  	    "$storage_dir/template/cache/archlinux-base_20190924-1_amd64.tar.gz",
>  	    "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.gz",
> +	    "$storage_dir/template/cache/debian-11.0-standard_11.0-1_amd64.tar.bz2",
>  	    "$storage_dir/template/cache/alpine-3.10-default_20190626_amd64.tar.xz",
>  	    "$storage_dir/snippets/userconfig.yaml",
>  	    "$storage_dir/snippets/hookscript.pl",
> @@ -343,6 +354,13 @@ my @tests = (
>  		'size'    => DEFAULT_SIZE,
>  		'volid'   => 'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.gz',
>  	    },
> +	    {
> +		'content' => 'vztmpl',
> +		'ctime'   => DEFAULT_CTIME,
> +		'format'  => 'tbz2',
> +		'size'    => DEFAULT_SIZE,
> +		'volid'   => 'local:vztmpl/debian-11.0-standard_11.0-1_amd64.tar.bz2',
> +	    },
>  	    {
>  		'content' => 'iso',
>  		'ctime'   => DEFAULT_CTIME,
> @@ -438,9 +456,7 @@ my @tests = (
>  	    "$storage_dir/images/ssss/vm-1234-disk-0.qcow2",
>  	    "$storage_dir/template/iso/yet-again-a-installation-disk.dvd",
>  	    "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.zip.gz",
> -	    "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.bz2",
>  	    "$storage_dir/private/subvol-19254-disk-0/19254",
> -	    "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",
>  	    "$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",
> diff --git a/src/test/parse_volname_test.pm b/src/test/parse_volname_test.pm
> index d6ac8854..6c5ba043 100644
> --- a/src/test/parse_volname_test.pm
> +++ b/src/test/parse_volname_test.pm
> @@ -49,6 +49,11 @@ my $tests = [
>  	volname     => 'vztmpl/debian-10.0-standard_10.0-1_amd64.tar.xz',
>  	expected    => ['vztmpl', 'debian-10.0-standard_10.0-1_amd64.tar.xz'],
>      },
> +    {
> +	description => 'Container template tar.bz2',
> +	volname     => 'vztmpl/debian-10.0-standard_10.0-1_amd64.tar.bz2',
> +	expected    => ['vztmpl', 'debian-10.0-standard_10.0-1_amd64.tar.bz2'],
> +    },
>      #
>      # container rootdir
>      #
> @@ -98,11 +103,6 @@ my $tests = [
>  	volname     => 'vztmpl/debian-10.0-standard_10.0-1_amd64.zip.gz',
>  	expected    => "unable to parse directory volume name 'vztmpl/debian-10.0-standard_10.0-1_amd64.zip.gz'\n",
>      },
> -    {
> -	description => 'Failed match: Container template, tar.bz2',
> -	volname     => 'vztmpl/debian-10.0-standard_10.0-1_amd64.tar.bz2',
> -	expected    => "unable to parse directory volume name 'vztmpl/debian-10.0-standard_10.0-1_amd64.tar.bz2'\n",
> -    },
>      {
>  	description => 'Failed match: Container rootdir, subvol',
>  	volname     => "rootdir/subvol-$vmid-disk-0",
> @@ -177,7 +177,7 @@ 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' ],
> +    lxc    => [ 'tar', 'tgz', 'tar.gz', 'tar.lzo', 'tar.zst', 'tar.bz2' ],
>      openvz => [ 'tar', 'tgz', 'tar.gz', 'tar.lzo', 'tar.zst' ],
>  };
>  
> @@ -204,7 +204,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' ],
> -    lxc    => [ 'tar.bz2', 'zip.gz', 'tgz.lzo' ],
> +    lxc    => [ 'zip.gz', 'tgz.lzo' ],
>  };
>  foreach my $virt (keys %$non_bkp_suffix) {
>      my $suffix = $non_bkp_suffix->{$virt};
> diff --git a/src/test/path_to_volume_id_test.pm b/src/test/path_to_volume_id_test.pm
> index 8149c881..3198752f 100644
> --- a/src/test/path_to_volume_id_test.pm
> +++ b/src/test/path_to_volume_id_test.pm
> @@ -116,6 +116,14 @@ my @tests = (
>  	    'local:backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.zst'
>  	],
>      },
> +    {
> +	description => 'Backup, tar.bz2',
> +	volname     => "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",
> +	expected    => [
> +	    'backup',
> +	    'local:backup/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
> +	],
> +    },
>  
>      {
>  	description => 'ISO file',
> @@ -133,6 +141,14 @@ my @tests = (
>  	    'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.gz',
>  	],
>      },
> +    {
> +	description => 'CT template, wrong ending, tar bz2',
> +	volname     => "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.bz2",
> +	expected    => [
> +	    'vztmpl',
> +	    'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.bz2',
> +	],
> +    },
>  
>      {
>  	description => 'Rootdir',
> @@ -191,21 +207,11 @@ my @tests = (
>  	volname     => "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.zip.gz",
>  	expected    => [''],
>      },
> -    {
> -	description => 'CT template, wrong ending, tar bz2',
> -	volname     => "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.bz2",
> -	expected    => [''],
> -    },
>      {
>  	description => 'Rootdir as subvol, wrong path',
>  	volname     => "$storage_dir/private/subvol-19254-disk-0/",
>  	expected    => [''],
>      },
> -    {
> -	description => 'Backup, wrong ending, openvz, tar.bz2',
> -	volname     => "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",
> -	expected    => [''],
> -    },
>      {
>  	description => 'Backup, wrong format, openvz, zip.gz',
>  	volname     => "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.zip.gz",
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


      parent reply	other threads:[~2024-11-11  9:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-13  8:13 [pve-devel] " Maximiliano Sandoval
2024-09-13  8:13 ` [pve-devel] [PATCH storage v4 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd Maximiliano Sandoval
2024-09-13  8:23   ` Fabian Grünbichler
2024-09-13  8:25     ` Maximiliano Sandoval
2024-10-01 11:03       ` Maximiliano Sandoval
2024-10-07 11:57         ` Maximiliano Sandoval
2024-10-14 12:19           ` Maximiliano Sandoval
2024-11-05 10:31             ` Maximiliano Sandoval
2024-11-07 10:35 ` [pve-devel] [PATCH storage v4 1/2] fix #5267: storage: add bzip2 support Aaron Lauterer
2024-11-11  9:41 ` Fabian Grünbichler [this message]

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=1731317977.q2i514tlv6.astroid@yuna.none \
    --to=f.gruenbichler@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