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 ECB8B71B78 for ; Wed, 30 Jun 2021 08:24:38 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E046A1645F for ; Wed, 30 Jun 2021 08:24:38 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id DBED616454 for ; Wed, 30 Jun 2021 08:24:37 +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 AC8BE42AA7 for ; Wed, 30 Jun 2021 08:24:37 +0200 (CEST) Date: Wed, 30 Jun 2021 08:24:29 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20210629135316.2095374-1-l.stechauner@proxmox.com> In-Reply-To: <20210629135316.2095374-1-l.stechauner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1625033296.evhd3a6g3l.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.584 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 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. [proxmox.com, create.pm] Subject: Re: [pve-devel] [PATCH container] fix #3478: abort container creation on arch detection timeout 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, 30 Jun 2021 06:24:39 -0000 On June 29, 2021 3:53 pm, Lorenz Stechauner wrote: > increased the timeout for detect_arch from 5 to 10 seconds. >=20 > until now, on any error detect_architecture would fall back to amd64. > to avoid falling back due to an timeout error this function now dies > on timeout errors. >=20 > additionally minor changes to the error messages have been made. >=20 > Signed-off-by: Lorenz Stechauner > --- > src/PVE/LXC/Create.pm | 27 ++++++++++++++++++++++++--- > 1 file changed, 24 insertions(+), 3 deletions(-) >=20 > diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm > index 82d7ad9..0260578 100644 > --- a/src/PVE/LXC/Create.pm > +++ b/src/PVE/LXC/Create.pm > @@ -52,10 +52,31 @@ sub detect_architecture { > return $arch; > }; > =20 > - my $arch =3D eval { PVE::Tools::run_fork_with_timeout(5, $detect_arc= h) }; > - if (my $err =3D $@) { > + my $arch; > + my $status =3D 'error'; > + eval { > + $arch =3D PVE::Tools::run_fork_with_timeout(10, $detect_arch); > + if (!defined($arch)) { > + if ($@) { > + $status =3D 'timeout'; > + die $@; > + } else { > + $status =3D 'error'; > + die "unknown error\n"; > + } > + } > + $status =3D 'success'; > + }; I'd structure this this differently (the unknown error should not be=20 possible to reach, and this makes it more complicated than necessary=20 IMHO): my $arch =3D eval { run... }; now if $@ is set, $detect_arch died and we have an error (and can do the=20 fallback, like before). if $@ is not set, but $arch is undef, we know we've hit the timeout (as=20 run_fork_with_timeout does not die when it runs into a timeout, but=20 returns no result and prints a 'got timeout' warning). finally, if $@ is not set, but $arch is defined, architecture detection=20 worked. you can of course keep $@ as $err and re-order the conditions to have if (no $arch and no $err) { die # timeout } elsif ($err) { # fallback } else { # ok } > + > + my $err =3D $@; > + if ($status eq 'timeout') { > + # on timeout > + die "Architecture detection failed: $err"; # $err ends with \n > + } elsif ($status eq 'error') { > + # any other error > $arch =3D 'amd64'; > - print "Architecture detection failed: $err\nFalling back to amd64.\n" . > + print "Architecture detection failed: $err" . # $err ends with \n > + "Falling back to $arch.\n" . > "Use `pct set VMID --arch ARCH` to change.\n"; > } else { > print "Detected container architecture: $arch\n"; > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20