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

[Matthias Heiserer <m.heiserer@proxmox.com>]

On 03.03.2023 14:43, Matthieu Malvache wrote:
> 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?
Please send a new patch, mark it as "v2" (i.e. [PATCH v2 qemu-server] , 
and include the change there. You should also briefly describe the 
changes you made, the wiki describes this, or you can take a look at 
other patches in the mailing list.
> 
> Also, could you please let me know how I can "rename" the subject from 
> [PATCH container] to [PATCH qemu-server]?
Our wiki gives the command "git config --local format.subjectprefix 
'PATCH container'"
But that only works for the container repos. For the qemu-server repo, it's
"git config --local format.subjectprefix 'PATCH qemu-server'"

> 
> Thank you again for your help.
> 
> Matthieu Malvache
> Ma2t.
> matthieu@ma2t.com <mailto:matthieu@ma2t.com>
> 
> 
>> Le 3 mars 2023 à 13:06, Matthias Heiserer <m.heiserer@proxmox.com 
>> <mailto:m.heiserer@proxmox.com>> a écrit :
>>
>> Hi,
>>
>> Did you already submit a CLA? 
>> https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright <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 
>>> <mailto: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.
>>
>