From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id BA63E1FF0E9 for ; Thu, 16 Jul 2026 12:20:54 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 22E2A21387; Thu, 16 Jul 2026 12:20:54 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 16 Jul 2026 12:20:48 +0200 Message-Id: From: "Max R. Carrara" Subject: Re: [PATCH common/manager/proxmox-widget-toolkit/storage 00/13] GUI Support for Custom Storage Plugins To: "Ciro Iriarte" , X-Mailer: aerc 0.18.2-0-ge037c095a049 References: <20260623143402.772452-1-m.carrara@proxmox.com> <6a5791cb.f6216423.106138.670b@mx.google.com> In-Reply-To: <6a5791cb.f6216423.106138.670b@mx.google.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784197229354 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.175 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust 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: NYPELLKZU2EFNAX22YXZZPNJGD26MPMH X-Message-ID-Hash: NYPELLKZU2EFNAX22YXZZPNJGD26MPMH X-MailFrom: m.carrara@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 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 Wed Jul 15, 2026 at 3:57 PM CEST, Ciro Iriarte wrote: > Hi Max, list, > > For context: I had independently drafted an RFC for the same problem > (schema-driven storage dialog, Bugzilla #7808) before Dominik kindly > pointed me at this series. Our designs converge -- derive the dialog > from the already-registered SectionConfig property + options() rather > than duplicating a separate GUI schema -- and yours is already > implemented and tested, so there is no reason to carry a competing > proposal. I am closing #7808 as a duplicate of #3420 and consolidating > my requirements here instead. Big +1 on the direction. Hi Ciro, thank you very much for your interest and initiative! It's much appreciated. Thomas already covered everything (better than I ever could), but there are a few things I'd like to add as well. > > I maintain a third-party enterprise Fibre Channel plugin > (pve-FCLUPlugin, reference driver for Hitachi VSP) that today survives > by injecting JS into index.html.tpl -- exactly the workaround this > series removes -- so I have a concrete stake in it. The series already > covers the core (title/label, fixed->read-only, sensitive->Unchanged, > enum, multiline, min/max, defaults); everything below is a follow-up, > not a blocker. Prioritized by how much it decides whether an enterprise > plugin can drop its JS injection entirely: > > P1 -- Is a sanctioned escape hatch for custom widgets/validators in > scope, now or later? > Pure schema-derivation cannot express a "Test connection" button, an > async combo populated from the array (pool IDs, host groups), or a > cross-field validator. My worry is that without one blessed override > path, advanced plugins stay on index.html.tpl injection -- the very > thing this series eliminates -- for anything beyond simple fields. Woul= d > you be open to a plugin registering an ExtJS component (e.g. a public > registerStoragePlugin() / xtype hook) that the generic panel uses when > present and falls back to schema derivation otherwise? For a plugin lik= e > mine this is the item that decides whether the mechanism can be adopted > wholesale, so I would value your read on whether it belongs in this > series or a follow-up. I want to mention here that you can also make use of the hook methods that PVE::Storage::Plugin provides, if you aren't doing that already, in order to perform whatever checks you'd like. There are currently: * `on_add_hook()` * `on_update_hook_full()` * `on_update_hook()` (legacy, prefer the above) * `on_delete_hook()` I assume you have already implemented this, but `check_connection()` would be the preferred method to test whether connecting to the storage works -- this also runs as part of `PVE::Storage::activate_storage()` on creation (but only if the storage is enabled for the given node). FWIW, you could also try experimenting with performing your checks in `on_add_hook()` in addition to what you already have in `check_connection()`, but that shouldn't really make much of a difference. `check_connection()` is the preferred way to go, and `on_add_hook()` is not a replacement for it. > > P2 -- A per-property "advanced" (and optional group/section) hint for > custom plugins. > The advanced-section split is currently hardcoded for inbuilt > properties (preallocation, SAVC). Real drivers carry many options -- > mine has management endpoint, credentials, pool, ldev_range, > host_group_prefix, control_plane, QoS, consistency-group -- and a flat > field list buries the rarely-changed ones. Proposal: honor an > `advanced` (and optionally a `group`) key per property in the derived > schema, the same treatment inbuilt preallocation/SAVC already get. Actually, I like this suggestion a lot, so I might have a look whether I can add support for an "advanced" hint in v2. I'm not too sure yet about the group/section hint, since we don't really have any sections / groups in the main ("General") storage config tab itself. Though, one thing that we could perhaps support in the future is adding new tabs to the storage config dialogue -- most storage types only show "General" and "Backup Retention" currently, but I don't think it should be too hard to allow adding custom tabs through the schema that otherwise follow the same logic as the "General" tab. In any case, this is out of scope for this series. > > P3 -- Conditional / dependent field visibility. > Many options are only meaningful given another: an Ops Center CM URL + > credentials only when control_plane =3D ops-center-cm, QoS limits only > when QoS is enabled. Static derivation shows everything unconditionally= . > Proposal: a lightweight `show-if`/`depends` hint (field =3D value) driv= ing > visibility/enablement, keeping the server as source of truth and > avoiding custom JS for the common case. > > P4 -- Client-side validation from format/pattern beyond `multiline`. > Only the `multiline` format is mapped; string `pattern`/`format` > (ip-address, and driver-specific formats like an ldev_range "256-511" > or colon-hex) fall back to a plain textfield, so the user only learns > of a bad value on submit. Proposal: map `pattern` -> ExtJS vtype/regex > and reuse existing PVE formats (ip-address, etc.). Integer > minimum/maximum already flow through -- thanks for that. Converting the `pattern` regex to a different kind of regex is probably infeasible unfortunately, unless we perhaps limit ourselves to a common subset of the Perl and ECMAScript regex dialects or find some other kind of workaround. Such a subset is described here: https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-01#name-reg= ular-expressions However, I'm not sure how well we honor this subset, since it's not strictly required to use the ECMA-262 regex dialect (or a subset thereof). > > P5 -- Distinct enum display labels. > comboItems currently map [value, value], so operators see raw tokens > (e.g. "ops-center-cm") rather than a human label ("Ops Center > Configuration Manager"). Proposal: allow a verbose-label map alongside > enum (e.g. an enum-descriptions list, or reuse verbose_description). > > P6 -- A per-plugin onlineHelp/docs anchor. > Inbuilt dialogs carry a Help button to the docs; custom plugins have > nowhere to point operators. Proposal: honor an onlineHelp anchor in the > plugin metadata so the generic panel renders the standard Help button. > > I will run the series against pve-FCLUPlugin on our lab and follow up > with a Tested-by separately. Thanks for driving this -- it closes a > long-standing gap for third-party storage vendors. > > Thanks, > Ciro Thanks a lot for testing this, it's much appreciated! Please do share any feedback you might have, as it would most likely be very valuable for us to drive this forward. - Max