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 22CBA73A9F for ; Fri, 18 Jun 2021 18:39:39 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 134A6ADF5 for ; Fri, 18 Jun 2021 18:39:39 +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 7269FADE7 for ; Fri, 18 Jun 2021 18:39:38 +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 3CF5144244 for ; Fri, 18 Jun 2021 18:39:38 +0200 (CEST) Message-ID: <071fd22d-957d-c534-c554-5feb97d753c9@proxmox.com> Date: Fri, 18 Jun 2021 18:39:31 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:90.0) Gecko/20100101 Thunderbird/90.0 Content-Language: en-US To: Proxmox VE development discussion , Lorenz Stechauner References: <20210616093604.33668-1-l.stechauner@proxmox.com> <20210616093604.33668-6-l.stechauner@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20210616093604.33668-6-l.stechauner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.893 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.254 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. [nodes.pm] Subject: Re: [pve-devel] [PATCH v9 manager 2/5] api: nodes: refactor aplinfo to use common download function 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: Fri, 18 Jun 2021 16:39:39 -0000 On 16.06.21 11:36, Lorenz Stechauner wrote: > a common function to download arbitrary files from urls has been > defined as PVE::Tools::download_file_from_url and is now used. > > Signed-off-by: Lorenz Stechauner > --- > PVE/API2/Nodes.pm | 93 +++++++++-------------------------------------- > 1 file changed, 17 insertions(+), 76 deletions(-) > > diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm > index 77fa710a..740547e7 100644 > --- a/PVE/API2/Nodes.pm > +++ b/PVE/API2/Nodes.pm > @@ -1513,88 +1513,29 @@ __PACKAGE__->register_method({ > my $src = $pd->{location}; > my $tmpldir = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage}); > my $dest = "$tmpldir/$template"; > - my $tmpdest = "$tmpldir/${template}.tmp.$$"; > > - my $worker = sub { > - my $upid = shift; > - > - print "starting template download from: $src\n"; > - print "target file: $dest\n"; > - > - my $check_hash = sub { > - my ($template_info, $filename, $noerr) = @_; > - > - my $digest; > - my $expected; > - > - eval { > - open(my $fh, '<', $filename) or die "Can't open '$filename': $!"; > - binmode($fh); > - if (defined($template_info->{sha512sum})) { > - $expected = $template_info->{sha512sum}; > - $digest = Digest::SHA->new(512)->addfile($fh)->hexdigest; > - } elsif (defined($template_info->{md5sum})) { > - #fallback to MD5 > - $expected = $template_info->{md5sum}; > - $digest = Digest::MD5->new->addfile($fh)->hexdigest; > - } else { > - die "no expected checksum defined"; > - } > - close($fh); > - }; > - > - die "checking hash failed - $@\n" if $@ && !$noerr; > - > - return ($digest, $digest ? lc($digest) eq lc($expected) : 0); > - }; > - > - eval { > - if (-f $dest) { > - my ($hash, $correct) = &$check_hash($pd, $dest, 1); > - > - if ($hash && $correct) { > - print "file already exists $hash - no need to download\n"; > - return; > - } > - } > - > - local %ENV; > - my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg'); > - if ($dccfg->{http_proxy}) { > - $ENV{http_proxy} = $dccfg->{http_proxy}; > - } > - > - my @cmd = ('/usr/bin/wget', '--progress=dot:mega', '-O', $tmpdest, $src); > - if (system (@cmd) != 0) { > - die "download failed - $!\n"; > - } > - > - my ($hash, $correct) = &$check_hash($pd, $tmpdest); > - > - die "could not calculate checksum\n" if !$hash; > - > - if (!$correct) { > - my $expected = $pd->{sha512sum} // $pd->{md5sum}; > - die "wrong checksum: $hash != $expected\n"; > - } > + my $opts = { > + hash_required => 1, > + sha512sum => $pd->{sha512sum}, > + md5sum => $pd->{md5sum}, > + }; > > - if (!rename($tmpdest, $dest)) { > - die "unable to save file - $!\n"; > - } > - }; > - my $err = $@; > + my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg'); > + if ($dccfg->{http_proxy}) { > + $opts->{http_proxy} = $dccfg->{http_proxy}; > + } > > - unlink $tmpdest; > + my $worker = sub { > + my $upid = shift; > + PVE::Tools::download_file_from_url($dest, $src, $opts); > + }; > > - if ($err) { > - print "\n"; > - die $err if $err; > - } > + my $rpcenv = PVE::RPCEnvironment::get(); > + my $user = $rpcenv->get_user(); those two where already defined and so you re-defined them here, which, besides doing extra and needless work, gives one a warning every time this module is used, e.g., on autocomplete of pveam...