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 13DD11FF13B for ; Wed, 22 Apr 2026 09:49:15 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 40758DB86; Wed, 22 Apr 2026 09:49:11 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 22 Apr 2026 09:48:36 +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> In-Reply-To: <85e5c377-ab04-407a-9670-7552591791f1@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776844030321 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. [proxmox.com] Message-ID-Hash: FZR252T2G7QBE7WDAFDHGZH7O6GIHCMU X-Message-ID-Hash: FZR252T2G7QBE7WDAFDHGZH7O6GIHCMU 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 Tue Apr 21, 2026 at 3:52 PM CEST, Thomas Lamprecht wrote: > Am 21.04.26 um 15:05 schrieb Lukas Wagner: >> On Tue Apr 21, 2026 at 2:35 PM CEST, Thomas Lamprecht wrote: >>> Am 17.04.26 um 11:10 schrieb Lukas Wagner: >>>> On Fri Apr 17, 2026 at 10:54 AM CEST, Fiona Ebner wrote: >>>>> Am 15.04.26 um 1:57 PM schrieb Lukas Wagner: >>>>>> @@ -308,6 +308,7 @@ __PACKAGE__->register_method({ >>>>>> { subdir =3D> 'download-url' }, >>>>>> { subdir =3D> 'file-restore' }, >>>>>> { subdir =3D> 'import-metadata' }, >>>>>> + { subdir =3D> 'identity' }, >>>>> >>>>> Just bike-shedding, but I'm wondering if 'identifier' would be slight= ly >>>>> more natural? >>>> >>>> I can change it if you prefer, but I think I prefer 'identity', if I'm >>>> honest. >>> Why not "instance-id"? it's used widely in this series and is IMO quite >>> descriptive. >>=20 >> It was 'instance-id' in the RFC, but in the patch notes I mentioned that >> a more generic term could maybe be useful in case we want need to reuse = the >> concept of a identity for other storages as well, to which Chris agreed. > > IMO identity and instance-id are both basically just as generic, or > at least one isn't so much more specific that the difference actually > relays to the user something. >> After some brief discussion with Chris I went for 'identity' [1]. >>=20 >> In the PBS-client part of these patches, Chris mentioned that >> 'instance-id' alone could be misleading [2, 3], which is why I went with >> the names that I used in this series. > > Well, FWICT Chris didn't really mentioned that what the ID of the > "instance-id" part actually pointed at could be potentially misleading, > which I can somewhat relate to as concern, some might indeed expect this > ID/identity to pop up in logs or configs as "for this client instance/id)= . > But for one that's IMO not specific to using "instance-id", that's often = an > issue when talking about server properties in client, and IMO using "iden= tity" > doesn't really fixes this either, rather one would need to specify that > it's the (instance) ID of the server, e.g., by using "server-instance-id"= or > "server-identity" - again, both are fine in general, but there's really n= o > difference for the presented arguments here, important is to add the actu= al > subject of the id/identity/identifier/... over using some synonym or. > Sorry, I think could have been a bit more precise when wording my message. I did address Chris' concern by using the `server-` prefix in PBSClient and proxmox-backup-client (and then for the sake of consistency, I named the API route in PBS the same). The change from `instance-id` to `identity` was my doing, mostly pushed from the pve-storage route side of things. In the RFC I mentioned in the patch notes that maybe it might make sense to use a relatively generic term for the storage API route (to which Chris agreed), as to make it better reusable for other types of storage if the need arises. I went with `identity` here because it's IMO more generic than `instance-id` (as in, give me information about the identity of this storage) -- but I guess this is quite subjective. I did not use the 'server' prefix in the storage API route due to its more generic nature; after all a storage might be local (sure, technically also a server, but it felt weird to me). So both combined resulted in `(server)-identity`. I mentioned my intentions for the name change in [1], to which Chris agreed. [1] https://lore.proxmox.com/pve-devel/98f893be-871c-4a46-9b06-3ee17978131d= @proxmox.com/ >> If you prefer the more specific 'instance-id', I can of course change it >> back to that. Would you then also propose changing the terms used in >> PBS? There we now the 'server-identity' sub-commands for manager and >> client and the '/nodes/.../server-identity' routes, which *return* the >> `pbs-instance-id` field in the response. > > IMO it can be more than fine to e.g. use "xyz" in the server and > "server-xyz" for that thing in the client, albeit not a must and can have > a slight headache to hold up that difference when using shared (struct) > types. > > That said, I like the ring of instance-id and it somewhat fits in naming > with the existing machine-id, but that really isn't a deciding factor on > it's own. > My main point is that just omitting "instance" and spelling out ID as > identity IMO doesn't solves the - IMO not big but definitively real - > concern that Chris pointed out, for that IMO "server-identity", > "server-id", "server-instance-id" would be all much better, as they > actually relay where the ID comes from, i.e., what it actually identifies= . > 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. I'm not at all against `(server-)instance-id`, after all it was my first choice in the RFC. In the end, I don't have very hard feelings about the naming, I'd just like to get this change in for the next releases. Thanks!