From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 633691FF173 for ; Mon, 25 Nov 2024 11:04:54 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9BB5AF2DE; Mon, 25 Nov 2024 11:05:04 +0100 (CET) From: Aaron Lauterer To: pve-devel@lists.proxmox.com Date: Mon, 25 Nov 2024 11:04:27 +0100 Message-Id: <20241125100427.86341-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 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 v2] proxy: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "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 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 --- 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