From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 5C4BE1FF136 for ; Mon, 18 May 2026 13:21:03 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 704C1D297; Mon, 18 May 2026 13:21:02 +0200 (CEST) Date: Mon, 18 May 2026 13:20:55 +0200 From: Arthur Bied-Charreton To: Thomas Lamprecht Subject: Re: applied: [PATCH docs/manager/qemu-server v5 00/21] Add API and UI for custom CPU models Message-ID: References: <20260515092839.238064-1-a.bied-charreton@proxmox.com> <177909704332.3037978.4019137819812531064.b4-ty@b4> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <177909704332.3037978.4019137819812531064.b4-ty@b4> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1779103244710 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.493 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LOTSOFHASH 0.25 Emails with lots of hash-like gibberish POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: PWKSQ3X4OXJDTMLN4OQFDQH4CTZ2MAVC X-Message-ID-Hash: PWKSQ3X4OXJDTMLN4OQFDQH4CTZ2MAVC X-MailFrom: a.bied-charreton@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Mon, May 18, 2026 at 12:27:09PM +0200, Thomas Lamprecht wrote: > On Fri, 15 May 2026 11:28:17 +0200, Arthur Bied-Charreton wrote: > > This picks up and extends an old series [0] by Stefan Reiter. > > > > This series adds a full CRUD API and a UI editor for custom CPU models > > which allows users to manage them in the Datacenter interface rather > > than editing /etc/pve/virtual-guest/cpu-models.conf manually. > > > > It also improves on the existing VM CPU flags selector by providing a > > list of nodes supporting each flag to help gauge cluster compatibility. > > > > [...] > > Applied, thanks! > > [qemu-server]: > > [01/10] cpu config: rename CPU models config path variable > commit: 6cd77495450e586095ba1bfae09cd061599f894a > [02/10] cpu flags: move cpu flags-related utilities to their own module > commit: 71c42d38600595c7ea89901c0cb22529e74e09e3 > [03/10] cpu flags: compare against JSON::true when querying supported flags > commit: 8508fe8d987164cb771fa5df11589ca25563e549 > [04/10] cpu flags: normalize CPU flags to QEMU's format > commit: 52a3cfa08c9e45156a882d33d49bda8f481c62dd > [05/10] cpu flags: add helper querying CPU flags with nodes supporting them > commit: aba97f99dddd65d254af5a402b48f0a5444a046c > [06/10] cpu config: rename custom CPU model config loader > commit: ccb2c4787048c2610207cdcd7033f3eb6091a1c0 > [07/10] cpu config: add helpers to lock and write config > commit: 4483cfbee4ccee4b68ec1bd781085b4376acfcf3 > [08/10] cpu: register standard option for CPU format > commit: fcfe1c139e028f80244f8956881a7e3df75162bc > [09/10] api: cpu flags: improve flags list returned by endpoint > commit: 4154e623527ba6a1e8df8a87f28fdd3b7343920d > [10/10] custom cpu models: avoid redundant config load > commit: 9d8e6f7af6d0e97e19f56fae37d74923467bb86b > > [pve-manager]: > > [01/10] cluster: reorder imports > commit: c50ec74c3f10c42d493ee84d761b70a3dc49851a > [02/10] cluster: makefile: reorder perl sources and align backslashes > commit: e2ab0188a97772cc380cb974bc6afdda37c221e8 > [03/10] api: add endpoint querying available CPU flags cluster-wide > commit: 6ed37d55cb2a59855eb90350486f7a19c13547e4 > [04/10] api: add CRUD handlers for custom CPU models > commit: db32d51ec9f58a8fab5f5c16e33bc2c4ac022f88 > [05/10] ui: cpu model selector: allow filtering out custom models > commit: b3a515d25c276db64678d20661a5a395eaa603db > [06/10] ui: add basic custom CPU model editor > commit: e67867c9668b6724e32caa6a78ca83881501328d > [07/10] ui: cpu flags selector: add CPU flag editor for custom models > commit: 5589e4007457a751a16c81c38a93110a1ddedbab > [08/10] ui: cpu flags selector: allow filtering out flags supported on 0 nodes > commit: c5e48bb514eab33a03efbbfac94654c3c67c37f2 > [09/10] ui: cpu flags selector: add search bar for large lists of flags > commit: 41edb0b46326777a2b5c74b7e4c258a7f02d0161 > [10/10] ui: group custom CPU with resource mappings > commit: 48f151ad6b24c7803087194cd987ef532cb3c56b > > [pve-docs]: > > [1/1] qm: add anchor to "CPU Type" section > commit: 2cfbddac74d9227284fb7b25310137a2fc202eae > > A few notes from the fix-ups I made while applying, in case useful for future > submissions. Two UI patterns stood out as worth naming explicitly. > > When a backend property gets a UI, make the column header, form-field label, > API description, and docs anchor converge on one name. The custom CPU panel had > 'Phys-Bits' / 'Physical Address Bits' / 'phys-bits' across column / form / API, > 'Hidden' where users would say 'Hide Hypervisor', and 'HyperV-Vendor' (column) > vs 'Hyper-V Vendor' (form label). Each small mismatch costs a few seconds at > every support ticket or docs-grep. > > Co-locate filter controls with the data they filter, and make their defaults > context-aware. The flag editor originally had the accel toggle in a row below > the form and a 'supported by at least one node' checkbox under the grid - three > filter widgets in three different bands. Folding accel and the new per-node > multi-select into the grid's top toolbar next to the search field gives one > scannable filter band right above the data, with the blanket toggle remaining > below as a quick-disable. That blanket filter was also default-on, which on a > single-node cluster (where supported-on is empty for all flags) hides every > flag - defaults like that should be on for end-user editing but off for > cluster-wide curation where they actively hurt. > > The rest are smaller, mostly one-liners: > > - Tighten CRUD input schemas at the schema layer rather than trusting callers. > cputype accepted any string until I enforced pve-configid + maxLength=40, > re-verified after stripping the optional 'custom-' prefix, and rejected the > reserved name 'delete' (collides with PUT's delete= parameter). > - Prefer //= over = in property-merge helpers so a caller passing a stricter > schema isn't silently overwritten by the default; add_cpu_json_properties was > overwriting our stricter cputype format. > - Sibling endpoints should agree on return shape - the per-node cpu-flags > endpoint had 'description' as required while the new cluster-wide one can > omit it; mark optional to match. > - Any non-form widget inside a form panel needs submitValue: false + > isFormField: false. The CRUD Create dialog POSTed 'combo-NNNN-inputEl=...' > until the toolbar widgets were marked. > - Selection-bound buttons: Proxmox.button.Button with disabled: true auto-binds > to the grid's selModel; the bare Edit button was active with no row selected > and on an empty list. > - 'nested-virt' is a PVE shorthand resolved at VM start - whitelist it in > supported-on filters; x86-64-vN are PVE-internal abstract profiles > - tag with 'abstract' so the VM picker can hide them. > - 'Sys.Audit on /nodes' was misused as a 'trusted user' proxy for custom-CPU > assignment; replaced with Mapping.{Audit,Use,Modify} on /mapping/cpu/ > (matching PCI/USB/Dir) for both assignment and the listing endpoint. > - Perl style: 'keys $hash->%*' over 'keys %{$hash}' (the latter flattened by > accident here); compare to JSON::true directly, not the string '1'; optional > $errmsg on lock helpers matching the Mapping::* pattern lets callers surface > useful contention messages. > > In ProcessorEdit I also dropped a duplicate KVM checkbox and added a > hint pointing at the new per-node accel filter. Thanks a lot for the detailed feedback and the fixups, I will keep these patterns in mind for future submissions.