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 A7DB51FF140 for ; Fri, 27 Mar 2026 10:23:08 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3B0C0374A4; Fri, 27 Mar 2026 10:23:29 +0100 (CET) Date: Fri, 27 Mar 2026 10:23:22 +0100 From: Arthur Bied-Charreton To: Fiona Ebner Subject: Re: [PATCH pve-manager 4/8] ui: Add basic custom CPU model editor Message-ID: <3pvqrvjm7srqfaaok3o7ner4xa6s3sdqlw5thrhhxigdpvl4w5@wy5hunbsnjn7> References: <20260312084021.124465-1-a.bied-charreton@proxmox.com> <20260312084021.124465-5-a.bied-charreton@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774603354029 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.791 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 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: HLPHFEFCFZD6TGISET3C3JF56WWGC4VF X-Message-ID-Hash: HLPHFEFCFZD6TGISET3C3JF56WWGC4VF 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 Thu, Mar 26, 2026 at 04:10:34PM +0100, Fiona Ebner wrote: Thanks for the feedback! Regarding the onlineHelp feedback, I will add an anchor to CPU Type section in qm.adoc and link to that, qm_cpu is currently the closest we can link to otherwise. > Am 12.03.26 um 9:40 AM schrieb Arthur Bied-Charreton: > [...] > > + { > > + xtype: 'CPUModelSelector', > > + fieldLabel: gettext('Reported Model'), > > What about 'Base Model' with a tooltip that it's reported to the guest > (if that is even necessary)? I feel like 'Reported Model' doesn't make > it clear that the rest of the configuration is applied based off that model. > I agree that "Base Model" makes more sense than "Reported Model", however the latter is better aligned with the SectionConfig key. In order for pvesh to be consistent with the UI, we would need to expose `base-model` in the `custom-cpu-models` API and translate it to `reported-model` in the handlers. Which would however still not be consistent with the actual config file content and might lead to confusion for users who are/were manually editing the file. `reported-model` seems to be quite sticky, changing the SectionConfig key looks like a pretty big refactor? What do you think? Would we be okay with the naming inconcistency, and if so at what level should the break happen? Otherwise we could keep "Reported Model" and add a tooltip explaining it to avoid confusion. > > + allowCustom: false, > > + name: 'reported-model', > > + }, [...] > > Currently, the confirm dialog shows: > "Are you sure you want to remove entry 'custom-nested-for-wsl'?" > Would be nicer along the lines of > "Are you sure you want to remove the custom CPU model 'nested-for-wsl'" > if that can be done without much effort. Otherwise, not too important. > It's possible, I thought one could only pass static strings to `proxmoxStdRemoveButton`'s `confirmMsg` property, but turns out it also accepts callbacks (`function(rec)`). Will be updated in v2. > > + }, [...] > > + { > > + xtype: 'pveCPUTypeView', > > + iconCls: 'fa fa-microchip', > > + title: gettext('Custom CPU models'), > > + itemId: 'cputypes', > > + }, > > I feel like this might better fit further below, after the directory and > resource mappings items. Makes sense, will move it down. > > I wonder if we should collect the two mappings and this in a common > section, but I can't come up with a good name right now, something akin > to "Guest Resources/Hardware"? But that is something to be further > discussed so should be ordered at the end of the series or as a follow-up. I appended a commit to v2 implementing this so we can iterate on it :)