[RFC] add max-depth to the datastore snapshots API endpoint

[RFC] add max-depth to the datastore snapshots API endpoint

[thomas moore]

[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]

Hi all

This is a proposal for discussion, not a patch. The
developer documentation asks that plans be discussed before
starting work, so I'd like to check whether this would be
acceptable before writing it.

GET /api2/json/admin/datastore/{store}/snapshots returns only
the snapshots in the requested namespace, so a client wanting
a datastore-wide view has to call
/admin/datastore/{store}/namespace first and then issue one
snapshots request per namespace. I ran into this writing a
third-party monitoring client.

Tested against PBS 4.2:

/admin/datastore/pbs/snapshots           -> 485 snapshots (root
only) 

/admin/datastore/pbs/snapshots?ns=test   -> 1 snapshot 

/admin/datastore/pbs/snapshots?max-depth=7 -> 400
parameter verification failed - 'max-depth': schema does not
allow additional properties

Proposal: accept an optional max-depth using the
existing NS_MAX_DEPTH_SCHEMA, defaulting to 0 so behaviour is
unchanged for current callers.

This looks anticipated in the code. As of
5d95fc20e, src/api2/admin/datastore.rs lines 571, 575 and 583
each carry a "// FIXME: Recursion" on the group
construction in list_snapshots_blocking, with a further one at
565 about filtering by owner before collecting.
ListAccessibleBackupGroups already takes max_depth and applies
the per-namespace privilege and owner checks during iteration,
and get_snapshots_count uses it in roughly the shape list_snapshots
would need.

I'd send it as two patches: the refactor
onto ListAccessibleBackupGroups with no behavioural change, then
max-depth on top. I can test on a 4.2 instance including with a
token holding DatastoreAudit on the root namespace only, to
confirm namespaces the token cannot see stay absent from a
recursive listing.





Questions for the list

1. Is adding max-depth to the snapshots endpoint acceptable in
principle?

2. Is ListAccessibleBackupGroups the right vehicle here, or is
there a reason list_snapshots 	does not already use it?

3. Snapshot entries carry no namespace field, so a recursive
listing is ambiguous unless the 	client tracks it per request. Adding
one would mean a new field on SnapshotListItem, which 	is also
decoded from remote instances in server/sync.rs and server/push.rs

If you'd rather this were solved another way or not at all I'm
happy to hear it before writing anything.

Regards Thomas Moore

