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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id D28998438 for ; Fri, 3 Mar 2023 14:46:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BB2F6FC74 for ; Fri, 3 Mar 2023 14:46:34 +0100 (CET) Received: from mail.ma2t.com (mail02.ma2t.com [185.154.152.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 3 Mar 2023 14:46:33 +0100 (CET) Authentication-Results: mail.ma2t.com; auth=pass (plain) From: Matthieu Malvache Message-Id: Mime-Version: 1.0 (Mac OS X Mail 15.0 \(3693.60.0.1.1\)) Date: Fri, 3 Mar 2023 14:43:12 +0100 In-Reply-To: <3e15f154-8e08-e13c-e844-d445eee06940@proxmox.com> Cc: Proxmox VE development discussion To: Matthias Heiserer References: <20230303111604.2043-1-matthieu@ma2t.com> <20230303111604.2043-2-matthieu@ma2t.com> <3e15f154-8e08-e13c-e844-d445eee06940@proxmox.com> Received: from localhost (Unknown [127.0.0.1]) by mail.ma2t.com (Haraka) with ESMTPSA id A10E1FA5-F4B4-49FF-861C-7BDA6818A36D.1 envelope-from (authenticated bits=0) (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256); Fri, 03 Mar 2023 13:43:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; bh=XP2m7fWh6kx01/BZCkoVsn5FnjHHmyykGFmjFHhnVtg=; c=relaxed/simple; d=ma2t.com; h=from:subject:date:message-id:to:cc:mime-version; s=s20200418207; b=XEdm/fHW07h0oFLF8ShrKCrJFRFGXZhPOSQ8vkO7/eCcXuCHUdFAx3w7h+Hc4AC3DJzOTXQm1NvL3OmiClMMfHrPDxKtf7ONXAWy6PPI+cFoPeIIMvbwp1VIlYJZSuK5qrfi4GJHzIZ1H/YPAy58GhPeLOm0GwOIlI8i1h3P21k= X-SPAM-LEVEL: Spam detection results: 0 AWL 0.729 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain HTML_MESSAGE 0.001 HTML included in message MSGID_FROM_MTA_HEADER 0.001 Message-Id was added by a relay SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value in vncproxy method 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, 03 Mar 2023 13:46:34 -0000 Hi Matthias, Thank you for your email. I sent the CLA earlier this afternoon.=20 This is actually the first time I'm contributing to a project like this, = so thank you for your guidance=20 and pointing out the issues with the patch. Regarding the tab issue, should I submit a fix for this separately, or = can I include it in the same patch? Also, could you please let me know how I can "rename" the subject from = [PATCH container] to [PATCH qemu-server]? Thank you again for your help. Matthieu Malvache Ma2t. matthieu@ma2t.com > Le 3 mars 2023 =C3=A0 13:06, Matthias Heiserer = a =C3=A9crit : >=20 > Hi, >=20 > Did you already submit a CLA? = https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_= Copyright >=20 > I noticed two issues: >=20 > This patch is not for pve-container, but rather qemu-server, so the = subject should include [PATCH qemu-server] rather than [PATCH container] >=20 > On 03.03.2023 12:16, Matthieu Malvache wrote: >> This commit adds support for a custom timeout value in the 'vncproxy' >> method of the Proxmox PVE REST API. The timeout can be specified = using >> the 'timeout' parameter and defaults to 10 seconds if not set. >> Signed-off-by: Matthieu Malvache >> --- >> PVE/API2/Qemu.pm | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm >> index 587bb22..4ca5842 100644 >> --- a/PVE/API2/Qemu.pm >> +++ b/PVE/API2/Qemu.pm >> @@ -2164,6 +2164,13 @@ __PACKAGE__->register_method({ >> default =3D> 0, >> description =3D> "Generates a random password to be used = as ticket instead of the API ticket.", >> }, >> + timeout =3D> { >> + optional =3D> 1, >> + type =3D> 'integer', >> + minimum =3D> 5, >> + default =3D> 10, >> + description =3D> "Timeout in seconds for the vnc = proxy connection.", >> + }, > The indent here should be (\tab.... is a tab, \w.. are four spaces) > \tab....\w..timeout > \tab....\tab....optional =3D> 1 >=20 > so the difference in indents is always 4 spaces (1 tab =3D 8 spaces = wide) >> }, >> }, >> returns =3D> { >> @@ -2192,6 +2199,7 @@ __PACKAGE__->register_method({ >> my $vmid =3D $param->{vmid}; >> my $node =3D $param->{node}; >> my $websocket =3D $param->{websocket}; >> + my $timeout =3D $param->{timeout}; >> my $conf =3D PVE::QemuConfig->load_config($vmid, $node); # check = if VM exists >> @@ -2226,8 +2234,6 @@ __PACKAGE__->register_method({ >> my $port =3D PVE::Tools::next_vnc_port($family); >> - my $timeout =3D 10; >> - >> my $realcmd =3D sub { >> my $upid =3D shift; >> =20 > Other than that, I'd say it looks fine. >=20