public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Thomas Lamprecht" <t.lamprecht@proxmox.com>,
	"Lukas Wagner" <l.wagner@proxmox.com>,
	"Fiona Ebner" <f.ebner@proxmox.com>,
	<pbs-devel@lists.proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH pve-storage 7/7] api: add /nodes/<node>/storage/<storage>/identity route
Date: Wed, 22 Apr 2026 09:48:36 +0200	[thread overview]
Message-ID: <DHZIO6A6JJML.3W3N1PTAM19A6@proxmox.com> (raw)
In-Reply-To: <85e5c377-ab04-407a-9670-7552591791f1@proxmox.com>

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 => 'download-url' },
>>>>>>              { subdir => 'file-restore' },
>>>>>>              { subdir => 'import-metadata' },
>>>>>> +            { subdir => 'identity' },
>>>>>
>>>>> Just bike-shedding, but I'm wondering if 'identifier' would be slightly
>>>>> 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.
>> 
>> 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].
>> 
>> 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 "identity"
> 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 no
> difference for the presented arguments here, important is to add the actual
> 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!




  parent reply	other threads:[~2026-04-22  7:49 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-15 11:58 [PATCH common/proxmox{,-backup}/storage 0/7] establish unique instance-id for PBS nodes Lukas Wagner
2026-04-15 11:58 ` [PATCH proxmox 1/7] pbs-api-types: add ServerIdentity response type Lukas Wagner
2026-04-15 11:58 ` [PATCH proxmox 2/7] systemd: add support for machine-id generation Lukas Wagner
2026-04-15 11:58 ` [PATCH proxmox-backup 3/7] api: add /nodes/localhost/server-identity Lukas Wagner
2026-04-15 11:58 ` [PATCH proxmox-backup 4/7] client: add 'server-identity' sub-command Lukas Wagner
2026-04-15 11:58 ` [PATCH proxmox-backup 5/7] manager: add 'server-identity' subcommand Lukas Wagner
2026-04-15 11:58 ` [PATCH common 6/7] pbs-client: add support for the 'server-identity' command Lukas Wagner
2026-04-15 11:58 ` [PATCH pve-storage 7/7] api: add /nodes/<node>/storage/<storage>/identity route Lukas Wagner
2026-04-17  8:54   ` Fiona Ebner
2026-04-17  9:11     ` Lukas Wagner
2026-04-21 12:35       ` Thomas Lamprecht
2026-04-21 13:07         ` Lukas Wagner
2026-04-21 13:52           ` Thomas Lamprecht
2026-04-21 14:06             ` Christian Ebner
2026-04-22  7:48             ` Lukas Wagner [this message]
2026-04-22 15:13               ` Thomas Lamprecht
2026-04-23  9:25                 ` Lukas Wagner
2026-04-23 23:05                   ` Thomas Lamprecht
2026-04-24  6:56                     ` Lukas Wagner
2026-04-21 12:20 ` [PATCH common/proxmox{,-backup}/storage 0/7] establish unique instance-id for PBS nodes Christian Ebner
2026-04-24  8:31 ` superseded: " Lukas Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DHZIO6A6JJML.3W3N1PTAM19A6@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=f.ebner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal