[pve-devel] [PATCH manager] proxy: ui: vm console: autodetect novnc or xtermjs

* [pve-devel] [PATCH manager] proxy: ui: vm console: autodetect novnc or xtermjs
@ 2024-11-22 14:38 Aaron Lauterer
  2024-11-25  9:46 ` Daniel Herzig
  2024-11-25 10:06 ` Aaron Lauterer
  0 siblings, 2 replies; 3+ messages in thread
From: Aaron Lauterer @ 2024-11-22 14:38 UTC (permalink / raw)
  To: pve-devel

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.

Unfortunately, the main console panel defaulted to novnc, not matter
what the guest had configured. One always had to open the console of the
VM in a separate window to make use of xtermjs.

This patch changes the behavior and lets it autodetect the guest
configuration to either use xtermjs or novnc.

If we previously selected the console submenu in one VM and then
switched to another VM, the console submenu is the preselect item for
the VM we switched to. But at this point, the default would be used
(novnc), making it an unpleasant experience. If we would use the same
mechanism as for the top right console button - `me.mon()` - it would
work, but only if we (re)select the console submenu after the first API
call to `nodes/{nola}/qemu/{vmid}/status` finished. On the initial
load, if the console is the active submenu, it would still default to
novnc.

While we probably could have implemented in just in the UI, for example
by waiting until the first call to `status` is done, this would
potentially introduce "laggyness" when opening the console.

Another option is to add a new parameter 'autodetect' when we open the
console via the submenu of a VM. The backend can then check the VM
config and override the novnc/xtermjs setting.

The result is, that even when switching VMs in the web UI with the
console submenu selected, it will load xtermjs for the VMs that use it.

Potential further improvements could be to also check the console
settings in datacenter.cfg and consider it. As that isn't checked in the
inline console panel for CTs and VMs at all, with out without this
patch.
The setting does have an impact on the buttons that open the console in
a new window.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
Another thing that I noticed is that the property we use to decide if we
enable xtermjs for VMs in the top right console button, and for now also
in this patch, only checks if the VM has a serial device configured.
PVE::QemuServer::vmstatus() calls `conf_has_serial()`.

A better approach would be to have a vmstatus property that actually
tells us if the VM has serial set as display option. As the display
could be set to something else, even if a serial device exists. There
are other use-cases for a serial device in the VM, like passing through
an actual serial port.

But I didn't want to open that can of worms just yet ;)

 PVE/Service/pveproxy.pm     | 9 +++++++++
 www/manager6/VNCConsole.js  | 2 ++
 www/manager6/qemu/Config.js | 1 +
 3 files changed, 12 insertions(+)

diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index ac108545..345f47f5 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -228,6 +228,15 @@ sub get_index {
     my $novnc = defined($args->{console}) && $args->{novnc};
     my $xtermjs = defined($args->{console}) && $args->{xtermjs};
 
+    if (defined($args->{autodetect}) && $args->{autodetect}) {
+	my $vmid = $args->{vmid};
+	my $vmstatus = PVE::QemuServer::vmstatus($vmid);
+	if (defined($vmstatus->{$vmid}) && $vmstatus->{$vmid}->{serial}) {
+		$novnc = 0;
+		$xtermjs = 1;
+	}
+    }
+
     my $langfile = -f "$basedirs->{i18n}/pve-lang-$lang.js" ? 1 : 0;
 
     my $version = PVE::pvecfg::version();
diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
index 9057e447..c50aa1de 100644
--- a/www/manager6/VNCConsole.js
+++ b/www/manager6/VNCConsole.js
@@ -8,6 +8,7 @@ Ext.define('PVE.noVncConsole', {
 
     consoleType: undefined, // lxc, kvm, shell, cmd
     xtermjs: false,
+    autodetect: false,
 
     layout: 'fit',
     border: false,
@@ -47,6 +48,7 @@ Ext.define('PVE.noVncConsole', {
 			cmd: me.cmd,
 			'cmd-opts': me.cmdOpts,
 			resize: sp.get('novnc-scaling', 'scale'),
+			autodetect: me.autodetect,
 		    };
 		    queryDict[type] = 1;
 		    PVE.Utils.cleanEmptyObjectKeys(queryDict);
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 48eb753e..07fd710a 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -271,6 +271,7 @@ Ext.define('PVE.qemu.Config', {
 		xtype: 'pveNoVncConsole',
 		vmid: vmid,
 		consoleType: 'kvm',
+		autodetect: true,
 		nodename: nodename,
 	    });
 	}
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 3+ messages in thread