From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id A71C91FF15E
	for <inbox@lore.proxmox.com>; Tue, 25 Mar 2025 10:19:04 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id A8DE24806;
	Tue, 25 Mar 2025 10:18:58 +0100 (CET)
From: Aaron Lauterer <a.lauterer@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Tue, 25 Mar 2025 10:18:54 +0100
Message-Id: <20250325091854.1051956-1-a.lauterer@proxmox.com>
X-Mailer: git-send-email 2.39.5
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.035 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pve-devel] [PATCH manager v4] fix #1926 ui: vm console: autodetect
 novnc or xtermjs
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.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