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 601C31FF13B for ; Wed, 22 Apr 2026 17:13:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A123423C42; Wed, 22 Apr 2026 17:13:23 +0200 (CEST) Message-ID: <4cf9b5ee-5a27-4bf1-a32f-645660b0b58a@proxmox.com> Date: Wed, 22 Apr 2026 17:13:14 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH pve-storage 7/7] api: add /nodes//storage//identity route To: Lukas Wagner , Fiona Ebner , pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com References: <20260415115817.348947-1-l.wagner@proxmox.com> <20260415115817.348947-8-l.wagner@proxmox.com> <262d3355-b8ec-470c-8fed-32a7b151fee4@proxmox.com> <231a34ce-06cb-40b7-b0d8-6238e368e60c@proxmox.com> <85e5c377-ab04-407a-9670-7552591791f1@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776870706511 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.048 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 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: PMZTD6VCN524QVTUDAFT3GITQRJ5FO5N X-Message-ID-Hash: PMZTD6VCN524QVTUDAFT3GITQRJ5FO5N X-MailFrom: t.lamprecht@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: Am 22.04.26 um 09:47 schrieb Lukas Wagner: > So in summary, as of now, we have > - the storage API route is called `identity` and the response contains > `pbs-instance-id` > - the PBSClient method is called `get_server_identity`, returning a > hash that contains a `pbs-instance-id` > - in PBS, the route is `/nodes/localhost/server-identity` and the > response contains a `pbs-instance-id` field > - the PBS client binary has a `server-identity` sub-command > > In your reply you stated a preference for `instance-id`, but also I did > not understand it as concrete instructions for the next version. With my > (hopefully) more elaborated reasoning above, is there any one of those > that you'd like me change before this gets applied (and if so, what > exactly should they be named?) -- just to avoid any misunderstandings > and churn for the next version of this series. OK, to close this out with something concrete instead of all to much naming bike-shed back-and-forth: First, and largely separate from the naming: FWICT it really wouldn't be much extra work to wire up a proper plugin method in pve-storage right away. Bump APIVER + APIAGE in PVE::Storage, add a get_identity() (or similar) plugin method with a default impl that dies or returns undef, and have PBSPlugin override it. Here we then just call `$plugin->get_identity(...)` and the `if $scfg->{type} ne 'pbs'` special-case can be dropped. You wouldn't have to implement the method on any non-PBS plugin for now though; the main point is that the API surface isn't hard-wired to `pbs` at the API endpoint level any more, so opting another type in later mostly needs implementing the method in the respective plugin. With that in place IMO the response properties should be generic too, otherwise the plugin-method is a bit odd combined with using `pbs-instance-id` here, one option might be a sort of oneOf schema, but I'd actually rather go with an sorta "adjacently-tagged" (like serde would call it) `{ type: "", id: "" }` from the endpoint. PDM (or other clients) can then reconstruct and, if needed, also correctly verify the full identifier itself. The `pbs-instance-id` field name can stay PBS-side (ServerIdentity struct + PBS API return), where it's PBS-specific by construction, it just shouldn't sneak into the generic pve-storage endpoint (but in case you strongly favor that: I'd not be totally opposed to oneOf). While this has IMO no objective true answer, the consolidated naming I'd use, to make my reply a bit less ambiguous: - PBS API: `/nodes//identity` - drop `server-`, reasoning: we're on the server, the qualifier is redundant there. - PBS client CLI + Perl PBSClient: keep `server-identity` / `get_server_identity`, client-side the qualifier is worth it to correctly relay what this identifies. - pve-storage API: keep bare `identity`, adding server isn't true for all storage types after all and I do not find instance-id that much better to really request it, especially given that others found the "identity" variant better I'm fine with it. - pve-storage response: `{ type, id }` via the new plugin method. - PBS-internal `ServerIdentity { pbs_instance_id }`: unchanged. btw. CC'ing Max, in case this overlaps with his storage UI/schema work - definitively not a blocker here, just so it's on his radar.