[pve-devel] [PATCH manager v3] Fix #2124: Add support for zstd

[Fabian Ebner]

Two nits inline.

On 08.04.20 12:25, Alwin Antreich wrote:
> This patch adds zstd to the compression selection for backup on the GUI
> and add .zst to the backup file filter. Including zstd as package
> install dependency.
> 
> Signed-off-by: Alwin Antreich <a.antreich at proxmox.com>
> ---
>   PVE/VZDump.pm                            | 11 +++++++++--
>   debian/control                           |  1 +
>   www/manager6/form/CompressionSelector.js |  3 ++-
>   3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index f3274196..e97bd817 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -609,6 +609,13 @@ sub compressor_info {
>   	} else {
>   	    return ('gzip --rsyncable', 'gz');
>   	}
> +    } elsif ($opt_compress eq 'zstd') {
> +	    my $zstd_threads = $opts->{zstd} // 1;
> +	if ($zstd_threads == 0) {
> +	    my $cpuinfo = PVE::ProcFSTools::read_cpuinfo();
> +	    $zstd_threads = int(($cpuinfo->{cpus} + 1)/2);
> +	}
> +	    return ("zstd --threads=${zstd_threads}", 'zst');
>       } else {

Wrong indentation in the block above.

>   	die "internal error - unknown compression option '$opt_compress'";
>       }
> @@ -620,7 +627,7 @@ sub get_backup_file_list {
>       my $bklist = [];
>       foreach my $fn (<$dir/${bkname}-*>) {
>   	next if $exclude_fn && $fn eq $exclude_fn;
> -	if ($fn =~ m!/(${bkname}-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2})\.(tgz|((tar|vma)(\.(gz|lzo))?)))$!) {
> +	if ($fn =~ m!/(${bkname}-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2})\.(tgz|((tar|vma)(\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)))$!) {
>   	    $fn = "$dir/$1"; # untaint
>   	    my $t = timelocal ($7, $6, $5, $4, $3 - 1, $2);
>   	    push @$bklist, [$fn, $t];
> @@ -928,7 +935,7 @@ sub exec_backup_task {
>   		    debugmsg ('info', "delete old backup '$d->[0]'", $logfd);
>   		    unlink $d->[0];
>   		    my $logfn = $d->[0];
> -		    $logfn =~ s/\.(tgz|((tar|vma)(\.(gz|lzo))?))$/\.log/;
> +		    $logfn =~ s/\.(tgz|((tar|vma)(\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?))$/\.log/;
>   		    unlink $logfn;
>   		}
>   	    }
> diff --git a/debian/control b/debian/control
> index ec5267a4..4ba05c6f 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -60,6 +60,7 @@ Depends: apt-transport-https | apt (>= 1.5~),
>            logrotate,
>            lsb-base,
>            lzop,
> +         zstd,
>            novnc-pve,
>            pciutils,
>            perl (>= 5.10.0-19),
> diff --git a/www/manager6/form/CompressionSelector.js b/www/manager6/form/CompressionSelector.js
> index 8938fc0e..e8775e71 100644
> --- a/www/manager6/form/CompressionSelector.js
> +++ b/www/manager6/form/CompressionSelector.js
> @@ -4,6 +4,7 @@ Ext.define('PVE.form.CompressionSelector', {
>       comboItems: [
>                   ['0', Proxmox.Utils.noneText],
>                   ['lzo', 'LZO (' + gettext('fast') + ')'],
> -                ['gzip', 'GZIP (' + gettext('good') + ')']
> +                ['gzip', 'GZIP (' + gettext('good') + ')'],
> +                ['zstd', 'ZSTD (' + gettext('better') + ')']

Adding a comma after the last item will make the next patch touching 
this shorter ;). I found other places where we do that, so it should be 
fine, but I'm not too familiar with JavaScript so please correct me if 
I'm wrong.

>       ]
>   });
>