* [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console
@ 2025-04-08 12:27 Aaron Lauterer
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 1/4] QemuServer: add new public get_default_vga_type function Aaron Lauterer
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Aaron Lauterer @ 2025-04-08 12:27 UTC (permalink / raw)
To: pve-devel
Sorry for the noise, 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
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
qemu-server: Aaron Lauterer (2):
QemuServer: add new public get_default_vga_type function
api: status/current: add display property
PVE/API2/Qemu.pm | 13 +++++++++++++
PVE/QemuServer.pm | 29 ++++++++++++++++++++++-------
2 files changed, 35 insertions(+), 7 deletions(-)
manager: Aaron Lauterer (2):
fix #1926 ui: vm console: autodetect novnc or xtermjs
ui: console: check on activate if display info for VMs is present
www/manager6/Utils.js | 4 +++
www/manager6/VNCConsole.js | 60 ++++++++++++++++++++++++++-----------
www/manager6/qemu/Config.js | 7 ++++-
3 files changed, 53 insertions(+), 18 deletions(-)
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH qemu-server v7 1/4] QemuServer: add new public get_default_vga_type function
2025-04-08 12:27 [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer
@ 2025-04-08 12:27 ` Aaron Lauterer
2025-04-08 13:22 ` Fiona Ebner
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 2/4] api: status/current: add display property Aaron Lauterer
` (4 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Aaron Lauterer @ 2025-04-08 12:27 UTC (permalink / raw)
To: pve-devel
by moving that part out of get_vga_properties.
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>
---
changes since 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.
PVE/QemuServer.pm | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ccdceed..cc08f6f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3414,6 +3414,27 @@ my sub print_ovmf_drive_commandlines {
return ("if=pflash,unit=0,format=raw,readonly=on,file=$ovmf_code", $var_drive_str);
}
+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) = @_;
@@ -3423,13 +3444,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.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH qemu-server v7 2/4] api: status/current: add display property
2025-04-08 12:27 [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 1/4] QemuServer: add new public get_default_vga_type function Aaron Lauterer
@ 2025-04-08 12:27 ` Aaron Lauterer
2025-04-08 13:22 ` Fiona Ebner
2025-04-08 12:27 ` [pve-devel] [PATCH manager v7 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs Aaron Lauterer
` (3 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Aaron Lauterer @ 2025-04-08 12:27 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>
---
changes since
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.
PVE/API2/Qemu.pm | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 626cce4..d7305d5 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3074,6 +3074,16 @@ __PACKAGE__->register_method({
enum => ['vnc'],
optional => 1,
},
+ display => {
+ description => "Display settings",
+ type => 'object',
+ properties => {
+ type => {
+ description => "Display type configured",
+ type => 'string',
+ },
+ },
+ },
},
},
code => sub {
@@ -3087,8 +3097,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.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager v7 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs
2025-04-08 12:27 [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 1/4] QemuServer: add new public get_default_vga_type function Aaron Lauterer
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 2/4] api: status/current: add display property Aaron Lauterer
@ 2025-04-08 12:27 ` Aaron Lauterer
2025-04-08 12:27 ` [pve-devel] [PATCH manager v7 4/4] ui: console: check on activate if display info for VMs is present Aaron Lauterer
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Aaron Lauterer @ 2025-04-08 12:27 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>
---
changes since:
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 | 52 +++++++++++++++++++++++++------------
www/manager6/qemu/Config.js | 7 ++++-
3 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index a1bcbe7e..d6fd6a9e 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1986,6 +1986,10 @@ Ext.define('PVE.Utils', {
);
return Ext.htmlEncode(description);
},
+
+ isSerialDisplay: function(display) {
+ return display?.startsWith('serial') ?? false;
+ },
},
singleton: true,
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..1598adf8 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,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');
@@ -467,6 +470,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' &&
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager v7 4/4] ui: console: check on activate if display info for VMs is present
2025-04-08 12:27 [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer
` (2 preceding siblings ...)
2025-04-08 12:27 ` [pve-devel] [PATCH manager v7 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs Aaron Lauterer
@ 2025-04-08 12:27 ` Aaron Lauterer
2025-04-08 12:49 ` [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Hannes Duerr
2025-04-09 9:14 ` Michael Köppl
5 siblings, 0 replies; 9+ messages in thread
From: Aaron Lauterer @ 2025-04-08 12:27 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>
---
changes since
v6: none
v5:
* use new helper to check if serial
www/manager6/VNCConsole.js | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
index 3371c923..200145c0 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.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console
2025-04-08 12:27 [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer
` (3 preceding siblings ...)
2025-04-08 12:27 ` [pve-devel] [PATCH manager v7 4/4] ui: console: check on activate if display info for VMs is present Aaron Lauterer
@ 2025-04-08 12:49 ` Hannes Duerr
2025-04-09 9:14 ` Michael Köppl
5 siblings, 0 replies; 9+ messages in thread
From: Hannes Duerr @ 2025-04-08 12:49 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
Created a VM, started it and opened the console -> noVNC was selected
added a serial device and selected Serial output as Display -> xtermjs
was selected
selected SPICE as Display -> noVNC was selected
the patches behaved as i would have expected, please consider them
Tested-by: Hannes Duerr <h.duerr@proxmox.com>
On 4/8/25 14:27, Aaron Lauterer wrote:
> Sorry for the noise, 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
> 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
>
> qemu-server: Aaron Lauterer (2):
> QemuServer: add new public get_default_vga_type function
> api: status/current: add display property
>
> PVE/API2/Qemu.pm | 13 +++++++++++++
> PVE/QemuServer.pm | 29 ++++++++++++++++++++++-------
> 2 files changed, 35 insertions(+), 7 deletions(-)
>
>
> manager: Aaron Lauterer (2):
> fix #1926 ui: vm console: autodetect novnc or xtermjs
> ui: console: check on activate if display info for VMs is present
>
> www/manager6/Utils.js | 4 +++
> www/manager6/VNCConsole.js | 60 ++++++++++++++++++++++++++-----------
> www/manager6/qemu/Config.js | 7 ++++-
> 3 files changed, 53 insertions(+), 18 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] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v7 1/4] QemuServer: add new public get_default_vga_type function
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 1/4] QemuServer: add new public get_default_vga_type function Aaron Lauterer
@ 2025-04-08 13:22 ` Fiona Ebner
0 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2025-04-08 13:22 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
Nit: please don't use "QemuServer" as a prefix in the commit subject.
That contains no additional information (for humans, but also machines
can already see the modified file).
Am 08.04.25 um 14:27 schrieb Aaron Lauterer:
> by moving that part out of get_vga_properties.
>
> 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>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index ccdceed..cc08f6f 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3414,6 +3414,27 @@ my sub print_ovmf_drive_commandlines {
> return ("if=pflash,unit=0,format=raw,readonly=on,file=$ovmf_code", $var_drive_str);
> }
>
> +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);
Nit: could have used the PVE::QemuServer::Machine::extract_version() helper.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v7 2/4] api: status/current: add display property
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 2/4] api: status/current: add display property Aaron Lauterer
@ 2025-04-08 13:22 ` Fiona Ebner
0 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2025-04-08 13:22 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
Am 08.04.25 um 14:27 schrieb Aaron Lauterer:
> 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>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console
2025-04-08 12:27 [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer
` (4 preceding siblings ...)
2025-04-08 12:49 ` [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Hannes Duerr
@ 2025-04-09 9:14 ` Michael Köppl
5 siblings, 0 replies; 9+ messages in thread
From: Michael Köppl @ 2025-04-09 9:14 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
Tested this by creating a VM with "Serial terminal 0" set as display
during creation and then changing the display option in the hardware menu:
- Keeping serial terminal -> xtermjs is used
- Changing to default -> noVNC is used
- Changing to SPICE -> remote-viewer is used
The patches behave as I would expect them to and changing the display
option results in the correct console being used even if the serial port
is still connected as hardware.
I also had a look at the implementation and, since the suggested changes
from the previous versions have been made, did not notice anything I
would change.
So please consider this:
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
On 4/8/25 14:27, Aaron Lauterer wrote:
> Sorry for the noise, 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
> 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
>
> qemu-server: Aaron Lauterer (2):
> QemuServer: add new public get_default_vga_type function
> api: status/current: add display property
>
> PVE/API2/Qemu.pm | 13 +++++++++++++
> PVE/QemuServer.pm | 29 ++++++++++++++++++++++-------
> 2 files changed, 35 insertions(+), 7 deletions(-)
>
>
> manager: Aaron Lauterer (2):
> fix #1926 ui: vm console: autodetect novnc or xtermjs
> ui: console: check on activate if display info for VMs is present
>
> www/manager6/Utils.js | 4 +++
> www/manager6/VNCConsole.js | 60 ++++++++++++++++++++++++++-----------
> www/manager6/qemu/Config.js | 7 ++++-
> 3 files changed, 53 insertions(+), 18 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] 9+ messages in thread
end of thread, other threads:[~2025-04-09 9:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-08 12:27 [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Aaron Lauterer
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 1/4] QemuServer: add new public get_default_vga_type function Aaron Lauterer
2025-04-08 13:22 ` Fiona Ebner
2025-04-08 12:27 ` [pve-devel] [PATCH qemu-server v7 2/4] api: status/current: add display property Aaron Lauterer
2025-04-08 13:22 ` Fiona Ebner
2025-04-08 12:27 ` [pve-devel] [PATCH manager v7 3/4] fix #1926 ui: vm console: autodetect novnc or xtermjs Aaron Lauterer
2025-04-08 12:27 ` [pve-devel] [PATCH manager v7 4/4] ui: console: check on activate if display info for VMs is present Aaron Lauterer
2025-04-08 12:49 ` [pve-devel] [PATCH qemu-server, manager v7 0/4] fix #1926 autodetect xtermjs or novnc for VM console Hannes Duerr
2025-04-09 9:14 ` Michael Köppl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal