From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 647DA77EB6 for ; Wed, 21 Jul 2021 16:51:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 590F61704E for ; Wed, 21 Jul 2021 16:51:59 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 3A65817047 for ; Wed, 21 Jul 2021 16:51:58 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 0AF14423B6 for ; Wed, 21 Jul 2021 16:51:58 +0200 (CEST) Message-ID: <9bca870e-c601-3905-fb14-f3b6acef1385@proxmox.com> Date: Wed, 21 Jul 2021 16:51:31 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.0 Content-Language: en-US To: Proxmox VE development discussion , Lorenz Stechauner References: <20210721125217.3734806-1-l.stechauner@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20210721125217.3734806-1-l.stechauner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.412 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [status.pm] Subject: Re: [pve-devel] [PATCH storage] status: add unlink to worker in file upload X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Jul 2021 14:51:59 -0000 On 21.07.21 14:52, Lorenz Stechauner wrote: > this is the first step in which not the http server removes the > temporary file, but the worker itself. for now, this `unlink` > statement won't unlink anything existing. but after changes to > the http server have been made, the endpoint will be able to > clean up for itself afterwards. > > Signed-off-by: Lorenz Stechauner > --- > a version bump would be helpful, so the older versions can be > marked as 'breaks' in the pve-http-server repo after the above > mentioned changes. > You do not necesarrily need to send it out as its own patch, IMO this would be still benefit from being grouped in a series with the other changes that are related to the overall thing you want to achieve. As mentioned in the review, it's totally fine if you do not actually do the breaks, but you should be aware of the need for it and also mention it to ensure a maintainer does not forgets on actually applying + bumping. > PVE/API2/Storage/Status.pm | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm > index 72fd851..9275d1f 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; error handling still missing for the unlink. As unlink returns the files actually unlinked (it accepts a list) it's easy to do for the single-file case: unlink $tmpfilename or warn "bla - $!\n"; > if (my $err = $@) { > unlink $dest; > die $err; >