[pve-devel] [PATCH manager v4] fix #1926 ui: vm console: autodetect novnc or xtermjs

[Aaron Lauterer <a.lauterer@proxmox.com>]

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 for VMs and will first fetch the VM
config and if the display is set to "serialX", will set the console to
xtermjs. Otherwise it will fall back to regular noVNC.

Since getting the VM config is an async API call, the code had to be
restructured so we can do the actual loading of the console after the
config has been fetched.
Therefore we now have the 'loadConsole()' function which will be called
when the UI item is activated and in the KVM case, after loading and
handling the VM config. It is guarded by the 'activated' and
'configLoaded' variables.

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 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 did not want to introduce another new property for vmstatus() and
changing the meaning of the current serial property would be a breaking
change.
Therefore, this current UI only implementation.

changes since:
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 | 62 +++++++++++++++++++++++++++-----------
 1 file changed, 45 insertions(+), 17 deletions(-)

diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
index 9057e447..868ee203 100644
--- a/www/manager6/VNCConsole.js
+++ b/www/manager6/VNCConsole.js
@@ -27,31 +27,59 @@ Ext.define('PVE.noVncConsole', {
 	    throw "no VM ID specified";
 	}
 
+	let activated = false;
+	let configLoaded = false;
 	// 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" });
 
-	var type = me.xtermjs ? 'xtermjs' : 'novnc';
+	let loadConsole = function() {
+	    if (!activated || !configLoaded) {
+		return;
+	    }
+	    let type = me.xtermjs ? 'xtermjs' : 'novnc';
+	    let sp = Ext.state.Manager.getProvider();
+	    if (Ext.isFunction(me.beforeLoad)) {
+		me.beforeLoad();
+	    }
+	    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);
+	};
+
+	if (me.consoleType === "kvm") {
+	    Proxmox.Utils.API2Request({
+		url: `/api2/extjs/nodes/${me.nodename}/qemu/${me.vmid}/config`,
+		params: { 'current': '1' },
+		method: 'GET',
+		failure: response => Ext.Msg.alert('Error', response.htmlStatus),
+		success: function({ result }, options) {
+		    if (result.data.vga?.startsWith("serial")) {
+			me.xtermjs = true;
+		    }
+		    configLoaded = true;
+		    loadConsole();
+		},
+	    });
+	} else { // don't need to load anything else
+	    configLoaded = true;
+	}
+
 	Ext.apply(me, {
 	    items: box,
 	    listeners: {
 		activate: function() {
-		    let sp = Ext.state.Manager.getProvider();
-		    if (Ext.isFunction(me.beforeLoad)) {
-			me.beforeLoad();
-		    }
-		    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);
+		    activated = true;
+		    loadConsole();
 		},
 	    },
 	});
-- 
2.39.5



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