[pve-devel] [PATCH storage] storage: add bzip2 support

[pve-devel] [PATCH storage] storage: add bzip2 support

[Maximiliano Sandoval]

This was requested at [1].

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=5267

Suggested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

For the sake of not having a commit where tests fail, this is all in one commit.
Please tell me if I should split it.

The idea is to expose this algorithm only for bz2 decompression in the web UI
when downloading ISOs. A followup will contain the patch for pve-manager.

An popular iso that is compressed with bz2 is opnsense [2].

[2] https://opnsense.org/download/

 src/PVE/Storage.pm                 |  3 +++
 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     |  4 ++--
 src/test/path_to_volume_id_test.pm | 13 ++++++++-----
 6 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 57b2038..0caac71 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -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 6444390..e5ef9de 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 cf03c3d..53e37be 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 d155cb9..ba959ff 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',
+	    },
 	],
     },
     {
@@ -440,7 +450,6 @@ my @tests = (
 	    "$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 d6ac885..5f63941 100644
--- a/src/test/parse_volname_test.pm
+++ b/src/test/parse_volname_test.pm
@@ -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 8149c88..0f39b03 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',
@@ -201,11 +209,6 @@ my @tests = (
 	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

Re: [pve-devel] [PATCH storage] storage: add bzip2 support

[Stoiko Ivanov]

In general I think the addition of bz2 for ISOs makes sense (if only for
OPNSense's sake) - Thanks for addressing this!

You want to add bzip2 as dependency to debian/control
we probably should also add  lzop, zstd, gzip to the dependencies
although they are pulled in as dependency of pve-manger (due to
vzdump being there) - please do so in a separate commit


On Fri,  2 Aug 2024 16:34:14 +0200
Maximiliano Sandoval <m.sandoval@proxmox.com> wrote:

> This was requested at [1].
add a 'close #5267' to the commit message as suggested in the
developer-docs:
https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages
Your colleagues preparing the release-notes will be grateful.

> 
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=5267
> 
> Suggested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> 
> For the sake of not having a commit where tests fail, this is all in one commit.
> Please tell me if I should split it.
having each commit working is worth a lot if you have to bisect at some
point - and in this case following the changes works well for me - so I'd
agree to keep this as one commit

> 
> The idea is to expose this algorithm only for bz2 decompression in the web UI
> when downloading ISOs. A followup will contain the patch for pve-manager.
you say you only want it for ISOs here ...
> 
> An popular iso that is compressed with bz2 is opnsense [2].
That motivation could also be useful in the commit-message for the future.
(I did not expect that bz2 as sole compressed format is something I'd run
into nowadays)

will give it a spin with the pve-manager commit.


> 
> [2] https://opnsense.org/download/
> 
>  src/PVE/Storage.pm                 |  3 +++
>  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     |  4 ++--
>  src/test/path_to_volume_id_test.pm | 13 ++++++++-----
>  6 files changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index 57b2038..0caac71 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -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'],
... but add it for tar and vma here?
(haven't tried if adding it only for one of the formats works explicitly
though)

also - but this is really in the realm of cosmetics - why do we have
zcat/bzcat, but not zstdcat (lzop does not ship a link 
to lzopcat ..)? (but again - I don't think there's a need to change
stuff here)


>  	},
>  	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 6444390..e5ef9de 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 cf03c3d..53e37be 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 d155cb9..ba959ff 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',
> +	    },
>  	],
>      },
>      {
> @@ -440,7 +450,6 @@ my @tests = (
>  	    "$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 d6ac885..5f63941 100644
> --- a/src/test/parse_volname_test.pm
> +++ b/src/test/parse_volname_test.pm
> @@ -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 8149c88..0f39b03 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',
> @@ -201,11 +209,6 @@ my @tests = (
>  	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",



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

Re: [pve-devel] [PATCH storage] storage: add bzip2 support

[Christian Ebner]

On 8/2/24 18:41, Stoiko Ivanov wrote:
>>
>> [2] https://opnsense.org/download/
>>
>>   src/PVE/Storage.pm                 |  3 +++
>>   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     |  4 ++--
>>   src/test/path_to_volume_id_test.pm | 13 ++++++++-----
>>   6 files changed, 28 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
>> index 57b2038..0caac71 100755
>> --- a/src/PVE/Storage.pm
>> +++ b/src/PVE/Storage.pm
>> @@ -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'],
> ... but add it for tar and vma here?
> (haven't tried if adding it only for one of the formats works explicitly
> though)
> 

Same as Stoiko, I was a bit surprised to finding this hunk here and the 
others related to vma archives further below as well, while looking at 
your patch.
I suggest to add a bit more description to the commit message and 
additional context to the commit title as well.



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

Re: [pve-devel] [PATCH storage] storage: add bzip2 support

[Maximiliano Sandoval]

Stoiko Ivanov <s.ivanov@proxmox.com> writes:

> In general I think the addition of bz2 for ISOs makes sense (if only for
> OPNSense's sake) - Thanks for addressing this!
>
> You want to add bzip2 as dependency to debian/control
> we probably should also add  lzop, zstd, gzip to the dependencies
> although they are pulled in as dependency of pve-manger (due to
> vzdump being there) - please do so in a separate commit

👌

> On Fri,  2 Aug 2024 16:34:14 +0200
> Maximiliano Sandoval <m.sandoval@proxmox.com> wrote:
>
>> This was requested at [1].
> add a 'close #5267' to the commit message as suggested in the
> developer-docs:
> https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages
> Your colleagues preparing the release-notes will be grateful.
>

#5267 also asks for xz support and it is loosely titled "Please support
 more decompression algorithms" hence why this does not really fix this
 issue.

>> 
>> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=5267
>> 
>> Suggested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>> ---
>> 
>> For the sake of not having a commit where tests fail, this is all in one commit.
>> Please tell me if I should split it.
> having each commit working is worth a lot if you have to bisect at some
> point - and in this case following the changes works well for me - so I'd
> agree to keep this as one commit
>

👌

>> 
>> The idea is to expose this algorithm only for bz2 decompression in the web UI
>> when downloading ISOs. A followup will contain the patch for pve-manager.
> you say you only want it for ISOs here ...
>> 
>> An popular iso that is compressed with bz2 is opnsense [2].
> That motivation could also be useful in the commit-message for the future.
> (I did not expect that bz2 as sole compressed format is something I'd run
> into nowadays)
>
> will give it a spin with the pve-manager commit.
>
>
>> 
>> [2] https://opnsense.org/download/
>> 
>>  src/PVE/Storage.pm                 |  3 +++
>>  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     |  4 ++--
>>  src/test/path_to_volume_id_test.pm | 13 ++++++++-----
>>  6 files changed, 28 insertions(+), 11 deletions(-)
>> 
>> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
>> index 57b2038..0caac71 100755
>> --- a/src/PVE/Storage.pm
>> +++ b/src/PVE/Storage.pm
>> @@ -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'],
> ... but add it for tar and vma here?
> (haven't tried if adding it only for one of the formats works explicitly
> though)

The patch requires adding a new value to `KNOWN_COMPRESSION_FORMATS`
which at least for the current usecase is used to define the supported
formats for the API. this change can potentially break something else if
these methods are not implemented.

>
> also - but this is really in the realm of cosmetics - why do we have
> zcat/bzcat, but not zstdcat (lzop does not ship a link 
> to lzopcat ..)? (but again - I don't think there's a need to change
> stuff here)
>
>
>>  	},
>>  	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 6444390..e5ef9de 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 cf03c3d..53e37be 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 d155cb9..ba959ff 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',
>> +	    },
>>  	],
>>      },
>>      {
>> @@ -440,7 +450,6 @@ my @tests = (
>>  	    "$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 d6ac885..5f63941 100644
>> --- a/src/test/parse_volname_test.pm
>> +++ b/src/test/parse_volname_test.pm
>> @@ -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 8149c88..0f39b03 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',
>> @@ -201,11 +209,6 @@ my @tests = (
>>  	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",

I will add more info to the commit message and add dependencies to
d/control before sending v2.

-- 
Maximiliano


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