[pve-devel] [PATCH storage] Fix #582: Make removing backups a background task

[Dominik Csapak]

i would prefer that it behaves like the update_vm_api sub in
PVE/API2/Qemu.pm (line ~1000)

there we have a parameter 'background_delay' which we could use to
make the removal async (this can already work in the gui)
and the return value is type string but optional, so
you could return undef without violating our return validator

On 5/2/19 11:42 AM, Dominic Jäger wrote:
> Previously, the web gui timed out when removing a
> backup took long. Doing the main part of the API
>   DELETE call in a fork_worker solves this.
> 
> Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
> ---
> Thomas suggested returning undef on bugzilla. Returning "" is what seemed
> closest to this while still being of return type 'string' to me.
> 
>   PVE/API2/Storage/Content.pm | 32 +++++++++++++++++++++-----------
>   1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
> index cd4746b..eb9c191 100644
> --- a/PVE/API2/Storage/Content.pm
> +++ b/PVE/API2/Storage/Content.pm
> @@ -287,7 +287,7 @@ __PACKAGE__->register_method ({
>   	    },
>   	},
>       },
> -    returns => { type => 'null' },
> +    returns => { type => 'string' },
>       code => sub {
>   	my ($param) = @_;
>   
> @@ -306,16 +306,26 @@ __PACKAGE__->register_method ({
>   	    $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate']);
>   	}
>   
> -	PVE::Storage::vdisk_free ($cfg, $volid);
> -
> -	if ($vtype eq 'backup'
> -	    && $path =~ /(.*\/vzdump-\w+-\d+-\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})[^\/]+$/) {
> -	    my $logpath = "$1.log";
> -	    # try to cleanup our backup log file too, if still exisiting, #318
> -	    unlink($logpath) if -e $logpath;
> -	}
> -
> -	return undef;
> +	my $worker = sub {
> +	    PVE::Storage::vdisk_free ($cfg, $volid);
> +	    if ($vtype eq 'backup'
> +		&& $path =~ /(.*\/vzdump-\w+-\d+-\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})[^\/]+$/) {
> +		my $logpath = "$1.log";
> +		# try to cleanup our backup log file too, if still exisiting, #318
> +		unlink($logpath) if -e $logpath;
> +	    }
> +	};
> +	my $upid = $rpcenv->fork_worker('imgdel', $ownervm, $authuser, $worker);
> +	# 5 seconds time window gives a good feed back and stays well under the
> +	# 30 seconds per api request answer timeout
> +	my $end_time = time() + 5;
> +	my $task = PVE::Tools::upid_decode($upid);
> +	my $still_deleting; # not necessarily true, e.g. sequential api call from cli
> +	do {
> +	    $still_deleting = PVE::ProcFSTools::check_process_running($task->{pid}, $task->{pstart});
> +	    $still_deleting ? sleep 1 : last;
> +	} while (time() < $end_time);
> +	return $still_deleting ? $upid : "";
>       }});
>   
>   __PACKAGE__->register_method ({
>