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

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

On Fri Apr 24, 2026 at 1:05 AM CEST, Thomas Lamprecht wrote:
> Am 23.04.26 um 11:24 schrieb Lukas Wagner:
>> 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.
>
> Fair point but I'd keep `id` scoped to the server instance though,
> because:
>
> - The pve-storage entry already pins datastore + namespace via
>   storage.cfg, so a caller that knows which pve-storage it queried has
>   those two from local context. Callers doing content-level matching
>   anyway start from a specific (datastore, namespace) target, so once
>   the instance ids line up the rest just falls out of the storage (or
>   e.g. PBS remote) config. Baking datastore + namespace into `id` mostly
>   duplicates what the caller already has and I'm not really a of such
>   packed strings (reminds me of PBS' backup repo syntax I recently split
>   up for better UX, albeit here it at least wouldn't be handled by the
>   user directly)
> - Content-location identity as a general plugin contract is also
>   awkward: not every storage type decomposes into a "server + container
>   + sub-path" shape.
>
> So the contract should IMO rather stay "`id` identifies the server/service
> instance", which matches PDM's verification use. If auxiliary fields turn
> out to be useful later, `{ type, id, ... }` extends additively without
> breaking anyone, and I'd have nothing against adding datastore and
> namespace as extra (nested) fields there if you strongly prefer that and
> it really helps, but I would prefer to not "cram" it in the (instance) id.

Ack, then I'll just use the instance-id there. Thanks!

I'll post the new version ASAP :)