[pve-devel] [PATCH qemu-server, manager v5 0/4] fix #1926 autodetect xtermjs or novnc for VM console

[pve-devel] [PATCH qemu-server, manager v5 0/4] fix #1926 autodetect xtermjs or novnc for VM console

[Aaron Lauterer]

This version 5 of the series uses a  different approach as discussed [0] in
the last interation.
We add a new property in the VM status/current API result that includes
the display configurtion of the VM. This way we can check in the
frontend what to do with it.

I chose a nested return value, as that makes it easier to add/move
additional display properties into it.

The first patch just adds the serial property to the API docs as it was
missing. So not really related to this series, but I came across it.


Patch 2/4 adds the new display property. If not explicitly set in the VM
config, it will return the default 'std' value.

Patch 3/4 implements the changes in the UI. The final result isn't
really a lot simpler on the UI side than in V4 where we had the extra
API call to the VM's config directly. Because we still need to wait for
the API call to finish when initially navigating to the VM. But we have
one fewer call.

Patch 4/4 then introduces some changes to make loading of the console
faster if we just navigate in the submenu of a VM itself where we
already have the current status of a VM already cached.

[0] https://lore.proxmox.com/pve-devel/49bcd58f-aba2-4c1e-a6a0-d21828a335bf@proxmox.com/

qemu-server: Aaron Lauterer (2):
  vmstatus_return_properties: add missing serial property
  api: status/current: add display property

 PVE/API2/Qemu.pm  | 13 +++++++++++++
 PVE/QemuServer.pm |  5 +++++
 2 files changed, 18 insertions(+)


manager: Aaron Lauterer (2):
  fix #1926 ui: vm console: autodetect novnc or xtermjs
  ui: console: check on activate if display info for VMs is present

 www/manager6/VNCConsole.js  | 60 ++++++++++++++++++++++++++-----------
 www/manager6/qemu/Config.js |  8 ++++-
 2 files changed, 50 insertions(+), 18 deletions(-)

-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server v5 1/4] vmstatus_return_properties: add missing serial property

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
first introduces with v5

 PVE/QemuServer.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ea45366..76121d6 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2748,6 +2748,11 @@ our $vmstatus_return_properties = {
 	optional => 1,
 	default => 0,
     },
+    serial => {
+	description => "Guest has serial device configured.",
+	type => 'boolean',
+	optional => 1,
+    },
 };
 
 my $last_proc_pid_stat;
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH qemu-server v5 1/4] vmstatus_return_properties: add missing serial property

[Thomas Lamprecht]

Am 07.04.25 um 18:27 schrieb Aaron Lauterer:
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> first introduces with v5
> 
>  PVE/QemuServer.pm | 5 +++++
>  1 file changed, 5 insertions(+)
> 
>

applied this one, added a reference to what commit this fixes though, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server v5 2/4] api: status/current: add display property

[Aaron Lauterer]

This new property returns the configured or default display for a VM.

