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

[Matthieu Malvache <matthieu@ma2t.com>]

Hi Matthias,

Thank you for your email. I sent the CLA earlier this afternoon. 

This is actually the first time I'm contributing to a project like this, so thank you for your guidance 
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 à 13:06, Matthias Heiserer <m.heiserer@proxmox.com> a écrit :
> 
> Hi,
> 
> Did you already submit a CLA? https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright
> 
> I noticed two issues:
> 
> This patch is not for pve-container, but rather qemu-server, so the subject should include [PATCH qemu-server] rather than [PATCH container]
> 
> 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 <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.",
>> +	    },
> The indent here should be (\tab.... is a tab, \w.. are four spaces)
> \tab....\w..timeout
> \tab....\tab....optional => 1
> 
> so the difference in indents is always 4 spaces (1 tab = 8 spaces wide)
>>  	},
>>      },
>>      returns => {
>> @@ -2192,6 +2199,7 @@ __PACKAGE__->register_method({
>>  	my $vmid = $param->{vmid};
>>  	my $node = $param->{node};
>>  	my $websocket = $param->{websocket};
>> +	my $timeout = $param->{timeout};
>>    	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;
>>  
> Other than that, I'd say it looks fine.
>