From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
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 4E48777569
for <pve-devel@lists.proxmox.com>; Tue, 20 Jul 2021 15:32:13 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 414B6AA68
for <pve-devel@lists.proxmox.com>; Tue, 20 Jul 2021 15:32:13 +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 2039AAA58
for <pve-devel@lists.proxmox.com>; Tue, 20 Jul 2021 15:32:12 +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 E390140579
for <pve-devel@lists.proxmox.com>; Tue, 20 Jul 2021 15:32:11 +0200 (CEST)
Message-ID: <105cc2a7-3860-a782-fc30-828cbbdffbcf@proxmox.com>
Date: Tue, 20 Jul 2021 15:31:45 +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 <pve-devel@lists.proxmox.com>,
Lorenz Stechauner <l.stechauner@proxmox.com>
References: <20210720115147.1988565-1-l.stechauner@proxmox.com>
<20210720115147.1988565-3-l.stechauner@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
In-Reply-To: <20210720115147.1988565-3-l.stechauner@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.420 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 1/3] status: move unlink from
http-server to endpoint
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 20 Jul 2021 13:32:13 -0000
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({
>