* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.