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 B5D6270578 for ; Tue, 21 Jun 2022 17:52:39 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A4A057799 for ; Tue, 21 Jun 2022 17:52:09 +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 4BF82778B for ; Tue, 21 Jun 2022 17:52:08 +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 1904643C23 for ; Tue, 21 Jun 2022 17:52:08 +0200 (CEST) Message-ID: <9ddd6d92-5442-7d4b-f21a-6605f8a6ff95@proxmox.com> Date: Tue, 21 Jun 2022 17:52:07 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.0 Content-Language: en-GB To: Proxmox VE development discussion , Daniel Tschlatscher References: <20220621152026.496514-1-d.tschlatscher@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20220621152026.496514-1-d.tschlatscher@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.004 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [qemuserver.pm, helpers.pm, qemu.pm] Subject: Re: [pve-devel] [PATCH qemu-server 1/2] fix #3502: VM start timeout config parameter 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: Tue, 21 Jun 2022 15:52:39 -0000 fix #3502: add start timeout config parameter Am 21/06/2022 um 17:20 schrieb Daniel Tschlatscher: > It was already possible to set the timeout parameter for the VM config > via the API. However, the value was not considered when the function > config_aware_timeout() was called. but you only add the timeout param now?! IOW. it was never possible to set a timeout config, but one could set the parameter for a single start. Please reword it, as of is its confusing. > Now, if the timeout parameter is set, it will override the heuristic > calculation of the VM start timeout. you mean if a timeout config property/option is set, as the API parameter was already honored in vm_start_nolock: my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $resume); > > During testing I found a problem where really big values (10^20)+ > would be converted to scientific notation, which means they no longer > pass the integer type check. To get around this, I set the maximum > value for the timeout to 2,680,000 seconds, which is around 31 days. > This I'd wager, is an upper limit in which nobody should realistically > run into. 24h is already really big enough, so please use 86000, better to go lower (but still reasonable) first, we can always easily relax it, but not really make it stricter without breaking existing setups. Also, did you test HA for the case when a really long timeout is set and triggers (by faking it with some sleep added somewhere)? > > Signed-off-by: Daniel Tschlatscher > --- > PVE/API2/Qemu.pm | 1 + > PVE/QemuServer.pm | 7 +++++++ > PVE/QemuServer/Helpers.pm | 3 +++ > 3 files changed, 11 insertions(+) > > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm > index a824657..d0a4eaa 100644 > --- a/PVE/API2/Qemu.pm > +++ b/PVE/API2/Qemu.pm > @@ -2494,6 +2494,7 @@ __PACKAGE__->register_method({ > description => "Wait maximal timeout seconds.", > type => 'integer', > minimum => 0, > + maximum => 2680000, > default => 'max(30, vm memory in GiB)', > optional => 1, > }, > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index e9aa248..81a7f6d 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -713,6 +713,13 @@ EODESCR > description => "Some (read-only) meta-information about this guest.", > optional => 1, > }, > + timeout => { "timeout what"? in the API it's clear as there it's for an specific actions, but as config options one cannot have any idea about what this timeout actually does with such an overly generic name. I'd maybe put it behind a 'start-options' format string, then it could keep the simple name and would be still telling, it would also allow to put any future startup related options in there, so not bloating config line amount up too much. start-options: timeout=12345 > + optional => 1, > + type => 'integer', > + description => 'The maximum timeout to wait for a VM to start', Please describe what is done when this isn't passed. FWIW, there's the "verbose_description" schema property for very long ones, but I think a few sentences describing config_aware_timeout could still fit in the normal "description" one. > + minimum => 0, > + maximum => 2680000, > + } > }; > > my $cicustom_fmt = { > diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm > index c10d842..c26d0dc 100644 > --- a/PVE/QemuServer/Helpers.pm > +++ b/PVE/QemuServer/Helpers.pm > @@ -142,6 +142,9 @@ sub version_cmp { > > sub config_aware_timeout { > my ($config, $is_suspended) = @_; > + > + return $config->{timeout} if defined($config->{timeout}); > + > my $memory = $config->{memory}; > my $timeout = 30; >