[pve-devel] applied: [PATCH v4 storage 1/4] status: move unlink from http-server to enpoint

[Thomas Lamprecht]

meh, only noticed the s/enpoint/endpoint/ spelling error in the subject after
pushing it out..

On 31.08.21 12:16, Lorenz Stechauner wrote:
> this is the first step in which not the http server removes the
> temporary file, but the worker itself.
> 
> Signed-off-by: Lorenz Stechauner <l.stechauner at proxmox.com>
> ---
>  PVE/API2/Storage/Status.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index b838461..cc8ed5a 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -486,6 +486,12 @@ __PACKAGE__->register_method ({
>  	    print "command: " . join(' ', @$cmd) . "\n";
>  
>  	    eval { run_command($cmd, errmsg => 'import failed'); };
> +
> +	    # unlinks only the temporary file from the http server

this is just describing what I can read from the code below anyway? And it implies
to me that it removes the file from some directory a http server serves. I adapted
this trying to give a slightly better hint for a "why?".

"the temporary file got only uploaded locally, no need to rm remote"

> +	    unlink $tmpfilename;
> +	    warn "unable to clean up temporary file '$tmpfilename' - $!\n"
> +		if $! && $! != ENOENT;

nit: I used a single line in the follow-up as I touched the area anyway (wouldn't
have cared to much else). I also updated the style guide wiki slightly to reflect
that post-if's are most often fine to use the upper limit of 100cc text-width as
their main use is short/concise error handling anyway. But as said more of a nit
from my side.

> +
>  	    if (my $err = $@) {
>  		eval { $err_cleanup->() };
>  		warn "$@" if $@;
>