public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Aaron Lauterer <a.lauterer@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH manager v4] fix #1926 ui: vm console: autodetect novnc or xtermjs
Date: Mon, 7 Apr 2025 18:29:51 +0200	[thread overview]
Message-ID: <b8d427d0-7129-4fd4-9c3e-3788c7754c89@proxmox.com> (raw)
In-Reply-To: <49bcd58f-aba2-4c1e-a6a0-d21828a335bf@proxmox.com>

sent a v5 
https://lore.proxmox.com/pve-devel/20250407162718.495812-1-a.lauterer@proxmox.com/

On  2025-04-03  13:03, Thomas Lamprecht wrote:
> Am 26.03.25 um 13:04 schrieb Aaron Lauterer:
>> 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?
> As vm_status already has all information parsed that required for this and
> already has a 'spice` boolean flag, it seems fine to handle that in
> vm_status. But it might be better to add a new slightly more general
> property where we can absorb the spice flag in the long run, like:
> 
> display: (serial;qxl;...)
> 
> or already default to a property format-string now, but
> 
> display: type=[serial;qxl;...]
> 
> but we can transform it to that later one too if we're unsure about
> potential additional data added here, besides maybe merging in the
> clipboard too – then it might be better to have something like:
> 
> user-interface: display=...[,clipboard=...][,...?]
> 
> But no hard feelings on that, maybe someone else has input here, else
> I probably would go for `display: (serial;qxl;...)` for now. We need
> to handle "allow-spice" (as spice virt-viewer can be used for more
> than just QXL) then in the frontend though once we drop the spice
> flag then.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

      reply	other threads:[~2025-04-07 16:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-25  9:18 Aaron Lauterer
2025-03-25 17:09 ` Thomas Lamprecht
2025-03-26 12:04   ` Aaron Lauterer
2025-04-03 11:03     ` Thomas Lamprecht
2025-04-07 16:29       ` Aaron Lauterer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8d427d0-7129-4fd4-9c3e-3788c7754c89@proxmox.com \
    --to=a.lauterer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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