* [pve-devel] [PATCH container 0/1] Add support for custom timeout in vncproxy method @ 2023-03-03 11:16 Matthieu Malvache 2023-03-03 11:16 ` [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value " Matthieu Malvache 0 siblings, 1 reply; 6+ messages in thread From: Matthieu Malvache @ 2023-03-03 11:16 UTC (permalink / raw) To: pve-devel Dear Maintainer, This patch 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. The purpose of this change is to allow users to configure the timeout duration for the VNC proxy connection to better suit their needs. By default, the timeout is set to 10 seconds, which should work well for most use cases. However, some users may need to increase or decrease the timeout based on factors such as network latency. Thank you for considering this patch. Sincerely, Matthieu Malvache Matthieu Malvache (1): vnc: Allow custom timeout value in vncproxy method PVE/API2/Qemu.pm | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value in vncproxy method 2023-03-03 11:16 [pve-devel] [PATCH container 0/1] Add support for custom timeout in vncproxy method Matthieu Malvache @ 2023-03-03 11:16 ` Matthieu Malvache 2023-03-03 12:06 ` Matthias Heiserer 2023-03-03 14:50 ` Dominik Csapak 0 siblings, 2 replies; 6+ messages in thread From: Matthieu Malvache @ 2023-03-03 11:16 UTC (permalink / raw) To: pve-devel 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}; 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; -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value in vncproxy method 2023-03-03 11:16 ` [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value " Matthieu Malvache @ 2023-03-03 12:06 ` Matthias Heiserer 2023-03-03 13:43 ` Matthieu Malvache 2023-03-03 14:50 ` Dominik Csapak 1 sibling, 1 reply; 6+ messages in thread From: Matthias Heiserer @ 2023-03-03 12:06 UTC (permalink / raw) To: Proxmox VE development discussion, Matthieu Malvache 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value in vncproxy method 2023-03-03 12:06 ` Matthias Heiserer @ 2023-03-03 13:43 ` Matthieu Malvache 2023-03-03 15:07 ` Matthias Heiserer 0 siblings, 1 reply; 6+ messages in thread From: Matthieu Malvache @ 2023-03-03 13:43 UTC (permalink / raw) To: Matthias Heiserer; +Cc: Proxmox VE development discussion 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. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value in vncproxy method 2023-03-03 13:43 ` Matthieu Malvache @ 2023-03-03 15:07 ` Matthias Heiserer 0 siblings, 0 replies; 6+ messages in thread From: Matthias Heiserer @ 2023-03-03 15:07 UTC (permalink / raw) To: Matthieu Malvache; +Cc: Proxmox VE development discussion 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. >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value in vncproxy method 2023-03-03 11:16 ` [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value " Matthieu Malvache 2023-03-03 12:06 ` Matthias Heiserer @ 2023-03-03 14:50 ` Dominik Csapak 1 sibling, 0 replies; 6+ messages in thread From: Dominik Csapak @ 2023-03-03 14:50 UTC (permalink / raw) To: Proxmox VE development discussion, Matthieu Malvache 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; > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-03 15:07 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-03 11:16 [pve-devel] [PATCH container 0/1] Add support for custom timeout in vncproxy method Matthieu Malvache 2023-03-03 11:16 ` [pve-devel] [PATCH container 1/1] vnc: Allow custom timeout value " Matthieu Malvache 2023-03-03 12:06 ` Matthias Heiserer 2023-03-03 13:43 ` Matthieu Malvache 2023-03-03 15:07 ` Matthias Heiserer 2023-03-03 14:50 ` Dominik Csapak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox