From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 9AD6C1FF13F for ; Thu, 23 Apr 2026 11:26:36 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 34800D500; Thu, 23 Apr 2026 11:26:33 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 23 Apr 2026 11:25:57 +0200 Message-Id: Subject: Re: [PATCH pve-storage 7/7] api: add /nodes//storage//identity route From: "Lukas Wagner" To: "Thomas Lamprecht" , "Lukas Wagner" , "Fiona Ebner" , , X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty 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> <4cf9b5ee-5a27-4bf1-a32f-645660b0b58a@proxmox.com> In-Reply-To: <4cf9b5ee-5a27-4bf1-a32f-645660b0b58a@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776936268505 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.004 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [store.com] Message-ID-Hash: NKFKLPNAS623TEFEIRIR3U3EVAEVP2LU X-Message-ID-Hash: NKFKLPNAS623TEFEIRIR3U3EVAEVP2LU X-MailFrom: l.wagner@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 Apr 22, 2026 at 5:13 PM CEST, Thomas Lamprecht wrote: > 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 >>=20 >> 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. Thanks for the elaborate response! Sounds good to me in general.=20 One last point, but maybe I'm overthinking this: To me, the { type: "", id: "(/)?@ e.g. store/some/name/space@dade89fe26104bc3858ae719c23594a7 This would then actually uniquely identify the PBS storage (as in, two storages with the same ID should always point to the same content), while still allowing PDM to parse the identifier to retrieve the pbs-instance-id, if needed. Happy to hear your thoughts on this.