Instead of a flat property, we use a nested 'type' object that contains
the actual information. This way we can add other properties that belong
to a VM's display in the future without much hassle, to have them all in
one place.
Candidates to be moved into the clipboard property are for example the
spice and clipboard property.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
first introduces with v5

 PVE/API2/Qemu.pm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8b51c04..e11247b 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3035,6 +3035,16 @@ __PACKAGE__->register_method({
 		enum => ['vnc'],
 		optional => 1,
 	    },
+	    display => {
+		description => "Display settings",
+		type => 'object',
+		properties => {
+		    type => {
+			description => "Display type configured",
+			type => 'string',
+		    },
+		},
+	    },
 	},
     },
     code => sub {
@@ -3048,8 +3058,11 @@ __PACKAGE__->register_method({
 
 	$status->{ha} = PVE::HA::Config::get_service_status("vm:$param->{vmid}");
 
+	$status->{display}->{type} = 'std';
 	if ($conf->{vga}) {
 	    my $vga = PVE::QemuServer::parse_vga($conf->{vga});
+	    $status->{display}->{type} = $vga->{type} if defined($vga->{type});
+
 	    my $spice = defined($vga->{type}) && $vga->{type} =~ /^virtio/;
 	    $spice ||= PVE::QemuServer::vga_conf_has_spice($conf->{vga});
 	    $status->{spice} = 1 if $spice;
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server v5 2/4] api: status/current: add display property

[Thomas Lamprecht]

Am 07.04.25 um 18:27 schrieb Aaron Lauterer:
> This new property returns the configured or default display for a VM.
> 
> Instead of a flat property, we use a nested 'type' object that contains
> the actual information. This way we can add other properties that belong
> to a VM's display in the future without much hassle, to have them all in
> one place.
> Candidates to be moved into the clipboard property are for example the
> spice and clipboard property.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> first introduces with v5
> 
>  PVE/API2/Qemu.pm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 8b51c04..e11247b 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -3035,6 +3035,16 @@ __PACKAGE__->register_method({
>  		enum => ['vnc'],
>  		optional => 1,
>  	    },
> +	    display => {
> +		description => "Display settings",
> +		type => 'object',
> +		properties => {
> +		    type => {
> +			description => "Display type configured",
> +			type => 'string',
> +		    },
> +		},
> +	    },
>  	},
>      },
>      code => sub {
> @@ -3048,8 +3058,11 @@ __PACKAGE__->register_method({
>  
>  	$status->{ha} = PVE::HA::Config::get_service_status("vm:$param->{vmid}");
>  
> +	$status->{display}->{type} = 'std';
>  	if ($conf->{vga}) {
>  	    my $vga = PVE::QemuServer::parse_vga($conf->{vga});
> +	    $status->{display}->{type} = $vga->{type} if defined($vga->{type});
> +
>  	    my $spice = defined($vga->{type}) && $vga->{type} =~ /^virtio/;
>  	    $spice ||= PVE::QemuServer::vga_conf_has_spice($conf->{vga});
>  	    $status->{spice} = 1 if $spice;

Looks alright to me, but I do not want to rush this in, so a full test
would be nice, and I probably won't get around to that.
Cannot promise anything for 100% but if you get a positive code review
from Dominik and an additional T-b I might still look into this.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server v5 2/4] api: status/current: add display property

[Fiona Ebner]

Am 07.04.25 um 18:27 schrieb Aaron Lauterer:
> @@ -3048,8 +3058,11 @@ __PACKAGE__->register_method({
>  
>  	$status->{ha} = PVE::HA::Config::get_service_status("vm:$param->{vmid}");
>  
> +	$status->{display}->{type} = 'std';

This is not the correct default in all cases (e.g. aarch64 VM, old
Windows). Please use the get_vga_properties() helper from QemuServer.pm
(you'll need to make it public by dropping its "my" of course).

>  	if ($conf->{vga}) {
>  	    my $vga = PVE::QemuServer::parse_vga($conf->{vga});
> +	    $status->{display}->{type} = $vga->{type} if defined($vga->{type});
> +
>  	    my $spice = defined($vga->{type}) && $vga->{type} =~ /^virtio/;
>  	    $spice ||= PVE::QemuServer::vga_conf_has_spice($conf->{vga});
>  	    $status->{spice} = 1 if $spice;



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server v5 2/4] api: status/current: add display property

[Aaron Lauterer]

On  2025-04-08  09:49, Fiona Ebner wrote:
> Am 07.04.25 um 18:27 schrieb Aaron Lauterer:
>> @@ -3048,8 +3058,11 @@ __PACKAGE__->register_method({
>>   
>>   	$status->{ha} = PVE::HA::Config::get_service_status("vm:$param->{vmid}");
>>   
>> +	$status->{display}->{type} = 'std';
> 
> This is not the correct default in all cases (e.g. aarch64 VM, old
> Windows). Please use the get_vga_properties() helper from QemuServer.pm
> (you'll need to make it public by dropping its "my" of course).

Thanks! Will do

> 
>>   	if ($conf->{vga}) {
>>   	    my $vga = PVE::QemuServer::parse_vga($conf->{vga});
>> +	    $status->{display}->{type} = $vga->{type} if defined($vga->{type});
>> +
>>   	    my $spice = defined($vga->{type}) && $vga->{type} =~ /^virtio/;
>>   	    $spice ||= PVE::QemuServer::vga_conf_has_spice($conf->{vga});
>>   	    $status->{spice} = 1 if $spice;
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v5 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs

[Aaron Lauterer]

Some users configure their VMs to use serial as their display. The big
benefit is that in combination with the xtermjs remote console, copy &
paste works a lot better than via novnc.

While the console button in the top right allows to manually choose the
console type, the Console in the main submenu of a VM does not.

This patch changes the behavior to not load the console right away if
set to 'kvm'. Instead, the callback from the VM's status/current call
will run the final steps to load the console. Because then we know if it
should be noVNC or xtermjs.

That means that in the VNCConsole component we need to keep track if the
console has been fully set up and is running. The actual code that loads
the console into the iframe is moved into a function, so we cann call it
from the API callback.

One result is that the behavior changes. Loading the console on a VM
will take a little bit longer:
* When then Console is the selected and one switches between VMs, a new
  status/current call is made and on a good connection, the console
  should load just a few moments later.
* When one switches to the console from another submenu of the VM, the
  console will load once the next API call to status/current finished.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
Overall not really less complex than v4 [0] but with the new display
property definitely more ready to changes in the future and we save one
API call.

[0] https://lore.proxmox.com/pve-devel/20250325091854.1051956-1-a.lauterer@proxmox.com/

changes since:
v4:
* use new status/current display property

v3:
* fixed spacing issues
* add 'current' parameter when fetching config as the pending might have
  a different display set

v2:
* change approach and do it in the UI alone by fetching the VM
  config before deciding which console to use

v1:
* set 'autodetect' to always true in 'VNCConsole.js'
* add additional checks in pveproxy
  * only if autodetect is enabled and console is set to 'kvm'
  * username exists and has VM.Console permissions for the guest



 www/manager6/VNCConsole.js  | 52 +++++++++++++++++++++++++------------
 www/manager6/qemu/Config.js |  8 +++++-
 2 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
index 9057e447..3371c923 100644
--- a/www/manager6/VNCConsole.js
+++ b/www/manager6/VNCConsole.js
@@ -12,6 +12,36 @@ Ext.define('PVE.noVncConsole', {
     layout: 'fit',
     border: false,
 
+    setupDone: false, // initial setup is done and the session is running
+
+    loadConsole: function(xtermjs, consoleType) {
+	let me = this;
+	if (me.setupDone) {
+	    return;
+	}
+
+	let type = xtermjs ? 'xtermjs' : 'novnc';
+	let sp = Ext.state.Manager.getProvider();
+	if (Ext.isFunction(me.beforeLoad)) {
+	    me.beforeLoad();
+	}
+	let queryDict = {
+	    console: consoleType, // kvm, lxc, upgrade or shell
+	    vmid: me.vmid,
+	    node: me.nodename,
+	    cmd: me.cmd,
+	    'cmd-opts': me.cmdOpts,
+	    resize: sp.get('novnc-scaling', 'scale'),
+	};
+	let box = this.down('[itemid=vncconsole]');
+	queryDict[type] = 1;
+	PVE.Utils.cleanEmptyObjectKeys(queryDict);
+	let url = '/?' + Ext.Object.toQueryString(queryDict);
+	Proxmox.Utils.setErrorMask(me);
+	box.load(url);
+	me.setupDone = true;
+    },
+
     initComponent: function() {
 	var me = this;
 
@@ -29,37 +59,25 @@ Ext.define('PVE.noVncConsole', {
 
 	// always use same iframe, to avoid running several noVnc clients
 	// at same time (to avoid performance problems)
-	var box = Ext.create('Ext.ux.IFrame', { itemid: "vncconsole" });
+	let box = Ext.create('Ext.ux.IFrame', { itemid: "vncconsole" });
 
-	var type = me.xtermjs ? 'xtermjs' : 'novnc';
 	Ext.apply(me, {
 	    items: box,
 	    listeners: {
 		activate: function() {
-		    let sp = Ext.state.Manager.getProvider();
-		    if (Ext.isFunction(me.beforeLoad)) {
-			me.beforeLoad();
+		    if (me.consoleType !== 'kvm') {
+			me.loadConsole(me.xtermjs, me.consoleType);
 		    }
-		    let queryDict = {
-			console: me.consoleType, // kvm, lxc, upgrade or shell
-			vmid: me.vmid,
-			node: me.nodename,
-			cmd: me.cmd,
-			'cmd-opts': me.cmdOpts,
-			resize: sp.get('novnc-scaling', 'scale'),
-		    };
-		    queryDict[type] = 1;
-		    PVE.Utils.cleanEmptyObjectKeys(queryDict);
-		    var url = '/?' + Ext.Object.toQueryString(queryDict);
-		    box.load(url);
 		},
 	    },
 	});
 
+
 	me.callParent();
 
 	me.on('afterrender', function() {
 	    me.focus();
+	    Proxmox.Utils.setErrorMask(me, true);
 	});
     },
 
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 0699175e..03692abe 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -5,6 +5,8 @@ Ext.define('PVE.qemu.Config', {
     onlineHelp: 'chapter_virtual_machines',
     userCls: 'proxmox-tags-full',
 
+    referenceHolder: true,
+
     initComponent: function() {
         var me = this;
 	var vm = me.pveSelNode.data;
@@ -283,6 +285,7 @@ Ext.define('PVE.qemu.Config', {
 		vmid: vmid,
 		consoleType: 'kvm',
 		nodename: nodename,
+		reference: 'submenuConsoleBtn',
 	    });
 	}
 
@@ -444,7 +447,8 @@ Ext.define('PVE.qemu.Config', {
 		lock = rec ? rec.data.value : undefined;
 
 		spice = !!s.data.get('spice');
-		xtermjs = !!s.data.get('serial');
+		rec = s.data.get('display');
+		xtermjs = rec ? rec.data.value.type.startsWith('serial') : false;
 	    }
 
 	    rec = s.data.get('tags');
@@ -467,6 +471,8 @@ Ext.define('PVE.qemu.Config', {
 	    consoleBtn.setEnableSpice(spice);
 	    consoleBtn.setEnableXtermJS(xtermjs);
 
+	    me.lookup('submenuConsoleBtn')?.loadConsole(xtermjs, 'kvm');
+
 	    statusTxt.update({ lock: lock });
 
 	    let guest_running = status === 'running' &&
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager v5 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs

[Dominik Csapak]

tiny nits inline,

with or without that, consider this

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>

On 4/7/25 18:27, Aaron Lauterer wrote:
> Some users configure their VMs to use serial as their display. The big
> benefit is that in combination with the xtermjs remote console, copy &
> paste works a lot better than via novnc.
> 
> While the console button in the top right allows to manually choose the
> console type, the Console in the main submenu of a VM does not.
> 
> This patch changes the behavior to not load the console right away if
> set to 'kvm'. Instead, the callback from the VM's status/current call
> will run the final steps to load the console. Because then we know if it
> should be noVNC or xtermjs.
> 
> That means that in the VNCConsole component we need to keep track if the
> console has been fully set up and is running. The actual code that loads
> the console into the iframe is moved into a function, so we cann call it
> from the API callback.
> 
> One result is that the behavior changes. Loading the console on a VM
> will take a little bit longer:
> * When then Console is the selected and one switches between VMs, a new
>    status/current call is made and on a good connection, the console
>    should load just a few moments later.
> * When one switches to the console from another submenu of the VM, the
>    console will load once the next API call to status/current finished.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> Overall not really less complex than v4 [0] but with the new display
> property definitely more ready to changes in the future and we save one
> API call.
> 
> [0] https://lore.proxmox.com/pve-devel/20250325091854.1051956-1-a.lauterer@proxmox.com/
> 
> changes since:
> v4:
> * use new status/current display property
> 
> v3:
> * fixed spacing issues
> * add 'current' parameter when fetching config as the pending might have
>    a different display set
> 
> v2:
> * change approach and do it in the UI alone by fetching the VM
>    config before deciding which console to use
> 
> v1:
> * set 'autodetect' to always true in 'VNCConsole.js'
> * add additional checks in pveproxy
>    * only if autodetect is enabled and console is set to 'kvm'
>    * username exists and has VM.Console permissions for the guest
> 
> 
> 
>   www/manager6/VNCConsole.js  | 52 +++++++++++++++++++++++++------------
>   www/manager6/qemu/Config.js |  8 +++++-
>   2 files changed, 42 insertions(+), 18 deletions(-)
> 
> diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
> index 9057e447..3371c923 100644
> --- a/www/manager6/VNCConsole.js
> +++ b/www/manager6/VNCConsole.js
> @@ -12,6 +12,36 @@ Ext.define('PVE.noVncConsole', {
>       layout: 'fit',
>       border: false,
>   
> +    setupDone: false, // initial setup is done and the session is running
> +
> +    loadConsole: function(xtermjs, consoleType) {
> +	let me = this;
> +	if (me.setupDone) {
> +	    return;
> +	}
> +
> +	let type = xtermjs ? 'xtermjs' : 'novnc';
> +	let sp = Ext.state.Manager.getProvider();
> +	if (Ext.isFunction(me.beforeLoad)) {
> +	    me.beforeLoad();
> +	}
> +	let queryDict = {
> +	    console: consoleType, // kvm, lxc, upgrade or shell
> +	    vmid: me.vmid,
> +	    node: me.nodename,
> +	    cmd: me.cmd,
> +	    'cmd-opts': me.cmdOpts,
> +	    resize: sp.get('novnc-scaling', 'scale'),
> +	};
> +	let box = this.down('[itemid=vncconsole]');
> +	queryDict[type] = 1;
> +	PVE.Utils.cleanEmptyObjectKeys(queryDict);
> +	let url = '/?' + Ext.Object.toQueryString(queryDict);
> +	Proxmox.Utils.setErrorMask(me);
> +	box.load(url);
> +	me.setupDone = true;
> +    },
> +
>       initComponent: function() {
>   	var me = this;
>   
> @@ -29,37 +59,25 @@ Ext.define('PVE.noVncConsole', {
>   
>   	// always use same iframe, to avoid running several noVnc clients
>   	// at same time (to avoid performance problems)
> -	var box = Ext.create('Ext.ux.IFrame', { itemid: "vncconsole" });
> +	let box = Ext.create('Ext.ux.IFrame', { itemid: "vncconsole" });
>   
> -	var type = me.xtermjs ? 'xtermjs' : 'novnc';
>   	Ext.apply(me, {
>   	    items: box,
>   	    listeners: {
>   		activate: function() {
> -		    let sp = Ext.state.Manager.getProvider();
> -		    if (Ext.isFunction(me.beforeLoad)) {
> -			me.beforeLoad();
> +		    if (me.consoleType !== 'kvm') {
> +			me.loadConsole(me.xtermjs, me.consoleType);
>   		    }
> -		    let queryDict = {
> -			console: me.consoleType, // kvm, lxc, upgrade or shell
> -			vmid: me.vmid,
> -			node: me.nodename,
> -			cmd: me.cmd,
> -			'cmd-opts': me.cmdOpts,
> -			resize: sp.get('novnc-scaling', 'scale'),
> -		    };
> -		    queryDict[type] = 1;
> -		    PVE.Utils.cleanEmptyObjectKeys(queryDict);
> -		    var url = '/?' + Ext.Object.toQueryString(queryDict);
> -		    box.load(url);
>   		},
>   	    },
>   	});
>   
> +
>   	me.callParent();
>   
>   	me.on('afterrender', function() {
>   	    me.focus();
> +	    Proxmox.Utils.setErrorMask(me, true);
>   	});
>       },
>   
> diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
> index 0699175e..03692abe 100644
> --- a/www/manager6/qemu/Config.js
> +++ b/www/manager6/qemu/Config.js
> @@ -5,6 +5,8 @@ Ext.define('PVE.qemu.Config', {
>       onlineHelp: 'chapter_virtual_machines',
>       userCls: 'proxmox-tags-full',
>   
> +    referenceHolder: true,
> +
>       initComponent: function() {
>           var me = this;
>   	var vm = me.pveSelNode.data;
> @@ -283,6 +285,7 @@ Ext.define('PVE.qemu.Config', {
>   		vmid: vmid,
>   		consoleType: 'kvm',
>   		nodename: nodename,
> +		reference: 'submenuConsoleBtn',
>   	    });
>   	}
>   
> @@ -444,7 +447,8 @@ Ext.define('PVE.qemu.Config', {
>   		lock = rec ? rec.data.value : undefined;
>   
>   		spice = !!s.data.get('spice');
> -		xtermjs = !!s.data.get('serial');
> +		rec = s.data.get('display');
> +		xtermjs = rec ? rec.data.value.type.startsWith('serial') : false;

two tiny things here:

* rec is imho not a good name for the display portion, you could just name it 'display'
* for the result of xtermjs you could use something like:

xtermjs = rec?.data.value.type.startsWith('serial') ?? false;

instead of the ternary operator


>   	    }
>   
>   	    rec = s.data.get('tags');
> @@ -467,6 +471,8 @@ Ext.define('PVE.qemu.Config', {
>   	    consoleBtn.setEnableSpice(spice);
>   	    consoleBtn.setEnableXtermJS(xtermjs);
>   
> +	    me.lookup('submenuConsoleBtn')?.loadConsole(xtermjs, 'kvm');
> +
>   	    statusTxt.update({ lock: lock });
>   
>   	    let guest_running = status === 'running' &&



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v5 4/4] ui: console: check on activate if display info for VMs is present

[Aaron Lauterer]

If we already have the display information for a VM, we can proceed
loading the correct console (noVNC or xtermjs).

This way, we don't need to wait for the callback of the VM's
status/current API call to finish setting up the console.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
first introduces with v5

 www/manager6/VNCConsole.js | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
index 3371c923..60772373 100644
--- a/www/manager6/VNCConsole.js
+++ b/www/manager6/VNCConsole.js
@@ -67,6 +67,14 @@ Ext.define('PVE.noVncConsole', {
 		activate: function() {
 		    if (me.consoleType !== 'kvm') {
 			me.loadConsole(me.xtermjs, me.consoleType);
+		    } else {
+			let display = me.up().statusStore.getById('display');
+			if (display && display.data.value.type?.startsWith('serial')) {
+			    me.xtermjs = true;
+			}
+			if (display) {
+			    me.loadConsole(me.xtermjs, me.consoleType);
+			}
 		    }
 		},
 	    },
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager v5 4/4] ui: console: check on activate if display info for VMs is present

[Dominik Csapak]

On 4/7/25 18:27, Aaron Lauterer wrote:
> If we already have the display information for a VM, we can proceed
> loading the correct console (noVNC or xtermjs).
> 
> This way, we don't need to wait for the callback of the VM's
> status/current API call to finish setting up the console.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> first introduces with v5
> 
>   www/manager6/VNCConsole.js | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
> index 3371c923..60772373 100644
> --- a/www/manager6/VNCConsole.js
> +++ b/www/manager6/VNCConsole.js
> @@ -67,6 +67,14 @@ Ext.define('PVE.noVncConsole', {
>   		activate: function() {
>   		    if (me.consoleType !== 'kvm') {
>   			me.loadConsole(me.xtermjs, me.consoleType);
> +		    } else {
> +			let display = me.up().statusStore.getById('display');
> +			if (display && display.data.value.type?.startsWith('serial')) {

this is the same check done in the other patch, no?
a helper could be good here to make sure the checks stay in sync

> +			    me.xtermjs = true;
> +			}
> +			if (display) {
> +			    me.loadConsole(me.xtermjs, me.consoleType);
> +			}
>   		    }
>   		},
>   	    },



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager v5 4/4] ui: console: check on activate if display info for VMs is present

[Aaron Lauterer]

On  2025-04-08  10:10, Dominik Csapak wrote:
> On 4/7/25 18:27, Aaron Lauterer wrote:
>> If we already have the display information for a VM, we can proceed
>> loading the correct console (noVNC or xtermjs).
>>
>> This way, we don't need to wait for the callback of the VM's
>> status/current API call to finish setting up the console.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> first introduces with v5
>>
>>   www/manager6/VNCConsole.js | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
>> index 3371c923..60772373 100644
>> --- a/www/manager6/VNCConsole.js
>> +++ b/www/manager6/VNCConsole.js
>> @@ -67,6 +67,14 @@ Ext.define('PVE.noVncConsole', {
>>           activate: function() {
>>               if (me.consoleType !== 'kvm') {
>>               me.loadConsole(me.xtermjs, me.consoleType);
>> +            } else {
>> +            let display = me.up().statusStore.getById('display');
>> +            if (display && 
>> display.data.value.type?.startsWith('serial')) {
> 
> this is the same check done in the other patch, no?
> a helper could be good here to make sure the checks stay in sync

Yeah, that thought also crossed my mind since yesterday. I'll change 
that to a shared helper.

> 
>> +                me.xtermjs = true;
>> +            }
>> +            if (display) {
>> +                me.loadConsole(me.xtermjs, me.consoleType);
>> +            }
>>               }
>>           },
>>           },
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server, manager v5 0/4] fix #1926 autodetect xtermjs or novnc for VM console

[Aaron Lauterer]

sent a v6 
https://lore.proxmox.com/pve-devel/20250408103715.1081055-1-a.lauterer@proxmox.com/

On  2025-04-07  18:27, Aaron Lauterer wrote:
> This version 5 of the series uses a  different approach as discussed [0] in
> the last interation.
> We add a new property in the VM status/current API result that includes
> the display configurtion of the VM. This way we can check in the
> frontend what to do with it.
> 
> I chose a nested return value, as that makes it easier to add/move
> additional display properties into it.
> 
> The first patch just adds the serial property to the API docs as it was
> missing. So not really related to this series, but I came across it.
> 
> 
> Patch 2/4 adds the new display property. If not explicitly set in the VM
> config, it will return the default 'std' value.
> 
> Patch 3/4 implements the changes in the UI. The final result isn't
> really a lot simpler on the UI side than in V4 where we had the extra
> API call to the VM's config directly. Because we still need to wait for
> the API call to finish when initially navigating to the VM. But we have
> one fewer call.
> 
> Patch 4/4 then introduces some changes to make loading of the console
> faster if we just navigate in the submenu of a VM itself where we
> already have the current status of a VM already cached.
> 
> [0] https://lore.proxmox.com/pve-devel/49bcd58f-aba2-4c1e-a6a0-d21828a335bf@proxmox.com/
> 
> qemu-server: Aaron Lauterer (2):
>    vmstatus_return_properties: add missing serial property
>    api: status/current: add display property
> 
>   PVE/API2/Qemu.pm  | 13 +++++++++++++
>   PVE/QemuServer.pm |  5 +++++
>   2 files changed, 18 insertions(+)
> 
> 
> manager: Aaron Lauterer (2):
>    fix #1926 ui: vm console: autodetect novnc or xtermjs
>    ui: console: check on activate if display info for VMs is present
> 
>   www/manager6/VNCConsole.js  | 60 ++++++++++++++++++++++++++-----------
>   www/manager6/qemu/Config.js |  8 ++++-
>   2 files changed, 50 insertions(+), 18 deletions(-)
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel