Re: [PATCH pve-storage 7/7] api: add /nodes/<node>/storage/<storage>/identity route

["Lukas Wagner" <l.wagner@proxmox.com>]

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
>> 
>> 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: "<plugin>", id: "<string>" }` 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/<node>/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. 

One last point, but maybe I'm overthinking this:

To me, the { type: "<plugin>", id: "<string" } approach kind of implies that
this ID would uniquely identify the PBS instance *as well as* the
datastore/namespace, which it does not at the moment.

Would you think that it would make sense for PBS storage plugin to
maybe combine the datastore, namespace and the
pbs-instance-id into a single identifier here?

Maybe something like, in pseudo-regex

<datastore>(/<namespace>)?@<pbs-instance-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.