Re: [pve-devel] [PATCH v4 storage 4/6] storage: introduce decompress_archive_into_pipe helper

[Daniel Kral <d.kral@proxmox.com>]

On 9/18/24 16:49, Filip Schauer wrote:
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>

Great cleanup! It would only be helpful to have a little more context 
information what has been done here. As far as I can tell, there are no 
functional changes (except that the newly introduced subroutine will die 
in case of an uncompressed archive) but just making the code more 
readable and adhere to the Perl Style Guide.

> ---
>   src/PVE/Storage.pm | 64 +++++++++++++++++++++++++++++-----------------
>   1 file changed, 40 insertions(+), 24 deletions(-)
> 
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index 12f7b3f..e5f5326 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -1682,6 +1682,45 @@ sub extract_vzdump_config_tar {
>       return wantarray ? ($raw, $file) : $raw;
>   }
>   
> +sub decompress_archive_into_pipe {
> +    my ($archive, $cmd, $outfunc) = @_;
> +
> +    my $info = archive_info($archive);
> +    die "archive is not compressed\n" if !$info->{compression};
> +    my $decompressor = $info->{decompressor};
> +    my $full_cmd = [ [@$decompressor, $archive], $cmd ];
> +
> +    # lzop/zcat exits with 1 when the pipe is closed early,
> +    # detect this and ignore the exit code later
> +    my $broken_pipe;
> +    my $errstring;
> +    my $err = sub {
> +	my $output = shift;
> +	if (
> +	    $output =~ m/lzop: Broken pipe: <stdout>/
> +	    || $output =~ m/gzip: stdout: Broken pipe/
> +	    || $output =~ m/zstd: error 70 : Write error.*Broken pipe/
> +	) {
> +	    $broken_pipe = 1;
> +	} elsif (!defined ($errstring) && $output !~ m/^\s*$/) {
> +	    $errstring = "failed to decompress archive: $output\n";
> +	}
> +    };
> +
> +    my $rc = eval { run_command($full_cmd, outfunc => $outfunc, errfunc => $err, noerr => 1) };
> +    my $rerr = $@;
> +
> +    $broken_pipe ||= $rc == 141; # broken pipe from cmd POV
> +
> +    if (!$errstring && !$broken_pipe && $rc != 0) {
> +	die "$rerr\n" if $rerr;
> +	die "archive decompression failed with exit code $rc\n";
> +    }
> +    die "$errstring\n" if $errstring;
> +
> +    return;
> +}
> +
>   sub extract_vzdump_config_vma {
>       my ($archive, $comp) = @_;
>   
> @@ -1693,30 +1732,7 @@ sub extract_vzdump_config_vma {
>       my $decompressor = $info->{decompressor};
>   
>       if ($comp) {
> -	my $cmd = [ [@$decompressor, $archive], ["vma", "config", "-"] ];
> -
> -	# lzop/zcat exits with 1 when the pipe is closed early by vma, detect this and ignore the exit code later
> -	my $broken_pipe;
> -	my $errstring;
> -	my $err = sub {
> -	    my $output = shift;
> -	    if ($output =~ m/lzop: Broken pipe: <stdout>/ || $output =~ m/gzip: stdout: Broken pipe/ || $output =~ m/zstd: error 70 : Write error.*Broken pipe/) {
> -		$broken_pipe = 1;
> -	    } elsif (!defined ($errstring) && $output !~ m/^\s*$/) {
> -		$errstring = "Failed to extract config from VMA archive: $output\n";
> -	    }
> -	};
> -
> -	my $rc = eval { run_command($cmd, outfunc => $out, errfunc => $err, noerr => 1) };
> -	my $rerr = $@;
> -
> -	$broken_pipe ||= $rc == 141; # broken pipe from vma POV
> -
> -	if (!$errstring && !$broken_pipe && $rc != 0) {
> -	    die "$rerr\n" if $rerr;
> -	    die "config extraction failed with exit code $rc\n";
> -	}
> -	die "$errstring\n" if $errstring;
> +	decompress_archive_into_pipe($archive, ["vma", "config", "-"], $out);
>       } else {
>   	run_command(["vma", "config", $archive], outfunc => $out);
>       }

Reviewed-by: Daniel Kral <d.kral@proxmox.com>


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