* [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console
@ 2025-10-03 15:00 Aaron Lauterer
2025-10-03 15:00 ` [pve-devel] [PATCH qemu-server v8 1/2] add new public get_default_vga_type function Aaron Lauterer
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Aaron Lauterer @ 2025-10-03 15:00 UTC (permalink / raw)
To: pve-devel
Here is a new verson for this fix. Nothing changed since v7 expect
rebasing. Quite a bit happened since then, but the rebase should be fine
by now. I gave it a quick test on my workstation and things seem to work
as expected. A quick test by someone else might be warranted though,
just to be safe. I added all the R-b and T-b tags that were added on v7
direclty into each patch. Hopefully as they should be (chronological).
The old cover letter that explains what is happening below:
After the feedback from Fiona on v6 and some
off-list discussion, we decided to improve the backend part by moving
the "default" logic out of `get_vga_properties` and move it into a
separate public `get_default_vga_type` function. This way we can leave
all the other functions private to the QemuServer.pm module.
We add a new property in the VM status/current API result that includes
the display configurtion of the VM. This way we can check in the
frontend what to do with it.
I chose a nested return value, as that makes it easier to add/move
additional display properties into it.
Patch 1/4 moves the default display logic into its own public function
Patch 2/4 adds the new display property. If not explicitly set in the VM
config, it will return the default value.
Patch 3/4 implements the changes in the UI. The final result isn't
really a lot simpler on the UI side than in V4 where we had the extra
API call to the VM's config directly. Because we still need to wait for
the API call to finish when initially navigating to the VM. But we have
one fewer call.
Patch 4/4 then introduces some changes to make loading of the console
faster if we just navigate in the submenu of a VM itself where we
already have the current status of a VM already cached.
Changes from
v7: rebased
v6: backend only: create new `get_default_vga_type` function.
v5: implement suggestions:
* use get_vga_properties for default VGA
* UI: use helper to determine if serial display
Aaron Lauterer (2):
add new public get_default_vga_type function
api: status/current: add display property
src/PVE/API2/Qemu.pm | 13 +++++++++++++
src/PVE/QemuServer.pm | 29 ++++++++++++++++++++++-------
2 files changed, 35 insertions(+), 7 deletions(-)
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] [PATCH qemu-server v8 1/2] add new public get_default_vga_type function 2025-10-03 15:00 [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer @ 2025-10-03 15:00 ` Aaron Lauterer 2025-10-03 15:00 ` [pve-devel] [PATCH qemu-server v8 2/2] api: status/current: add display property Aaron Lauterer ` (3 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Aaron Lauterer @ 2025-10-03 15:00 UTC (permalink / raw) To: pve-devel we resolve missing parameters if necessary to make it easier to call from another module where we likely only have the VM config ready. Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> Tested-by: Hannes Duerr <h.duerr@proxmox.com> Reviewed-by: Michael Köppl <m.koeppl@proxmox.com> Tested-by: Michael Köppl <m.koeppl@proxmox.com> --- changes since v7: * rebased v6: * instead of making get_vga_properties and extract_version public, we create the new get_default_vga_type function and only make it public. src/PVE/QemuServer.pm | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm index 9f35de2e..adcdac5b 100644 --- a/src/PVE/QemuServer.pm +++ b/src/PVE/QemuServer.pm @@ -3044,6 +3044,27 @@ my sub should_disable_smm { && $vga->{type} =~ m/^(serial\d+|none)$/; } +sub get_default_vga_type { + my ($conf, $arch, $machine_version, $winversion) = @_; + + $arch //= PVE::QemuServer::Helpers::get_vm_arch($conf); + $winversion //= PVE::QemuServer::Helpers::windows_version($conf->{ostype}); + if (!$machine_version) { + my $kvm_binary = PVE::QemuServer::Helpers::get_command_for_arch($arch); + my $kvmver = kvm_user_version($kvm_binary); + my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf, undef, $arch); + $machine_version = extract_version($machine_type, $kvmver); + } + + if ($arch eq 'aarch64') { + return 'virtio'; + } elsif (min_version($machine_version, 2, 9)) { + return (!$winversion || $winversion >= 6) ? 'std' : 'cirrus'; + } else { + return ($winversion >= 6) ? 'std' : 'cirrus'; + } +} + my sub get_vga_properties { my ($conf, $arch, $machine_version, $winversion) = @_; @@ -3053,13 +3074,7 @@ my sub get_vga_properties { $vga->{type} = 'qxl' if $qxlnum; if (!$vga->{type}) { - if ($arch eq 'aarch64') { - $vga->{type} = 'virtio'; - } elsif (min_version($machine_version, 2, 9)) { - $vga->{type} = (!$winversion || $winversion >= 6) ? 'std' : 'cirrus'; - } else { - $vga->{type} = ($winversion >= 6) ? 'std' : 'cirrus'; - } + $vga->{type} = get_default_vga_type($conf, $arch, $machine_version, $winversion); } return ($vga, $qxlnum); -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH qemu-server v8 2/2] api: status/current: add display property 2025-10-03 15:00 [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer 2025-10-03 15:00 ` [pve-devel] [PATCH qemu-server v8 1/2] add new public get_default_vga_type function Aaron Lauterer @ 2025-10-03 15:00 ` Aaron Lauterer 2025-10-03 15:00 ` [pve-devel] [PATCH manager v8 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs Aaron Lauterer ` (2 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Aaron Lauterer @ 2025-10-03 15:00 UTC (permalink / raw) To: pve-devel This new property returns the configured or default display for a VM. Instead of a flat property, we use a nested 'type' object that contains the actual information. This way we can add other properties that belong to a VM's display in the future without much hassle, to have them all in one place. Candidates to be moved into the display property are for example the spice and clipboard property. Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> Tested-by: Hannes Duerr <h.duerr@proxmox.com> Reviewed-by: Michael Köppl <m.koeppl@proxmox.com> Tested-by: Michael Köppl <m.koeppl@proxmox.com> --- changes since v7: * rebased v6: * use the new QemuServer::get_default_vga_type function, which means we can drop a lot of boilerplate code v5: * use QemuServer::get_vga_properties to determine the default * properties. src/PVE/API2/Qemu.pm | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm index 7fced6c6..d1a4eafb 100644 --- a/src/PVE/API2/Qemu.pm +++ b/src/PVE/API2/Qemu.pm @@ -3310,6 +3310,16 @@ __PACKAGE__->register_method({ enum => ['vnc'], optional => 1, }, + display => { + description => "Display settings", + type => 'object', + properties => { + type => { + description => "Display type configured", + type => 'string', + }, + }, + }, }, }, code => sub { @@ -3323,8 +3333,11 @@ __PACKAGE__->register_method({ $status->{ha} = PVE::HA::Config::get_service_status("vm:$param->{vmid}"); + $status->{display}->{type} = PVE::QemuServer::get_default_vga_type($conf); if ($conf->{vga}) { my $vga = PVE::QemuServer::parse_vga($conf->{vga}); + $status->{display}->{type} = $vga->{type} if defined($vga->{type}); + my $spice = defined($vga->{type}) && $vga->{type} =~ /^virtio/; $spice ||= PVE::QemuServer::vga_conf_has_spice($conf->{vga}); $status->{spice} = 1 if $spice; -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH manager v8 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs 2025-10-03 15:00 [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer 2025-10-03 15:00 ` [pve-devel] [PATCH qemu-server v8 1/2] add new public get_default_vga_type function Aaron Lauterer 2025-10-03 15:00 ` [pve-devel] [PATCH qemu-server v8 2/2] api: status/current: add display property Aaron Lauterer @ 2025-10-03 15:00 ` Aaron Lauterer 2025-10-27 14:42 ` Michael Köppl 2025-10-03 15:00 ` [pve-devel] [PATCH manager v8 4/4] ui: console: check on activate if display info for VMs is present Aaron Lauterer 2025-10-27 14:48 ` [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Michael Köppl 4 siblings, 1 reply; 8+ messages in thread From: Aaron Lauterer @ 2025-10-03 15:00 UTC (permalink / raw) To: 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. 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> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com> Tested-by: Hannes Duerr <h.duerr@proxmox.com> Reviewed-by: Michael Köppl <m.koeppl@proxmox.com> Tested-by: Michael Köppl <m.koeppl@proxmox.com> --- changes since: v7: * rebased v6: none v5: * introduce PVE.Utils.isSerialDisplay helper * avoid 'rec' assignment in callback when setting the 'xtermjs' variable 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/Utils.js | 4 +++ www/manager6/VNCConsole.js | 51 ++++++++++++++++++++++++------------- www/manager6/qemu/Config.js | 9 ++++++- 3 files changed, 46 insertions(+), 18 deletions(-) diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js index c48ee0b2..03eca22d 100644 --- a/www/manager6/Utils.js +++ b/www/manager6/Utils.js @@ -2051,6 +2051,10 @@ Ext.define('PVE.Utils', { }, }, + isSerialDisplay: function (display) { + return display?.startsWith('serial') ?? false; + }, + singleton: true, constructor: function () { var me = this; diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js index e4f22cf9..d903f2d5 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,29 +59,15 @@ 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); }, }, }); @@ -60,6 +76,7 @@ Ext.define('PVE.noVncConsole', { 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 842d35de..ffec14de 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; @@ -311,6 +313,7 @@ Ext.define('PVE.qemu.Config', { vmid: vmid, consoleType: 'kvm', nodename: nodename, + reference: 'submenuConsoleBtn', }); } @@ -474,7 +477,7 @@ Ext.define('PVE.qemu.Config', { lock = rec ? rec.data.value : undefined; spice = !!s.data.get('spice'); - xtermjs = !!s.data.get('serial'); + xtermjs = PVE.Utils.isSerialDisplay(s.data.get('display')?.data.value.type); } rec = s.data.get('tags'); @@ -486,6 +489,10 @@ Ext.define('PVE.qemu.Config', { var resume = ['prelaunch', 'paused', 'suspended'].indexOf(qmpstatus) !== -1; + me.lookup('submenuConsoleBtn')?.loadConsole(xtermjs, 'kvm'); + + statusTxt.update({ lock: lock }); + if (resume || lock === 'suspended') { startBtn.setVisible(false); resumeBtn.setVisible(true); -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH manager v8 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs 2025-10-03 15:00 ` [pve-devel] [PATCH manager v8 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs Aaron Lauterer @ 2025-10-27 14:42 ` Michael Köppl 0 siblings, 0 replies; 8+ messages in thread From: Michael Köppl @ 2025-10-27 14:42 UTC (permalink / raw) To: Proxmox VE development discussion; +Cc: pve-devel On Fri Oct 3, 2025 at 5:00 PM CEST, Aaron Lauterer wrote: > @@ -474,7 +477,7 @@ Ext.define('PVE.qemu.Config', { > lock = rec ? rec.data.value : undefined; > > spice = !!s.data.get('spice'); > - xtermjs = !!s.data.get('serial'); > + xtermjs = PVE.Utils.isSerialDisplay(s.data.get('display')?.data.value.type); > } > > rec = s.data.get('tags'); > @@ -486,6 +489,10 @@ Ext.define('PVE.qemu.Config', { > > var resume = ['prelaunch', 'paused', 'suspended'].indexOf(qmpstatus) !== -1; > > + me.lookup('submenuConsoleBtn')?.loadConsole(xtermjs, 'kvm'); > + > + statusTxt.update({ lock: lock }); > + Might be that I'm just missing something, but why was this moved up compared to v7? I don't think this is related to the rebase? > if (resume || lock === 'suspended') { > startBtn.setVisible(false); > resumeBtn.setVisible(true); _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH manager v8 4/4] ui: console: check on activate if display info for VMs is present 2025-10-03 15:00 [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer ` (2 preceding siblings ...) 2025-10-03 15:00 ` [pve-devel] [PATCH manager v8 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs Aaron Lauterer @ 2025-10-03 15:00 ` Aaron Lauterer 2025-10-27 14:48 ` [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Michael Köppl 4 siblings, 0 replies; 8+ messages in thread From: Aaron Lauterer @ 2025-10-03 15:00 UTC (permalink / raw) To: pve-devel If we already have the display information for a VM, we can proceed loading the correct console (noVNC or xtermjs). This way, we don't need to wait for the callback of the VM's status/current API call to finish setting up the console. Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com> Tested-by: Hannes Duerr <h.duerr@proxmox.com> Reviewed-by: Michael Köppl <m.koeppl@proxmox.com> Tested-by: Michael Köppl <m.koeppl@proxmox.com> --- changes since: v7: * rebased v6: none v5: * introduce PVE.Utils.isSerialDisplay helper * avoid 'rec' assignment in callback when setting the 'xtermjs' variable 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 | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js index d903f2d5..b7f90bda 100644 --- a/www/manager6/VNCConsole.js +++ b/www/manager6/VNCConsole.js @@ -67,6 +67,14 @@ Ext.define('PVE.noVncConsole', { activate: function () { if (me.consoleType !== 'kvm') { me.loadConsole(me.xtermjs, me.consoleType); + } else { + let display = me.up().statusStore.getById('display'); + if (PVE.Utils.isSerialDisplay(display?.data.value.type)) { + me.xtermjs = true; + } + if (display) { + me.loadConsole(me.xtermjs, me.consoleType); + } } }, }, -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console 2025-10-03 15:00 [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer ` (3 preceding siblings ...) 2025-10-03 15:00 ` [pve-devel] [PATCH manager v8 4/4] ui: console: check on activate if display info for VMs is present Aaron Lauterer @ 2025-10-27 14:48 ` Michael Köppl 2025-11-03 21:04 ` Aaron Lauterer 4 siblings, 1 reply; 8+ messages in thread From: Michael Köppl @ 2025-10-27 14:48 UTC (permalink / raw) To: Proxmox VE development discussion; +Cc: pve-devel Gave this another quick spin, just to be sure. Tested the following: - Created VM with serial terminal as display - xtermjs was used for serial - Switching to "default" or explicitly to "Standard VGA" uses noVNC - Switching to "SPICE" uses remote-viewer Seems to work as advertised. Left a comment regarding one code change on 3/4. Renewing the T-b just in case: Tested-by: Michael Köppl <m.koeppl@proxmox.com> On Fri Oct 3, 2025 at 5:00 PM CEST, Aaron Lauterer wrote: > Here is a new verson for this fix. Nothing changed since v7 expect > rebasing. Quite a bit happened since then, but the rebase should be fine > by now. I gave it a quick test on my workstation and things seem to work > as expected. A quick test by someone else might be warranted though, > just to be safe. I added all the R-b and T-b tags that were added on v7 > direclty into each patch. Hopefully as they should be (chronological). > > The old cover letter that explains what is happening below: > > After the feedback from Fiona on v6 and some > off-list discussion, we decided to improve the backend part by moving > the "default" logic out of `get_vga_properties` and move it into a > separate public `get_default_vga_type` function. This way we can leave > all the other functions private to the QemuServer.pm module. > > > We add a new property in the VM status/current API result that includes > the display configurtion of the VM. This way we can check in the > frontend what to do with it. > > I chose a nested return value, as that makes it easier to add/move > additional display properties into it. > > Patch 1/4 moves the default display logic into its own public function > > Patch 2/4 adds the new display property. If not explicitly set in the VM > config, it will return the default value. > > Patch 3/4 implements the changes in the UI. The final result isn't > really a lot simpler on the UI side than in V4 where we had the extra > API call to the VM's config directly. Because we still need to wait for > the API call to finish when initially navigating to the VM. But we have > one fewer call. > > Patch 4/4 then introduces some changes to make loading of the console > faster if we just navigate in the submenu of a VM itself where we > already have the current status of a VM already cached. > > > Changes from > v7: rebased > v6: backend only: create new `get_default_vga_type` function. > v5: implement suggestions: > > * use get_vga_properties for default VGA > * UI: use helper to determine if serial display > > Aaron Lauterer (2): > add new public get_default_vga_type function > api: status/current: add display property > > src/PVE/API2/Qemu.pm | 13 +++++++++++++ > src/PVE/QemuServer.pm | 29 ++++++++++++++++++++++------- > 2 files changed, 35 insertions(+), 7 deletions(-) _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console 2025-10-27 14:48 ` [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Michael Köppl @ 2025-11-03 21:04 ` Aaron Lauterer 0 siblings, 0 replies; 8+ messages in thread From: Aaron Lauterer @ 2025-11-03 21:04 UTC (permalink / raw) To: Proxmox VE development discussion, Michael Köppl; +Cc: pve-devel thanks for catching that line in 3/4 that got added by accident! I sent a v9 that fixes that https://lore.proxmox.com/pve-devel/20251103210233.2432522-2-a.lauterer@proxmox.com/T/#ma0b9e1e65e336734f20dc993799dbb743de25da7 On 2025-10-27 15:48, Michael Köppl wrote: > Gave this another quick spin, just to be sure. Tested the following: > - Created VM with serial terminal as display > - xtermjs was used for serial > - Switching to "default" or explicitly to "Standard VGA" uses noVNC > - Switching to "SPICE" uses remote-viewer > > Seems to work as advertised. Left a comment regarding one code change on > 3/4. > > Renewing the T-b just in case: > Tested-by: Michael Köppl <m.koeppl@proxmox.com> > > On Fri Oct 3, 2025 at 5:00 PM CEST, Aaron Lauterer wrote: >> Here is a new verson for this fix. Nothing changed since v7 expect >> rebasing. Quite a bit happened since then, but the rebase should be fine >> by now. I gave it a quick test on my workstation and things seem to work >> as expected. A quick test by someone else might be warranted though, >> just to be safe. I added all the R-b and T-b tags that were added on v7 >> direclty into each patch. Hopefully as they should be (chronological). >> >> The old cover letter that explains what is happening below: >> >> After the feedback from Fiona on v6 and some >> off-list discussion, we decided to improve the backend part by moving >> the "default" logic out of `get_vga_properties` and move it into a >> separate public `get_default_vga_type` function. This way we can leave >> all the other functions private to the QemuServer.pm module. >> >> >> We add a new property in the VM status/current API result that includes >> the display configurtion of the VM. This way we can check in the >> frontend what to do with it. >> >> I chose a nested return value, as that makes it easier to add/move >> additional display properties into it. >> >> Patch 1/4 moves the default display logic into its own public function >> >> Patch 2/4 adds the new display property. If not explicitly set in the VM >> config, it will return the default value. >> >> Patch 3/4 implements the changes in the UI. The final result isn't >> really a lot simpler on the UI side than in V4 where we had the extra >> API call to the VM's config directly. Because we still need to wait for >> the API call to finish when initially navigating to the VM. But we have >> one fewer call. >> >> Patch 4/4 then introduces some changes to make loading of the console >> faster if we just navigate in the submenu of a VM itself where we >> already have the current status of a VM already cached. >> >> >> Changes from >> v7: rebased >> v6: backend only: create new `get_default_vga_type` function. >> v5: implement suggestions: >> >> * use get_vga_properties for default VGA >> * UI: use helper to determine if serial display >> >> Aaron Lauterer (2): >> add new public get_default_vga_type function >> api: status/current: add display property >> >> src/PVE/API2/Qemu.pm | 13 +++++++++++++ >> src/PVE/QemuServer.pm | 29 ++++++++++++++++++++++------- >> 2 files changed, 35 insertions(+), 7 deletions(-) > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-11-03 21:03 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-03 15:00 [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer 2025-10-03 15:00 ` [pve-devel] [PATCH qemu-server v8 1/2] add new public get_default_vga_type function Aaron Lauterer 2025-10-03 15:00 ` [pve-devel] [PATCH qemu-server v8 2/2] api: status/current: add display property Aaron Lauterer 2025-10-03 15:00 ` [pve-devel] [PATCH manager v8 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs Aaron Lauterer 2025-10-27 14:42 ` Michael Köppl 2025-10-03 15:00 ` [pve-devel] [PATCH manager v8 4/4] ui: console: check on activate if display info for VMs is present Aaron Lauterer 2025-10-27 14:48 ` [pve-devel] [PATCH qemu-server, manager v8 0/4] fix #1926 autodetect xtermjs or novnc for VM console Michael Köppl 2025-11-03 21:04 ` Aaron Lauterer
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.