public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models
Date: Tue, 24 Mar 2026 10:27:47 +0100	[thread overview]
Message-ID: <cebbd87a-d6fd-4e70-904f-eec906f6c1b5@proxmox.com> (raw)
In-Reply-To: <itxpdokw3sek7fs2e2dhc7olkw5pyme6mn77fvhqyg4rqyx6wz@smbpv3clke7o>

Am 23.03.26 um 5:09 PM schrieb Arthur Bied-Charreton:
> On Mon, Mar 23, 2026 at 05:04:15PM +0100, Arthur Bied-Charreton wrote:
>> On Mon, Mar 23, 2026 at 03:46:45PM +0100, Fiona Ebner wrote:
>>> Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton:
>>>
> [...]
>>> Having the 'model' endpoint below here seems wrong to me, for two reasons:
>>>
>>> 1. Since the 'cpu' endpoint already returns a result other than an index
>>> of sub-endpoints, it cannot at the same time show in its result that the
>>> sub-endpoint exists, and this breaks the directory structure.
>>>
>>> 2. It's below /nodes/ but it's touching cluster-wide configuration.
>>>
>>> I think we should just have a new, dedicated endpoint, maybe
>>> /cluster/qemu/custom-cpu-models? And then, we might want to drop the
>>> need for specifying a 'custom-' prefix when using the calls?
>> I agree that the current state feels weird. I went with the old patch's
>> approach but I should have rethought it a bit more, especially the
>> custom- prefix, the URL already carries that so it's quite redundant. 
>>
>> I like the idea of a new /cluster/qemu/custom-cpu-models route, will
>> move the endpoint over to pve-manager for v2 and drop the custom- prefix
>> in the process. 
>>
>> By that logic, the cpu-flags endpoint should probably also be cluster-wide, 
>> since it returns data for all nodes in the cluster? It technically takes
>> a node parameter, but that is ignored by the handler (both before and
>> after this series). What do you think?
>>
> Just realizing that this would be a breaking change since that endpoint
> is pre-existing, would be better not to touch it, my bad...

No worries! If we would have a good reason to move it, we could add a
second endpoint, deprecate the old one and drop it with the next major
release.

But maybe we should keep that endpoint as-is too, since it is concerned
with the VM-specific flags on the node (it does depend on the installed
qemu-server version, even if new VM-specific flags are rarely addded),
and introduce a new one below /cluster/qemu/ for the cluster-wide flags
querying? We could still think about deprecating it in the long run and
also sending the VM-specific flags via kv and have them via the
cluster-wide endpoint, but not sure about that and I don't think it's
too relevant for the series. How does that sound?




  reply	other threads:[~2026-03-24  9:28 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-12  8:40 [PATCH manager/qemu-server 0/8] Add API and UI " Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 1/8] ui: VMCPUFlagSelector: Fix unknownFlags behaviour Arthur Bied-Charreton
2026-03-25 15:57   ` Fiona Ebner
2026-03-26 13:47     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 2/8] ui: CPUModelSelector: Fix dirty state on default Arthur Bied-Charreton
2026-03-26  9:53   ` Fiona Ebner
2026-03-26 14:14     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 3/8] ui: CPUModelSelector: Allow filtering out custom models Arthur Bied-Charreton
2026-03-26  9:59   ` Fiona Ebner
2026-03-26 14:17     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor Arthur Bied-Charreton
2026-03-26 15:10   ` Fiona Ebner
2026-03-27  9:23     ` Arthur Bied-Charreton
2026-03-27  9:32       ` Fiona Ebner
2026-03-27  9:34         ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH pve-manager 5/8] ui: Add CPU flag editor for custom models Arthur Bied-Charreton
2026-03-26 15:22   ` Fiona Ebner
2026-03-27  9:34     ` Arthur Bied-Charreton
2026-03-26 15:40   ` Maximiliano Sandoval
2026-03-27  7:48     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH qemu-server 6/8] qemu: Add helpers for new custom models endpoints Arthur Bied-Charreton
2026-03-20 17:20   ` Fiona Ebner
2026-03-23  6:56     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH qemu-server 7/8] api: qemu: Extend cpu-flags endpoint to return actually supported flags Arthur Bied-Charreton
2026-03-20 17:20   ` Fiona Ebner
2026-03-23  7:25     ` Arthur Bied-Charreton
2026-03-12  8:40 ` [PATCH qemu-server 8/8] api: qemu: Add CRUD handlers for custom CPU models Arthur Bied-Charreton
2026-03-23 14:46   ` Fiona Ebner
2026-03-23 16:04     ` Arthur Bied-Charreton
2026-03-23 16:10       ` Arthur Bied-Charreton
2026-03-24  9:27         ` Fiona Ebner [this message]
2026-03-26 14:54 ` [PATCH manager/qemu-server 0/8] Add API and UI " Fiona Ebner
2026-03-27 13:07   ` Arthur Bied-Charreton
2026-03-27 13:28     ` Fiona Ebner

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=cebbd87a-d6fd-4e70-904f-eec906f6c1b5@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=a.bied-charreton@proxmox.com \
    --cc=pve-devel@lists.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