[-- Attachment #2: Type: text/html, Size: 4201 bytes --]

Re: [RFC] add max-depth to the datastore snapshots API endpoint

[Christian Ebner]

Hi!

On 8/22/26 2:30 AM, thomas moore wrote:
> Hi all
> 
> This is a proposal for discussion, not a patch. The
> developer documentation asks that plans be discussed before
> starting work, so I'd like to check whether this would be
> acceptable before writing it.
> 
> GET /api2/json/admin/datastore/{store}/snapshots returns only
> the snapshots in the requested namespace, so a client wanting
> a datastore-wide view has to call
> /admin/datastore/{store}/namespace first and then issue one
> snapshots request per namespace. I ran into this writing a
> third-party monitoring client.
> 
> Tested against PBS 4.2:
> 
> /admin/datastore/pbs/snapshots           -> 485 snapshots (root
> only)
> 
> /admin/datastore/pbs/snapshots?ns=test   -> 1 snapshot
> 
> /admin/datastore/pbs/snapshots?max-depth=7 -> 400
> parameter verification failed - 'max-depth': schema does not
> allow additional properties
> 
> Proposal: accept an optional max-depth using the
> existing NS_MAX_DEPTH_SCHEMA, defaulting to 0 so behaviour is
> unchanged for current callers.
> 
> This looks anticipated in the code. As of
> 5d95fc20e, src/api2/admin/datastore.rs lines 571, 575 and 583
> each carry a "// FIXME: Recursion" on the group
> construction in list_snapshots_blocking, with a further one at
> 565 about filtering by owner before collecting.
> ListAccessibleBackupGroups already takes max_depth and applies
> the per-namespace privilege and owner checks during iteration,
> and get_snapshots_count uses it in roughly the shape list_snapshots
> would need.
> 
> I'd send it as two patches: the refactor
> onto ListAccessibleBackupGroups with no behavioural change, then
> max-depth on top. I can test on a 4.2 instance including with a
> token holding DatastoreAudit on the root namespace only, to
> confirm namespaces the token cannot see stay absent from a
> recursive listing.
> 
> 
> 
> 
> 
> Questions for the list
> 
> 1. Is adding max-depth to the snapshots endpoint acceptable in
> principle?

Yes, although there has already been some work on this [0], implementing 
a streaming API which should already cover your case as well. This 
however is not applied yet, but might be picked up again.

Nevertheless, may I ask what your goal is and what limitation which is 
not already covered by the current API you are running into? Does 
performing the additional requests cause to much latency for your usecase?

> 2. Is ListAccessibleBackupGroups the right vehicle here, or is
> there a reason list_snapshots 	does not already use it?

list_snapshots_blocking() is rather heavy weight and performance 
critical, especially for datastores with lots of groups/snapshots on 
slow (spin rust) storage. So performance and memory analysis is a must 
here. Further, current behavior must be retained also with respect to 
group listing by type and backup id only, respectively. But again, I 
think patch [0] already would cover what you are asking for.

> 3. Snapshot entries carry no namespace field, so a recursive
> listing is ambiguous unless the 	client tracks it per request. Adding
> one would mean a new field on SnapshotListItem, which 	is also
> decoded from remote instances in server/sync.rs and server/push.rs

While not that critical, adding the additional field does have 
implications for the response payload size, especially for deeply nested 
datastores with lots of snapshots.

> If you'd rather this were solved another way or not at all I'm
> happy to hear it before writing anything.
> 
> Regards Thomas Moore

[0] 
https://lore.proxmox.com/pbs-devel/20251008134344.3512958-8-d.csapak@proxmox.com/

Best regards,
Chris

Re: [RFC] add max-depth to the datastore snapshots API endpoint

[thomas moore]

[-- Attachment #1: Type: text/plain, Size: 5223 bytes --]

> Yes, although there has already been some work on this [0],

> implementing a streaming API which should already cover your case as

> well.



Thanks, that patch covers both halves of what I asked for, and better

than what I proposed. I had not found it.



> Nevertheless, may I ask what your goal is and what limitation which is

> not already covered by the current API you are running into? Does

> performing the additional requests cause to much latency for your

> usecase?



It is not latency. I have three namespaces, so the extra requests are

not a problem at that scale. At larger namespace counts it clearly.

would be, which is what the streaming call addresses. The problem was

that a client calling the documented snapshots endpoint gets root-only

results with nothing indicating other namespaces exist. I have since

implemented enumerate then fetch per namespace, so this is about

discoverability for third-party clients rather than something I am

blocked on.



Happy to drop my proposal and wait for the streaming content API to

land.



Regards,

Thomas Moore





From: Christian Ebner <c.ebner@proxmox.com>
To: <sales@mooresolutions.co.nz>, "pbs-devel"<pbs-devel@lists.proxmox.com>
Date: Mon, 24 Aug 2026 21:39:23 +1200
Subject: Re: [RFC] add max-depth to the datastore snapshots API endpoint



Hi! 
 
On 8/22/26 2:30 AM, thomas moore wrote: 
> Hi all 
> 
> This is a proposal for discussion, not a patch. The 
> developer documentation asks that plans be discussed before 
> starting work, so I'd like to check whether this would be 
> acceptable before writing it. 
> 
> GET /api2/json/admin/datastore/{store}/snapshots returns only 
> the snapshots in the requested namespace, so a client wanting 
> a datastore-wide view has to call 
> /admin/datastore/{store}/namespace first and then issue one 
> snapshots request per namespace. I ran into this writing a 
> third-party monitoring client. 
> 
> Tested against PBS 4.2: 
> 
> /admin/datastore/pbs/snapshots           -> 485 snapshots (root 
> only) 
> 
> /admin/datastore/pbs/snapshots?ns=test   -> 1 snapshot 
> 
> /admin/datastore/pbs/snapshots?max-depth=7 -> 400 
> parameter verification failed - 'max-depth': schema does not 
> allow additional properties 
> 
> Proposal: accept an optional max-depth using the 
> existing NS_MAX_DEPTH_SCHEMA, defaulting to 0 so behaviour is 
> unchanged for current callers. 
> 
> This looks anticipated in the code. As of 
> 5d95fc20e, src/api2/admin/datastore.rs lines 571, 575 and 583 
> each carry a "// FIXME: Recursion" on the group 
> construction in list_snapshots_blocking, with a further one at 
> 565 about filtering by owner before collecting. 
> ListAccessibleBackupGroups already takes max_depth and applies 
> the per-namespace privilege and owner checks during iteration, 
> and get_snapshots_count uses it in roughly the shape list_snapshots 
> would need. 
> 
> I'd send it as two patches: the refactor 
> onto ListAccessibleBackupGroups with no behavioural change, then 
> max-depth on top. I can test on a 4.2 instance including with a 
> token holding DatastoreAudit on the root namespace only, to 
> confirm namespaces the token cannot see stay absent from a 
> recursive listing. 
> 
> 
> 
> 
> 
> Questions for the list 
> 
> 1. Is adding max-depth to the snapshots endpoint acceptable in 
> principle? 
 
Yes, although there has already been some work on this [0], implementing 
a streaming API which should already cover your case as well. This 
however is not applied yet, but might be picked up again. 
 
Nevertheless, may I ask what your goal is and what limitation which is 
not already covered by the current API you are running into? Does 
performing the additional requests cause to much latency for your usecase? 
 
> 2. Is ListAccessibleBackupGroups the right vehicle here, or is 
> there a reason list_snapshots     does not already use it? 
 
list_snapshots_blocking() is rather heavy weight and performance 
critical, especially for datastores with lots of groups/snapshots on 
slow (spin rust) storage. So performance and memory analysis is a must 
here. Further, current behavior must be retained also with respect to 
group listing by type and backup id only, respectively. But again, I 
think patch [0] already would cover what you are asking for. 
 
> 3. Snapshot entries carry no namespace field, so a recursive 
> listing is ambiguous unless the     client tracks it per request. Adding 
> one would mean a new field on SnapshotListItem, which     is also 
> decoded from remote instances in server/sync.rs and server/push.rs 
 
While not that critical, adding the additional field does have 
implications for the response payload size, especially for deeply nested 
datastores with lots of snapshots. 
 
> If you'd rather this were solved another way or not at all I'm 
> happy to hear it before writing anything. 
> 
> Regards Thomas Moore 
 
[0] 
https://lore.proxmox.com/pbs-devel/20251008134344.3512958-8-d.csapak@proxmox.com/  
 
Best regards, 
Chris

[-- Attachment #2: Type: text/html, Size: 6836 bytes --]