* [pve-devel] [PATCH storage v3 1/2] fix #5267: storage: add bzip2 support
@ 2024-08-12 11:44 Maximiliano Sandoval
2024-08-12 11:44 ` [pve-devel] [PATCH storage v3 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd Maximiliano Sandoval
2024-09-10 12:24 ` [pve-devel] [PATCH storage v3 1/2] fix #5267: storage: add bzip2 support Fiona Ebner
0 siblings, 2 replies; 6+ messages in thread
From: Maximiliano Sandoval @ 2024-08-12 11:44 UTC (permalink / raw)
To: pve-devel
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 a 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 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 | 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH storage v3 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd
2024-08-12 11:44 [pve-devel] [PATCH storage v3 1/2] fix #5267: storage: add bzip2 support Maximiliano Sandoval
@ 2024-08-12 11:44 ` Maximiliano Sandoval
2024-09-10 12:24 ` Fiona Ebner
2024-09-10 12:24 ` [pve-devel] [PATCH storage v3 1/2] fix #5267: storage: add bzip2 support Fiona Ebner
1 sibling, 1 reply; 6+ messages in thread
From: Maximiliano Sandoval @ 2024-08-12 11:44 UTC (permalink / raw)
To: pve-devel
The decompressor_info method calls binaries provided by these packages
so they are (alphabetically) added explicitly as dependencies.
To avoid a build-time error
E: libpve-storage-perl: depends-on-essential-package-without-using-version Depends: gzip
the current minor version available in bullseye was set for gzip.
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
debian/control | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/debian/control b/debian/control
index d7afa98..8cf36a9 100644
--- a/debian/control
+++ b/debian/control
@@ -26,11 +26,13 @@ Breaks: libpve-guest-common-perl (<< 4.0-3),
pve-container (<< 3.1-2),
pve-manager (<< 5.2-12),
qemu-server (<< 6.1-14),
-Depends: ceph-common (>= 12.2~),
+Depends: bzip2,
+ ceph-common (>= 12.2~),
ceph-fuse,
cifs-utils,
cstream,
glusterfs-client (>= 3.4.0-2),
+ gzip (>= 1.10),
libfile-chdir-perl,
libposix-strptime-perl,
libpve-access-control (>= 8.1.2),
@@ -39,6 +41,7 @@ Depends: ceph-common (>= 12.2~),
libpve-common-perl (>= 8.1.1),
librados2-perl,
lvm2,
+ lzop,
nfs-common,
proxmox-backup-client (>= 2.1.10~),
proxmox-backup-file-restore,
@@ -47,6 +50,7 @@ Depends: ceph-common (>= 12.2~),
smbclient,
thin-provisioning-tools,
udev,
+ zstd,
${misc:Depends},
${perl:Depends},
Recommends: pve-esxi-import-tools (>= 0.6.0),
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage v3 1/2] fix #5267: storage: add bzip2 support
2024-08-12 11:44 [pve-devel] [PATCH storage v3 1/2] fix #5267: storage: add bzip2 support Maximiliano Sandoval
2024-08-12 11:44 ` [pve-devel] [PATCH storage v3 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd Maximiliano Sandoval
@ 2024-09-10 12:24 ` Fiona Ebner
1 sibling, 0 replies; 6+ messages in thread
From: Fiona Ebner @ 2024-09-10 12:24 UTC (permalink / raw)
To: Proxmox VE development discussion, Maximiliano Sandoval
Am 12.08.24 um 13:44 schrieb Maximiliano Sandoval:
> 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 a tar.
Typo here I presume.
> 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",
Why is this one still listed here in the non-matching test cases?
Shouldn't this be recognized too now?
> "$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",
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage v3 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd
2024-08-12 11:44 ` [pve-devel] [PATCH storage v3 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd Maximiliano Sandoval
@ 2024-09-10 12:24 ` Fiona Ebner
2024-09-10 12:52 ` Fabian Grünbichler
0 siblings, 1 reply; 6+ messages in thread
From: Fiona Ebner @ 2024-09-10 12:24 UTC (permalink / raw)
To: Proxmox VE development discussion, Maximiliano Sandoval
Am 12.08.24 um 13:44 schrieb Maximiliano Sandoval:
> The decompressor_info method calls binaries provided by these packages
> so they are (alphabetically) added explicitly as dependencies.
>
> To avoid a build-time error
>
> E: libpve-storage-perl: depends-on-essential-package-without-using-version Depends: gzip
>
> the current minor version available in bullseye was set for gzip.
>
Since I didn't get that error, I'm interested: what command are you
building with? Why the version for Bullseye?
I guess many systems already have bzip2 installed, but do we really want
to require it for everybody? Or should we rather keep it optional (could
be a Recommends or Suggests dependency) and add a clean error if not
installed?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage v3 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd
2024-09-10 12:24 ` Fiona Ebner
@ 2024-09-10 12:52 ` Fabian Grünbichler
2024-09-10 13:04 ` Fiona Ebner
0 siblings, 1 reply; 6+ messages in thread
From: Fabian Grünbichler @ 2024-09-10 12:52 UTC (permalink / raw)
To: Proxmox VE development discussion, Fiona Ebner, Maximiliano Sandoval
> Fiona Ebner <f.ebner@proxmox.com> hat am 10.09.2024 14:24 CEST geschrieben:
> Am 12.08.24 um 13:44 schrieb Maximiliano Sandoval:
> > The decompressor_info method calls binaries provided by these packages
> > so they are (alphabetically) added explicitly as dependencies.
> >
> > To avoid a build-time error
> >
> > E: libpve-storage-perl: depends-on-essential-package-without-using-version Depends: gzip
> >
> > the current minor version available in bullseye was set for gzip.
> >
>
> Since I didn't get that error, I'm interested: what command are you
> building with? Why the version for Bullseye?
it's wrong in any case - gzip is Essential (installed on all Debian systems). you don't ever need a dependency (build or otherwise) for it. this is what lintian is trying to tell you - it's wrong to depend on gzip without a version constraint (and okay to use it without a dependency). *only* if you have a certain version requirement you need to add the dependency (with the proper version qualifier) to be able to encode that version requirement.
> I guess many systems already have bzip2 installed, but do we really want
> to require it for everybody? Or should we rather keep it optional (could
> be a Recommends or Suggests dependency) and add a clean error if not
> installed?
I am not sure about *how* many (most dev machines yes, since it's build-essential and required by a lot of build-related packages ;)). it's only 124kB though, and libbz2-1.0 is always installed (it's transitively Essential via dpkg), so it doesn't pull in anything that isn't installed on all standard Debian-based systems anyway since it only depends on that and libc6. I don't really see a reason to use Recommends or Suggests here, even if bz2 compress isos are not that common.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage v3 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd
2024-09-10 12:52 ` Fabian Grünbichler
@ 2024-09-10 13:04 ` Fiona Ebner
0 siblings, 0 replies; 6+ messages in thread
From: Fiona Ebner @ 2024-09-10 13:04 UTC (permalink / raw)
To: Fabian Grünbichler, Proxmox VE development discussion,
Maximiliano Sandoval
Am 10.09.24 um 14:52 schrieb Fabian Grünbichler:
>> Fiona Ebner <f.ebner@proxmox.com> hat am 10.09.2024 14:24 CEST geschrieben:
>> Am 12.08.24 um 13:44 schrieb Maximiliano Sandoval:
>>> The decompressor_info method calls binaries provided by these packages
>>> so they are (alphabetically) added explicitly as dependencies.
>>>
>>> To avoid a build-time error
>>>
>>> E: libpve-storage-perl: depends-on-essential-package-without-using-version Depends: gzip
>>>
>>> the current minor version available in bullseye was set for gzip.
>>>
>>
>> Since I didn't get that error, I'm interested: what command are you
>> building with? Why the version for Bullseye?
>
> it's wrong in any case - gzip is Essential (installed on all Debian systems). you don't ever need a dependency (build or otherwise) for it. this is what lintian is trying to tell you - it's wrong to depend on gzip without a version constraint (and okay to use it without a dependency). *only* if you have a certain version requirement you need to add the dependency (with the proper version qualifier) to be able to encode that version requirement.
>
I see, thank you for the explanation!
>> I guess many systems already have bzip2 installed, but do we really want
>> to require it for everybody? Or should we rather keep it optional (could
>> be a Recommends or Suggests dependency) and add a clean error if not
>> installed?
>
> I am not sure about *how* many (most dev machines yes, since it's build-essential and required by a lot of build-related packages ;)). it's only 124kB though, and libbz2-1.0 is always installed (it's transitively Essential via dpkg), so it doesn't pull in anything that isn't installed on all standard Debian-based systems anyway since it only depends on that and libc6. I don't really see a reason to use Recommends or Suggests here, even if bz2 compress isos are not that common.
Okay :)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-10 13:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-12 11:44 [pve-devel] [PATCH storage v3 1/2] fix #5267: storage: add bzip2 support Maximiliano Sandoval
2024-08-12 11:44 ` [pve-devel] [PATCH storage v3 2/2] d/control: dependencies: add bzip2, gzip, lzop, zstd Maximiliano Sandoval
2024-09-10 12:24 ` Fiona Ebner
2024-09-10 12:52 ` Fabian Grünbichler
2024-09-10 13:04 ` Fiona Ebner
2024-09-10 12:24 ` [pve-devel] [PATCH storage v3 1/2] fix #5267: storage: add bzip2 support Fiona Ebner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox