Re: [pve-devel] [PATCH storage 1/3] status: move unlink from http-server to endpoint

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 20.07.21 13:51, Lorenz Stechauner wrote:

for some odd stuff like here I'd like to see a bit more in the commit message,
when did this come in, since when can it get done in another way (and why wasn't
that way then chosen then already?). Stuff is there for a reason, dropping
something fishy like this without any (commit) comment whatsoever is not too ideal.

Besides that, would need a versioned bump on pve-http-server's package, but as we
do not depend on that directly here, IIRC, we would break old manager and bump
the versioned dependency for http-server in there (manager).

Again, not really doable in a "for sure way" on sending, but should be noted ;-)

> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>  PVE/API2/Storage/Status.pm | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index 72fd851..b549d7d 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -478,6 +478,7 @@ __PACKAGE__->register_method ({
>  	    print "command: " . join(' ', @$cmd) . "\n";
>  
>  	    eval { PVE::Tools::run_command($cmd, errmsg => 'import failed'); };
> +	    unlink $tmpfilename;
>  	    if (my $err = $@) {
>  		unlink $dest;
>  		die $err;
> @@ -485,14 +486,7 @@ __PACKAGE__->register_method ({
>  	    print "finished file import successfully\n";
>  	};
>  
> -	my $upid = $rpcenv->fork_worker('imgcopy', undef, $user, $worker);
> -
> -	# apache removes the temporary file on return, so we need
> -	# to wait here to make sure the worker process starts and
> -	# opens the file before it gets removed.
> -	sleep(1);
> -
> -	return $upid;
> +	return $rpcenv->fork_worker('imgcopy', undef, $user, $worker);
>     }});
>  
>  __PACKAGE__->register_method({
>