* [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
@ 2023-09-11 13:56 Philipp Hufnagl
2023-09-11 13:56 ` [pve-devel] [PATCH manager v7 1/2] fix #4849: api: download to storage: automatically dectect and configure compression Philipp Hufnagl
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Philipp Hufnagl @ 2023-09-11 13:56 UTC (permalink / raw)
To: pve-devel
Many web pages offer the download of the disk images compressed. This
patch allows the download of archives (like .gz), automatically detects
the format and decompresses it
Changes since v6:
* remove detect-compression API parameter
* add compression field with cbind
Changes since v5:
* split manager patch into frontend/backend
* remove not needed regex group
Changes since v4:
* add commit messages
* fix nit
Changes since v3:
* generate compression regex from compression list
* fix logic errors
Changes since v2:
* move compression code to the download function in common
* minor code improvements
Changes since v1:
* Improve code quality as suggested by feedback
Philipp Hufnagl (2):
fix #4849: api: download to storage: automatically dectect and
configure compression
fix #4849: ui: download to storage: automatically dectect and
configure compression
PVE/API2/Nodes.pm | 15 ++++++++++++++-
www/manager6/Makefile | 1 +
www/manager6/form/DecompressionSelector.js | 13 +++++++++++++
www/manager6/window/DownloadUrlToStorage.js | 13 ++++++++++++-
4 files changed, 40 insertions(+), 2 deletions(-)
create mode 100644 www/manager6/form/DecompressionSelector.js
--
2.39.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager v7 1/2] fix #4849: api: download to storage: automatically dectect and configure compression
2023-09-11 13:56 [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs Philipp Hufnagl
@ 2023-09-11 13:56 ` Philipp Hufnagl
2023-09-11 13:56 ` [pve-devel] [PATCH manager v7 2/2] fix #4849: ui: " Philipp Hufnagl
2023-09-20 11:07 ` [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs Dominik Csapak
2 siblings, 0 replies; 9+ messages in thread
From: Philipp Hufnagl @ 2023-09-11 13:56 UTC (permalink / raw)
To: pve-devel
extends the query_url_metadata callback with the functionallity to
detect used compressions. If a compression is used it tells the ui which
one
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
---
PVE/API2/Nodes.pm | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 5a148d1d..8d059440 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -34,6 +34,7 @@ use PVE::RRD;
use PVE::Report;
use PVE::SafeSyslog;
use PVE::Storage;
+use PVE::Storage::Plugin;
use PVE::Tools;
use PVE::pvecfg;
@@ -1564,7 +1565,7 @@ __PACKAGE__->register_method({
type => 'boolean',
optional => 1,
default => 1,
- }
+ },
},
},
returns => {
@@ -1583,6 +1584,11 @@ __PACKAGE__->register_method({
type => 'string',
optional => 1,
},
+ compression => {
+ type => 'string',
+ enum => $PVE::Storage::Plugin::KNOWN_COMPRESSION_FORMATS,
+ optional => 1,
+ },
},
},
code => sub {
@@ -1615,6 +1621,7 @@ __PACKAGE__->register_method({
my $disposition = $res->header("Content-Disposition");
my $type = $res->header("Content-Type");
+ my $compression;
my $filename;
if ($disposition && ($disposition =~ m/filename="([^"]*)"/ || $disposition =~ m/filename=([^;]*)/)) {
@@ -1628,10 +1635,16 @@ __PACKAGE__->register_method({
$type = $1;
}
+ if ($filename =~ m!^(.+)\.(${\PVE::Storage::Plugin::COMPRESSOR_RE})$!) {
+ $filename = $1;
+ $compression = $2;
+ }
+
my $ret = {};
$ret->{filename} = $filename if $filename;
$ret->{size} = $size + 0 if $size;
$ret->{mimetype} = $type if $type;
+ $ret->{compression} = $compression if $compression;
return $ret;
}});
--
2.39.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager v7 2/2] fix #4849: ui: download to storage: automatically dectect and configure compression
2023-09-11 13:56 [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs Philipp Hufnagl
2023-09-11 13:56 ` [pve-devel] [PATCH manager v7 1/2] fix #4849: api: download to storage: automatically dectect and configure compression Philipp Hufnagl
@ 2023-09-11 13:56 ` Philipp Hufnagl
2023-09-20 11:07 ` [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs Dominik Csapak
2 siblings, 0 replies; 9+ messages in thread
From: Philipp Hufnagl @ 2023-09-11 13:56 UTC (permalink / raw)
To: pve-devel
extends the download iso prompt with a "compression algorithm" drop down
under advanced. User can configure there if a decompression algorithm
should be used from the storage backend. The compression algorithm will
be automatically guessed when calling query_url_metadata
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
---
www/manager6/Makefile | 1 +
www/manager6/form/DecompressionSelector.js | 13 +++++++++++++
www/manager6/window/DownloadUrlToStorage.js | 13 ++++++++++++-
3 files changed, 26 insertions(+), 1 deletion(-)
create mode 100644 www/manager6/form/DecompressionSelector.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 59a5d8a7..5dfc8215 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -34,6 +34,7 @@ JSSRC= \
form/ContentTypeSelector.js \
form/ControllerSelector.js \
form/DayOfWeekSelector.js \
+ form/DecompressionSelector.js \
form/DiskFormatSelector.js \
form/DiskStorageSelector.js \
form/FileSelector.js \
diff --git a/www/manager6/form/DecompressionSelector.js b/www/manager6/form/DecompressionSelector.js
new file mode 100644
index 00000000..abd19316
--- /dev/null
+++ b/www/manager6/form/DecompressionSelector.js
@@ -0,0 +1,13 @@
+Ext.define('PVE.form.DecompressionSelector', {
+ extend: 'Proxmox.form.KVComboBox',
+ alias: ['widget.pveDecompressionSelector'],
+ config: {
+ deleteEmpty: false,
+ },
+ comboItems: [
+ ['__default__', Proxmox.Utils.NoneText],
+ ['lzo', 'LZO'],
+ ['gz', 'GZIP'],
+ ['zst', 'ZSTD'],
+ ],
+});
diff --git a/www/manager6/window/DownloadUrlToStorage.js b/www/manager6/window/DownloadUrlToStorage.js
index 90320da4..08a6412c 100644
--- a/www/manager6/window/DownloadUrlToStorage.js
+++ b/www/manager6/window/DownloadUrlToStorage.js
@@ -84,6 +84,7 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
filename: data.filename || "",
size: (data.size && Proxmox.Utils.format_size(data.size)) || gettext("Unknown"),
mimetype: data.mimetype || gettext("Unknown"),
+ compression: data.compression || '__default__',
});
},
});
@@ -203,6 +204,17 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
change: 'setQueryEnabled',
},
},
+ {
+ xtype: 'pveDecompressionSelector',
+ name: 'compression',
+ fieldLabel: gettext('Decompression algorithm'),
+ allowBlank: true,
+ hasNoneOption: true,
+ value: '__default__',
+ cbind: {
+ hidden: get => get('content') !== 'iso',
+ },
+ },
],
},
{
@@ -223,7 +235,6 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
if (!me.storage) {
throw "no storage ID specified";
}
-
me.callParent();
},
});
--
2.39.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
2023-09-11 13:56 [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs Philipp Hufnagl
2023-09-11 13:56 ` [pve-devel] [PATCH manager v7 1/2] fix #4849: api: download to storage: automatically dectect and configure compression Philipp Hufnagl
2023-09-11 13:56 ` [pve-devel] [PATCH manager v7 2/2] fix #4849: ui: " Philipp Hufnagl
@ 2023-09-20 11:07 ` Dominik Csapak
2023-09-20 11:46 ` Fabian Grünbichler
2 siblings, 1 reply; 9+ messages in thread
From: Dominik Csapak @ 2023-09-20 11:07 UTC (permalink / raw)
To: Proxmox VE development discussion, Philipp Hufnagl
LGTM and works as advertised.
Needs pve-storage >= 8.0.3 (for compression parameter)
consider both patches:
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
2023-09-20 11:07 ` [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs Dominik Csapak
@ 2023-09-20 11:46 ` Fabian Grünbichler
2023-09-20 11:50 ` Dominik Csapak
0 siblings, 1 reply; 9+ messages in thread
From: Fabian Grünbichler @ 2023-09-20 11:46 UTC (permalink / raw)
To: Philipp Hufnagl, Proxmox VE development discussion
On September 20, 2023 1:07 pm, Dominik Csapak wrote:
> LGTM and works as advertised.
it breaks downloading container templates that are compressed with one
of the "known" compression algorithms (such as gz).
probably the detect-compression parameter and handling needs to go back
in (that was the reason it was there in the first place!), or some other
solution needs to be found..
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
2023-09-20 11:46 ` Fabian Grünbichler
@ 2023-09-20 11:50 ` Dominik Csapak
2023-09-20 11:54 ` Dominik Csapak
2023-09-20 12:09 ` Fabian Grünbichler
0 siblings, 2 replies; 9+ messages in thread
From: Dominik Csapak @ 2023-09-20 11:50 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler,
Philipp Hufnagl
On 9/20/23 13:46, Fabian Grünbichler wrote:
> On September 20, 2023 1:07 pm, Dominik Csapak wrote:
>> LGTM and works as advertised.
>
> it breaks downloading container templates that are compressed with one
> of the "known" compression algorithms (such as gz).
>
> probably the detect-compression parameter and handling needs to go back
> in (that was the reason it was there in the first place!), or some other
> solution needs to be found..
>
>
ah yes ofc, sorry for the oversight
couldn't we simply check in the backend for the download for the content type?
as we only really need to unpack isos?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
2023-09-20 11:50 ` Dominik Csapak
@ 2023-09-20 11:54 ` Dominik Csapak
2023-09-20 12:09 ` Fabian Grünbichler
1 sibling, 0 replies; 9+ messages in thread
From: Dominik Csapak @ 2023-09-20 11:54 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler,
Philipp Hufnagl
On 9/20/23 13:50, Dominik Csapak wrote:
> On 9/20/23 13:46, Fabian Grünbichler wrote:
>> On September 20, 2023 1:07 pm, Dominik Csapak wrote:
>>> LGTM and works as advertised.
>>
>> it breaks downloading container templates that are compressed with one
>> of the "known" compression algorithms (such as gz).
>>
>> probably the detect-compression parameter and handling needs to go back
>> in (that was the reason it was there in the first place!), or some other
>> solution needs to be found..
>>
>>
>
> ah yes ofc, sorry for the oversight
>
> couldn't we simply check in the backend for the download for the content type?
> as we only really need to unpack isos?
>
>
ah no that also does not work.
so you're right, having a parameter that controls this is probably best
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
2023-09-20 11:50 ` Dominik Csapak
2023-09-20 11:54 ` Dominik Csapak
@ 2023-09-20 12:09 ` Fabian Grünbichler
2023-09-20 13:12 ` Philipp Hufnagl
1 sibling, 1 reply; 9+ messages in thread
From: Fabian Grünbichler @ 2023-09-20 12:09 UTC (permalink / raw)
To: Dominik Csapak, Philipp Hufnagl, Proxmox VE development discussion
On September 20, 2023 1:50 pm, Dominik Csapak wrote:
> On 9/20/23 13:46, Fabian Grünbichler wrote:
>> On September 20, 2023 1:07 pm, Dominik Csapak wrote:
>>> LGTM and works as advertised.
>>
>> it breaks downloading container templates that are compressed with one
>> of the "known" compression algorithms (such as gz).
>>
>> probably the detect-compression parameter and handling needs to go back
>> in (that was the reason it was there in the first place!), or some other
>> solution needs to be found..
>>
>>
>
> ah yes ofc, sorry for the oversight
>
> couldn't we simply check in the backend for the download for the content type?
> as we only really need to unpack isos?
the "query url" part doesn't know about (storage) content types. and it
returns the file name, so we can't let it detect compression but throw
that part away, else we get the uncompressed filename instead of the
compressed one (exactly what happens with v7 now).
that's why we originally made the client/GUI make the choice:
iso download dialogue:
- query url with compression support
- allow overriding (de)compression
- pass (de)compression to download if set
other download dialogues (currently only templates):
- query url without compression support
- don't offer (de)compression choice
- (de)compression is never set, thus never passed to download
in addition, the download backend (which knows about content types) also
only allows decompression for isos (at least for the time being, if we
ever revisit and allow plain container template archives then all of
this is moot anyway ;))
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs
2023-09-20 12:09 ` Fabian Grünbichler
@ 2023-09-20 13:12 ` Philipp Hufnagl
0 siblings, 0 replies; 9+ messages in thread
From: Philipp Hufnagl @ 2023-09-20 13:12 UTC (permalink / raw)
To: Fabian Grünbichler, Dominik Csapak,
Proxmox VE development discussion
On 9/20/23 14:09, Fabian Grünbichler wrote:
> On September 20, 2023 1:50 pm, Dominik Csapak wrote:
>> On 9/20/23 13:46, Fabian Grünbichler wrote:
>>> On September 20, 2023 1:07 pm, Dominik Csapak wrote:
>>>> LGTM and works as advertised.
>>> it breaks downloading container templates that are compressed with one
>>> of the "known" compression algorithms (such as gz).
>>>
>>> probably the detect-compression parameter and handling needs to go back
>>> in (that was the reason it was there in the first place!), or some other
>>> solution needs to be found..
>>>
>>>
>> ah yes ofc, sorry for the oversight
>>
>> couldn't we simply check in the backend for the download for the content type?
>> as we only really need to unpack isos?
> the "query url" part doesn't know about (storage) content types. and it
> returns the file name, so we can't let it detect compression but throw
> that part away, else we get the uncompressed filename instead of the
> compressed one (exactly what happens with v7 now).
>
> that's why we originally made the client/GUI make the choice:
>
> iso download dialogue:
> - query url with compression support
> - allow overriding (de)compression
> - pass (de)compression to download if set
>
> other download dialogues (currently only templates):
> - query url without compression support
> - don't offer (de)compression choice
> - (de)compression is never set, thus never passed to download
>
> in addition, the download backend (which knows about content types) also
> only allows decompression for isos (at least for the time being, if we
> ever revisit and allow plain container template archives then all of
> this is moot anyway ;))
Thank you for reviewing this! I will make a v8 very soon featuring
detect_compression again!
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-09-20 13:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-11 13:56 [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs Philipp Hufnagl
2023-09-11 13:56 ` [pve-devel] [PATCH manager v7 1/2] fix #4849: api: download to storage: automatically dectect and configure compression Philipp Hufnagl
2023-09-11 13:56 ` [pve-devel] [PATCH manager v7 2/2] fix #4849: ui: " Philipp Hufnagl
2023-09-20 11:07 ` [pve-devel] [PATCH manager v7 0/2] fix #4849: allow download of compressed ISOs Dominik Csapak
2023-09-20 11:46 ` Fabian Grünbichler
2023-09-20 11:50 ` Dominik Csapak
2023-09-20 11:54 ` Dominik Csapak
2023-09-20 12:09 ` Fabian Grünbichler
2023-09-20 13:12 ` Philipp Hufnagl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox