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 07ADF1FF15E
for <inbox@lore.proxmox.com>; Tue, 8 Apr 2025 10:08:21 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 55BF5D38B;
Tue, 8 Apr 2025 10:08:16 +0200 (CEST)
Message-ID: <179f54d5-6249-4344-a432-07045c4fe204@proxmox.com>
Date: Tue, 8 Apr 2025 10:08:10 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird Beta
To: pve-devel@lists.proxmox.com
References: <20250407162718.495812-1-a.lauterer@proxmox.com>
<20250407162718.495812-4-a.lauterer@proxmox.com>
Content-Language: en-US
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <20250407162718.495812-4-a.lauterer@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.021 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
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [proxmox.com]
Subject: Re: [pve-devel] [PATCH manager v5 3/4] 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-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
tiny nits inline,
with or without that, consider this
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
On 4/7/25 18:27, Aaron Lauterer wrote:
> 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 to not load the console right away if
> set to 'kvm'. Instead, the callback from the VM's status/current call
> will run the final steps to load the console. Because then we know if it
> should be noVNC or xtermjs.
>
> That means that in the VNCConsole component we need to keep track if the
> console has been fully set up and is running. The actual code that loads
> the console into the iframe is moved into a function, so we cann call it
> from the API callback.
>
> One result is that the behavior changes. Loading the console on a VM
> will take a little bit longer:
> * When then Console is the selected and one switches between VMs, a new
> status/current call is made and on a good connection, the console
> should load just a few moments later.
> * When one switches to the console from another submenu of the VM, the
> console will load once the next API call to status/current finished.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> Overall not really less complex than v4 [0] but with the new display
> property definitely more ready to changes in the future and we save one
> API call.
>
> [0] https://lore.proxmox.com/pve-devel/20250325091854.1051956-1-a.lauterer@proxmox.com/
>
> changes since:
> v4:
> * use new status/current display property
>
> 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 | 52 +++++++++++++++++++++++++------------
> www/manager6/qemu/Config.js | 8 +++++-
> 2 files changed, 42 insertions(+), 18 deletions(-)
>
> diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
> index 9057e447..3371c923 100644
> --- a/www/manager6/VNCConsole.js
> +++ b/www/manager6/VNCConsole.js
> @@ -12,6 +12,36 @@ Ext.define('PVE.noVncConsole', {
> layout: 'fit',
> border: false,
>
> + setupDone: false, // initial setup is done and the session is running
> +
> + loadConsole: function(xtermjs, consoleType) {
> + let me = this;
> + if (me.setupDone) {
> + return;
> + }
> +
> + let type = xtermjs ? 'xtermjs' : 'novnc';
> + let sp = Ext.state.Manager.getProvider();
> + if (Ext.isFunction(me.beforeLoad)) {
> + me.beforeLoad();
> + }
> + let queryDict = {
> + console: 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'),
> + };
> + let box = this.down('[itemid=vncconsole]');
> + queryDict[type] = 1;
> + PVE.Utils.cleanEmptyObjectKeys(queryDict);
> + let url = '/?' + Ext.Object.toQueryString(queryDict);
> + Proxmox.Utils.setErrorMask(me);
> + box.load(url);
> + me.setupDone = true;
> + },
> +
> initComponent: function() {
> var me = this;
>
> @@ -29,37 +59,25 @@ Ext.define('PVE.noVncConsole', {
>
> // 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" });
> + let box = Ext.create('Ext.ux.IFrame', { itemid: "vncconsole" });
>
> - var type = me.xtermjs ? 'xtermjs' : 'novnc';
> Ext.apply(me, {
> items: box,
> listeners: {
> activate: function() {
> - let sp = Ext.state.Manager.getProvider();
> - if (Ext.isFunction(me.beforeLoad)) {
> - me.beforeLoad();
> + if (me.consoleType !== 'kvm') {
> + me.loadConsole(me.xtermjs, me.consoleType);
> }
> - 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);
> },
> },
> });
>
> +
> me.callParent();
>
> me.on('afterrender', function() {
> me.focus();
> + Proxmox.Utils.setErrorMask(me, true);
> });
> },
>
> diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
> index 0699175e..03692abe 100644
> --- a/www/manager6/qemu/Config.js
> +++ b/www/manager6/qemu/Config.js
> @@ -5,6 +5,8 @@ Ext.define('PVE.qemu.Config', {
> onlineHelp: 'chapter_virtual_machines',
> userCls: 'proxmox-tags-full',
>
> + referenceHolder: true,
> +
> initComponent: function() {
> var me = this;
> var vm = me.pveSelNode.data;
> @@ -283,6 +285,7 @@ Ext.define('PVE.qemu.Config', {
> vmid: vmid,
> consoleType: 'kvm',
> nodename: nodename,
> + reference: 'submenuConsoleBtn',
> });
> }
>
> @@ -444,7 +447,8 @@ Ext.define('PVE.qemu.Config', {
> lock = rec ? rec.data.value : undefined;
>
> spice = !!s.data.get('spice');
> - xtermjs = !!s.data.get('serial');
> + rec = s.data.get('display');
> + xtermjs = rec ? rec.data.value.type.startsWith('serial') : false;
two tiny things here:
* rec is imho not a good name for the display portion, you could just name it 'display'
* for the result of xtermjs you could use something like:
xtermjs = rec?.data.value.type.startsWith('serial') ?? false;
instead of the ternary operator
> }
>
> rec = s.data.get('tags');
> @@ -467,6 +471,8 @@ Ext.define('PVE.qemu.Config', {
> consoleBtn.setEnableSpice(spice);
> consoleBtn.setEnableXtermJS(xtermjs);
>
> + me.lookup('submenuConsoleBtn')?.loadConsole(xtermjs, 'kvm');
> +
> statusTxt.update({ lock: lock });
>
> let guest_running = status === 'running' &&
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel