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

[pve-devel] [PATCH manager v2] proxy: 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.

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 handle it in the backend. 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.

We only do it if the HTTP call has the new 'autodetect' parameter
enabled. Additionally we introduce a permission check for 'VM.Console'
at this level and only adjust the used console if the user does have the
correct permissions. Otherwise we leak the existence of the VM to
unauthorized users if it has 'serial' configured due to the change in
the returned console (noVNC/xtermjs).

The actual check if the user is allowed to access the console is
happening once the loaded console implementation establishes a
connection to the console API endpoint of the guest. But at this point,
either the noVNC or xtermjs console is already sent to the user's
browser.

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 ;)

changes since v2:
* 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

 PVE/Service/pveproxy.pm    | 16 ++++++++++++++++
 www/manager6/VNCConsole.js |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index ac108545..4cd281c7 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -228,6 +228,22 @@ sub get_index {
     my $novnc = defined($args->{console}) && $args->{novnc};
     my $xtermjs = defined($args->{console}) && $args->{xtermjs};
 
+    my $rpcenv = PVE::RPCEnvironment::get();
+    if (
+	defined($args->{console})
+	&& $args->{console} eq 'kvm'
+	&& $args->{autodetect}
+	&& $username
+	&& $rpcenv->check_full($username, "/vms/$args->{vmid}", ["VM.Console"], 1, 1)
+    ) {
+	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..8462b02e 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: true,
 
     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);
-- 
2.39.5



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