Re: [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value in vncproxy method

[Dominik Csapak <d.csapak@proxmox.com>]

one additional comment inline:

On 3/3/23 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 <matthieu@ma2t.com>
> ---
>   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 => 0,
>   		description => "Generates a random password to be used as ticket instead of the API ticket.",
>   	    },
> +	    timeout => {
> +		    optional => 1,
> +		    type => 'integer',
> +		    minimum => 5,
> +		    default => 10,
> +		    description => "Timeout in seconds for the vnc proxy connection.",
> +	    },
>   	},
>       },
>       returns => {
> @@ -2192,6 +2199,7 @@ __PACKAGE__->register_method({
>   	my $vmid = $param->{vmid};
>   	my $node = $param->{node};
>   	my $websocket = $param->{websocket};
> +	my $timeout = $param->{timeout};

that has actually a bug ;)

the 'default' option in the parameter definition is not set anywhere, it's just
used for the documentation generation


to actually make it use the default of 10 you should do:

my $timeout = $param->{timeout} // 10;


this way it is set to 10 when the parameter is not set

>   
>   	my $conf = PVE::QemuConfig->load_config($vmid, $node); # check if VM exists
>   
> @@ -2226,8 +2234,6 @@ __PACKAGE__->register_method({
>   
>   	my $port = PVE::Tools::next_vnc_port($family);
>   
> -	my $timeout = 10;
> -
>   	my $realcmd = sub {
>   	    my $upid = shift;
>