On 2025-03-25 18:09, Thomas Lamprecht wrote:
Am 25.03.25 um 10:18 schrieb 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.
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 for VMs and will first fetch the VM
config and if the display is set to "serialX", will set the console to
xtermjs. Otherwise it will fall back to regular noVNC.
Since getting the VM config is an async API call, the code had to be
restructured so we can do the actual loading of the console after the
config has been fetched.
Therefore we now have the 'loadConsole()' function which will be called
when the UI item is activated and in the KVM case, after loading and
handling the VM config. It is guarded by the 'activated' and
'configLoaded' variables.
A bit late in, so sorry for that, but FWIW, one can also use our async API
request wrapper for that, i.e. mark the function this is in as async and
then use something like:
let config = await Proxmox.Async.api2({
url: `/api2/extjs/nodes/${me.nodename}/qemu/${me.vmid}/config?current=1`
});
That would probably reduce the amount of changes required and also
keep the code a bit more linear.
Btw. don't we have the config available in some call-sites? I.e., would
it make sense to allow overriding it already now? Because if we can
reduce the need for an extra API call due to information being already
available then I'd always favor that, having experienced working over
some high latency links and/or spotty & congested (wifi) connections.
We can improve this later on, but would be good to evaluate the status
quo already; FWIW I'd be open to look into querying the respective
properties in the cluster resource API endpoint, which would then be
available in the in-memory resource store and really cheap to query
from the frontend. As we already get some properties the backend side
would not really get more expensive either. Again, that does not has
to be a prerequisite for applying something like this, but IMO worth
to evaluate at some (soonish) point.
I did not find if we already have the full VM config already. AFAICT we
go from `qemu/Config.js` -> `VNCConsole.js`.
Only the status of the VM. As I mentioned in the comment below the
commit msg, the backend does check against the wrong config property for
this use-case.
So if we actually have the config already and I just couldn't find it,
point me to it :)
Otherwise, to avoid additional API calls, the other options we have are:
* change the backend check that populates `serial` in the status. It
currently checks against the presence of a serial device. But we need to
know if the display is set to serial, otherwise we get a false positive
if the serial device is used for a real physical serial device.
But I don't know where else (externally?) that might be used, therefore
I consider this a breaking change.
* extend the vm status to have the infos we need.
** property like "serialdisplay"
** a "display" property that contains the configured display option?
If we introduce a new property to vmstatus, the display one would most
likely be more generic and could be more useful in the future?
Initially we decided against that to keep in lean.
But if you think that having a bit more info in the vmstatus is better
than another API call, I can send another patch that also extends the
backend.
If we decide to keep the additional API call, I will send another one
trying the async wrapper.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
