* [pbs-devel] [PATCH proxmox v2 1/1] pbs-api-types: add api types for streaming content api call
2025-10-08 13:43 [pbs-devel] [PATCH proxmox{, -backup} v2 0/7] introduce streaming content api call Dominik Csapak
@ 2025-10-08 13:43 ` Dominik Csapak
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 1/6] backup: hierarchy: add new can_access_any_namespace_below helper Dominik Csapak
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Dominik Csapak @ 2025-10-08 13:43 UTC (permalink / raw)
To: pbs-devel
Main point is the new struct
* DatastoreContent, which contains either
- a namespace
- a snapshot (including namespace)
- a backup group (including namespace)
Since the enums variants are newtypes, we need to annotate the 'serde
tag' and given them an 'id-property' and 'id-schema' even though we
don't need it, since it's only relevant for saving these on disk with
section config.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* no contenttype anymore (not necessary currently)
* make properties public (else we can't use them in the ui)
* rename the '*List' types to something better
pbs-api-types/Cargo.toml | 1 +
pbs-api-types/src/datastore.rs | 89 ++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+)
diff --git a/pbs-api-types/Cargo.toml b/pbs-api-types/Cargo.toml
index 358536d9..b3dee41d 100644
--- a/pbs-api-types/Cargo.toml
+++ b/pbs-api-types/Cargo.toml
@@ -23,5 +23,6 @@ proxmox-lang.workspace=true
proxmox-s3-client = { workspace = true, features = [ "api-types" ] }
proxmox-schema = { workspace = true, features = [ "api-macro" ] }
proxmox-serde.workspace = true
+proxmox-section-config.workspace = true
proxmox-time.workspace = true
proxmox-uuid = { workspace = true, features = [ "serde" ] }
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index fe73cbc4..9b4b9cfa 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -1479,6 +1479,86 @@ pub struct NamespaceListItem {
pub comment: Option<String>,
}
+#[api(
+ properties: {
+ ns: {
+ type: BackupNamespace,
+ },
+ snapshot: {
+ type: SnapshotListItem,
+ flatten: true,
+ },
+ },
+)]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Represents a snapshot in a datastore with namespace information
+pub struct SnapshotWithNamespace {
+ /// The namespace of the snapshot
+ pub ns: BackupNamespace,
+ #[serde(flatten)]
+ /// The snapshot information
+ pub snapshot: SnapshotListItem,
+}
+
+#[api(
+ properties: {
+ ns: {
+ type: BackupNamespace,
+ },
+ group: {
+ type: GroupListItem,
+ flatten: true,
+ },
+ },
+)]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Represents a snapshot in a datastore with namespace information
+pub struct BackupGroupWithNamespace {
+ /// The namespace of the snapshot
+ pub ns: BackupNamespace,
+ #[serde(flatten)]
+ /// The backup group information
+ pub group: GroupListItem,
+}
+
+#[api(
+ "id-property": "id",
+ "id-schema": {
+ type: String,
+ description: "ID",
+ },
+)]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+#[serde(tag = "type")]
+pub enum DatastoreContent {
+ NameSpace(NamespaceListItem),
+ Group(BackupGroupWithNamespace),
+ Snapshot(SnapshotWithNamespace),
+}
+
+impl From<NamespaceListItem> for DatastoreContent {
+ fn from(value: NamespaceListItem) -> Self {
+ DatastoreContent::NameSpace(value)
+ }
+}
+
+impl From<(BackupNamespace, GroupListItem)> for DatastoreContent {
+ fn from(value: (BackupNamespace, GroupListItem)) -> Self {
+ let (ns, group) = value;
+ DatastoreContent::Group(BackupGroupWithNamespace { ns, group })
+ }
+}
+
+impl From<(BackupNamespace, SnapshotListItem)> for DatastoreContent {
+ fn from(value: (BackupNamespace, SnapshotListItem)) -> Self {
+ let (ns, snapshot) = value;
+ DatastoreContent::Snapshot(SnapshotWithNamespace { ns, snapshot })
+ }
+}
+
#[api(
properties: {
"backup": { type: BackupDir },
@@ -1759,6 +1839,15 @@ pub const ADMIN_DATASTORE_LIST_NAMESPACE_RETURN_TYPE: ReturnType = ReturnType {
.schema(),
};
+pub const ADMIN_DATASTORE_LIST_CONTENT_RETURN_TYPE: ReturnType = ReturnType {
+ optional: false,
+ schema: &ArraySchema::new(
+ "Returns the list of namespaces, backup groups and snapshots of a datastore.",
+ &DatastoreContent::API_SCHEMA,
+ )
+ .schema(),
+};
+
pub const ADMIN_DATASTORE_PRUNE_RETURN_TYPE: ReturnType = ReturnType {
optional: false,
schema: &ArraySchema::new(
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 1/6] backup: hierarchy: add new can_access_any_namespace_below helper
2025-10-08 13:43 [pbs-devel] [PATCH proxmox{, -backup} v2 0/7] introduce streaming content api call Dominik Csapak
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox v2 1/1] pbs-api-types: add api types for " Dominik Csapak
@ 2025-10-08 13:43 ` Dominik Csapak
2025-10-08 20:57 ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 2/6] backup: hierarchy: reuse 'NS_PRIVS_OK' for namespace helper Dominik Csapak
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dominik Csapak @ 2025-10-08 13:43 UTC (permalink / raw)
To: pbs-devel
sometimes we need to check the permissions in a range from a starting
namespace with a certain depth.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* rename helper from 'can_access_any_namespace_in_range'
src/backup/hierarchy.rs | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/src/backup/hierarchy.rs b/src/backup/hierarchy.rs
index 8dd71fcf7..febcb9a83 100644
--- a/src/backup/hierarchy.rs
+++ b/src/backup/hierarchy.rs
@@ -68,19 +68,21 @@ pub fn check_ns_privs_full(
);
}
-pub fn can_access_any_namespace(
+/// Checks if the given user has read/access rights on any namespace on the given datastore,
+/// beginning with `start_ns` up to `max_depth` below.
+pub fn can_access_any_namespace_below(
store: Arc<DataStore>,
auth_id: &Authid,
user_info: &CachedUserInfo,
+ parent_ns: Option<BackupNamespace>,
+ max_depth: Option<usize>,
) -> bool {
+ let ns = parent_ns.unwrap_or_default();
// NOTE: traversing the datastore could be avoided if we had an "ACL tree: is there any priv
// below /datastore/{store}" helper
- let mut iter =
- if let Ok(iter) = store.recursive_iter_backup_ns_ok(BackupNamespace::root(), None) {
- iter
- } else {
- return false;
- };
+ let Ok(mut iter) = store.recursive_iter_backup_ns_ok(ns, max_depth) else {
+ return false;
+ };
let wanted =
PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_MODIFY | PRIV_DATASTORE_READ | PRIV_DATASTORE_BACKUP;
let name = store.name();
@@ -90,6 +92,15 @@ pub fn can_access_any_namespace(
})
}
+/// Checks if the given user has read/access rights on any namespace on given datastore
+pub fn can_access_any_namespace(
+ store: Arc<DataStore>,
+ auth_id: &Authid,
+ user_info: &CachedUserInfo,
+) -> bool {
+ can_access_any_namespace_below(store, auth_id, user_info, None, None)
+}
+
/// A privilege aware iterator for all backup groups in all Namespaces below an anchor namespace,
/// most often that will be the `BackupNamespace::root()` one.
///
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 2/6] backup: hierarchy: reuse 'NS_PRIVS_OK' for namespace helper
2025-10-08 13:43 [pbs-devel] [PATCH proxmox{, -backup} v2 0/7] introduce streaming content api call Dominik Csapak
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox v2 1/1] pbs-api-types: add api types for " Dominik Csapak
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 1/6] backup: hierarchy: add new can_access_any_namespace_below helper Dominik Csapak
@ 2025-10-08 13:43 ` Dominik Csapak
2025-10-08 20:57 ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 3/6] api: admin: datastore: refactor BackupGroup to GroupListItem conversion Dominik Csapak
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dominik Csapak @ 2025-10-08 13:43 UTC (permalink / raw)
To: pbs-devel
Aside from the order of the privileges, it's identical to what we
already used here, so simply reuse it.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes in v2
src/backup/hierarchy.rs | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/backup/hierarchy.rs b/src/backup/hierarchy.rs
index febcb9a83..be11ece05 100644
--- a/src/backup/hierarchy.rs
+++ b/src/backup/hierarchy.rs
@@ -83,12 +83,10 @@ pub fn can_access_any_namespace_below(
let Ok(mut iter) = store.recursive_iter_backup_ns_ok(ns, max_depth) else {
return false;
};
- let wanted =
- PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_MODIFY | PRIV_DATASTORE_READ | PRIV_DATASTORE_BACKUP;
let name = store.name();
iter.any(|ns| -> bool {
let user_privs = user_info.lookup_privs(auth_id, &["datastore", name, &ns.to_string()]);
- user_privs & wanted != 0
+ user_privs & NS_PRIVS_OK != 0
})
}
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 3/6] api: admin: datastore: refactor BackupGroup to GroupListItem conversion
2025-10-08 13:43 [pbs-devel] [PATCH proxmox{, -backup} v2 0/7] introduce streaming content api call Dominik Csapak
` (2 preceding siblings ...)
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 2/6] backup: hierarchy: reuse 'NS_PRIVS_OK' for namespace helper Dominik Csapak
@ 2025-10-08 13:43 ` Dominik Csapak
2025-10-08 20:57 ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 4/6] api: admin: datastore: factor out 'get_group_owner' Dominik Csapak
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dominik Csapak @ 2025-10-08 13:43 UTC (permalink / raw)
To: pbs-devel
We will reuse this later.
No functionial change intended.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes in v2
src/api2/admin/datastore.rs | 107 +++++++++++++++++++++---------------
1 file changed, 62 insertions(+), 45 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index a098aca8e..26799de6e 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -142,57 +142,74 @@ pub fn list_groups(
.try_fold(Vec::new(), |mut group_info, group| {
let group = group?;
- let owner = match datastore.get_owner(&ns, group.as_ref()) {
- Ok(auth_id) => auth_id,
- Err(err) => {
- eprintln!(
- "Failed to get owner of group '{}' in {} - {}",
- group.group(),
- print_store_and_ns(&store, &ns),
- err
- );
- return Ok(group_info);
- }
- };
- if !list_all && check_backup_owner(&owner, &auth_id).is_err() {
- return Ok(group_info);
+ let item =
+ backup_group_to_group_list_item(datastore.clone(), group, &ns, &auth_id, list_all);
+
+ if let Some(item) = item {
+ group_info.push(item);
}
- let snapshots = match group.list_backups() {
- Ok(snapshots) => snapshots,
- Err(_) => return Ok(group_info),
- };
+ Ok(group_info)
+ })
+}
- let backup_count: u64 = snapshots.len() as u64;
- if backup_count == 0 {
- return Ok(group_info);
- }
+fn backup_group_to_group_list_item(
+ datastore: Arc<DataStore>,
+ group: pbs_datastore::BackupGroup,
+ ns: &BackupNamespace,
+ auth_id: &Authid,
+ list_all: bool,
+) -> Option<GroupListItem> {
+ let owner = match datastore.get_owner(ns, group.as_ref()) {
+ Ok(auth_id) => auth_id,
+ Err(err) => {
+ eprintln!(
+ "Failed to get owner of group '{}' in {} - {}",
+ group.group(),
+ print_store_and_ns(datastore.name(), ns),
+ err
+ );
+ return None;
+ }
+ };
+ if !list_all && check_backup_owner(&owner, auth_id).is_err() {
+ return None;
+ }
- let last_backup = snapshots
- .iter()
- .fold(&snapshots[0], |a, b| {
- if a.is_finished() && a.backup_dir.backup_time() > b.backup_dir.backup_time() {
- a
- } else {
- b
- }
- })
- .to_owned();
-
- let notes_path = datastore.group_notes_path(&ns, group.as_ref());
- let comment = file_read_firstline(notes_path).ok();
-
- group_info.push(GroupListItem {
- backup: group.into(),
- last_backup: last_backup.backup_dir.backup_time(),
- owner: Some(owner),
- backup_count,
- files: last_backup.files,
- comment,
- });
+ let snapshots = match group.list_backups() {
+ Ok(snapshots) => snapshots,
+ Err(_) => return None,
+ };
- Ok(group_info)
+ let backup_count: u64 = snapshots.len() as u64;
+ if backup_count == 0 {
+ return None;
+ }
+
+ let last_backup = snapshots
+ .iter()
+ .fold(&snapshots[0], |a, b| {
+ if a.is_finished() && a.backup_dir.backup_time() > b.backup_dir.backup_time() {
+ a
+ } else {
+ b
+ }
})
+ .to_owned();
+
+ let notes_path = datastore.group_notes_path(ns, group.as_ref());
+ let comment = file_read_firstline(notes_path).ok();
+
+ let item = GroupListItem {
+ backup: group.into(),
+ last_backup: last_backup.backup_dir.backup_time(),
+ owner: Some(owner),
+ backup_count,
+ files: last_backup.files,
+ comment,
+ };
+
+ Some(item)
}
#[api(
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 4/6] api: admin: datastore: factor out 'get_group_owner'
2025-10-08 13:43 [pbs-devel] [PATCH proxmox{, -backup} v2 0/7] introduce streaming content api call Dominik Csapak
` (3 preceding siblings ...)
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 3/6] api: admin: datastore: refactor BackupGroup to GroupListItem conversion Dominik Csapak
@ 2025-10-08 13:43 ` Dominik Csapak
2025-10-08 20:57 ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 5/6] api: admin: datastore: optimize `groups` api call Dominik Csapak
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 6/6] api: admin: datastore: implement streaming content " Dominik Csapak
6 siblings, 1 reply; 14+ messages in thread
From: Dominik Csapak @ 2025-10-08 13:43 UTC (permalink / raw)
To: pbs-devel
and change the `eprintln` to a `log::warn`
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes in v2
src/api2/admin/datastore.rs | 47 +++++++++++++++++++------------------
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 26799de6e..0b133d166 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -160,18 +160,8 @@ fn backup_group_to_group_list_item(
auth_id: &Authid,
list_all: bool,
) -> Option<GroupListItem> {
- let owner = match datastore.get_owner(ns, group.as_ref()) {
- Ok(auth_id) => auth_id,
- Err(err) => {
- eprintln!(
- "Failed to get owner of group '{}' in {} - {}",
- group.group(),
- print_store_and_ns(datastore.name(), ns),
- err
- );
- return None;
- }
- };
+ let owner = get_group_owner(datastore.name(), ns, &group)?;
+
if !list_all && check_backup_owner(&owner, auth_id).is_err() {
return None;
}
@@ -439,6 +429,25 @@ pub async fn list_snapshots(
.map_err(|err| format_err!("failed to await blocking task: {err}"))?
}
+fn get_group_owner(
+ store: &str,
+ ns: &BackupNamespace,
+ group: &pbs_datastore::BackupGroup,
+) -> Option<Authid> {
+ match group.get_owner() {
+ Ok(auth_id) => Some(auth_id),
+ Err(err) => {
+ log::warn!(
+ "Failed to get owner of group '{}' in {} - {}",
+ group.group(),
+ print_store_and_ns(store, ns),
+ err
+ );
+ None
+ }
+ }
+}
+
/// This must not run in a main worker thread as it potentially does tons of I/O.
unsafe fn list_snapshots_blocking(
store: String,
@@ -482,17 +491,9 @@ unsafe fn list_snapshots_blocking(
};
groups.iter().try_fold(Vec::new(), |mut snapshots, group| {
- let owner = match group.get_owner() {
- Ok(auth_id) => auth_id,
- Err(err) => {
- eprintln!(
- "Failed to get owner of group '{}' in {} - {}",
- group.group(),
- print_store_and_ns(&store, &ns),
- err
- );
- return Ok(snapshots);
- }
+ let owner = match get_group_owner(&store, &ns, group) {
+ Some(auth_id) => auth_id,
+ None => return Ok(snapshots),
};
if !list_all && check_backup_owner(&owner, &auth_id).is_err() {
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 5/6] api: admin: datastore: optimize `groups` api call
2025-10-08 13:43 [pbs-devel] [PATCH proxmox{, -backup} v2 0/7] introduce streaming content api call Dominik Csapak
` (4 preceding siblings ...)
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 4/6] api: admin: datastore: factor out 'get_group_owner' Dominik Csapak
@ 2025-10-08 13:43 ` Dominik Csapak
2025-10-08 20:57 ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 6/6] api: admin: datastore: implement streaming content " Dominik Csapak
6 siblings, 1 reply; 14+ messages in thread
From: Dominik Csapak @ 2025-10-08 13:43 UTC (permalink / raw)
To: pbs-devel
Currently we always touch all files for each snapshot in a group when
listing them, even though we don't need all that info.
We're only interested in getting either the last finished snapshot
information, or the last unfinished one (which must the only one in
normal use, we can't have multiple unfinished snapshots usually)
Instead of getting all the information upfront, use the snapshot
iterator of the group to get only the id, sort them by time, and
use the first we're interested in, getting the snapshot specific info
only for those we want to check.
In my (admittedly extreme) setup with ~600 groups with ~1000 snapshots
each, this changes the time this api call needs from ~40s to <1s.
(on a relatively fast disk).
While at it, lift the restriction of only returning groups with
snapshots in them, now returning also empty ones.
To keep api compatibility, use a timestamp of 0 for those.
(no valid backup could have been made at that time anyway)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* split out the BackupGroup -> BackupInfo conversion into own
'filer_map' call, and use 'find' to find the first finished one
src/api2/admin/datastore.rs | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 0b133d166..643d1694b 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -166,36 +166,38 @@ fn backup_group_to_group_list_item(
return None;
}
- let snapshots = match group.list_backups() {
- Ok(snapshots) => snapshots,
+ let mut snapshots: Vec<_> = match group.iter_snapshots() {
+ Ok(snapshots) => snapshots.collect::<Result<Vec<_>, Error>>().ok()?,
Err(_) => return None,
};
let backup_count: u64 = snapshots.len() as u64;
- if backup_count == 0 {
- return None;
- }
+ let last = if backup_count == 1 {
+ // we may have only one unfinished snapshot
+ snapshots.pop().and_then(|dir| BackupInfo::new(dir).ok())
+ } else {
+ // we either have no snapshots, or at least one finished one, since we cannot have
+ // multiple unfinished ones
+ snapshots.sort_by_key(|b| std::cmp::Reverse(b.backup_time()));
+ snapshots
+ .iter()
+ .filter_map(|backup| BackupInfo::new(backup.clone()).ok())
+ .find(|info| info.is_finished())
+ };
- let last_backup = snapshots
- .iter()
- .fold(&snapshots[0], |a, b| {
- if a.is_finished() && a.backup_dir.backup_time() > b.backup_dir.backup_time() {
- a
- } else {
- b
- }
- })
- .to_owned();
+ let (last_backup, files) = last
+ .map(|info| (info.backup_dir.backup_time(), info.files))
+ .unwrap_or((0, Vec::new()));
let notes_path = datastore.group_notes_path(ns, group.as_ref());
let comment = file_read_firstline(notes_path).ok();
let item = GroupListItem {
backup: group.into(),
- last_backup: last_backup.backup_dir.backup_time(),
+ last_backup,
owner: Some(owner),
backup_count,
- files: last_backup.files,
+ files,
comment,
};
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 6/6] api: admin: datastore: implement streaming content api call
2025-10-08 13:43 [pbs-devel] [PATCH proxmox{, -backup} v2 0/7] introduce streaming content api call Dominik Csapak
` (5 preceding siblings ...)
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 5/6] api: admin: datastore: optimize `groups` api call Dominik Csapak
@ 2025-10-08 13:43 ` Dominik Csapak
2025-10-08 19:49 ` Thomas Lamprecht
6 siblings, 1 reply; 14+ messages in thread
From: Dominik Csapak @ 2025-10-08 13:43 UTC (permalink / raw)
To: pbs-devel
this is a new api call that utilizes `proxmox_router::Stream` to provide
a streaming interface to querying the datastore content.
This can be done when a client requests this api call with the
`application/json-seq` Accept header.
In contrast to the existing api calls, this one
* returns all types of content items (namespaces, groups, snapshots; can
be filtered with a parameter)
* iterates over them recursively (with the range that is given with the
parameter)
The api call returns the data in the following order:
* first all visible namespaces
* then for each ns in order
* each group
* each snapshot
This is done so that we can have a good way of building a tree view in
the ui.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* don't use a tokio::spawn around a 'spawn_blocking'
* don't use async-stream, but proxmox-router::Stream
* add new use statements from pbs_api_types in a new line
(makes the diff more readable..)
* remove content type filter from api call (can be done later still)
* return one level more for namespaces than given, otherwise
we'll list groups + snapshots for the levels, but not the existing
namespaces there
src/api2/admin/datastore.rs | 175 +++++++++++++++++++++++++++++++-----
1 file changed, 155 insertions(+), 20 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 643d1694b..811ee8713 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -23,7 +23,7 @@ use proxmox_compression::zstd::ZstdEncoder;
use proxmox_log::LogContext;
use proxmox_router::{
http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission,
- Router, RpcEnvironment, RpcEnvironmentType, SubdirMap,
+ Record, Router, RpcEnvironment, RpcEnvironmentType, SubdirMap,
};
use proxmox_rrd_api_types::{RrdMode, RrdTimeframe};
use proxmox_schema::*;
@@ -49,6 +49,7 @@ use pbs_api_types::{
PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ,
PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
};
+use pbs_api_types::{DatastoreContent, NamespaceListItem};
use pbs_client::pxar::{create_tar, create_zip};
use pbs_config::CachedUserInfo;
use pbs_datastore::backup_info::BackupInfo;
@@ -70,7 +71,10 @@ use proxmox_rest_server::{formatter, worker_is_active, WorkerTask};
use crate::api2::backup::optional_ns_param;
use crate::api2::node::rrd::create_value_from_rrd;
-use crate::backup::{check_ns_privs_full, ListAccessibleBackupGroups, VerifyWorker, NS_PRIVS_OK};
+use crate::backup::{
+ can_access_any_namespace_below, check_ns_privs, check_ns_privs_full,
+ ListAccessibleBackupGroups, VerifyWorker, NS_PRIVS_OK,
+};
use crate::server::jobstate::{compute_schedule_status, Job, JobState};
use crate::tools::{backup_info_to_snapshot_list_item, get_all_snapshot_files, read_backup_index};
@@ -387,7 +391,7 @@ pub async fn delete_snapshot(
}
#[api(
- serializing: true,
+ stream: true,
input: {
properties: {
store: { schema: DATASTORE_SCHEMA },
@@ -395,40 +399,125 @@ pub async fn delete_snapshot(
type: BackupNamespace,
optional: true,
},
- "backup-type": {
- optional: true,
- type: BackupType,
- },
- "backup-id": {
+ "max-depth": {
+ schema: NS_MAX_DEPTH_SCHEMA,
optional: true,
- schema: BACKUP_ID_SCHEMA,
},
},
},
- returns: pbs_api_types::ADMIN_DATASTORE_LIST_SNAPSHOTS_RETURN_TYPE,
access: {
permission: &Permission::Anybody,
description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_AUDIT for any \
or DATASTORE_BACKUP and being the owner of the group",
},
)]
-/// List backup snapshots.
-pub async fn list_snapshots(
+/// List datastore content, recursively through all namespaces.
+pub async fn list_content(
store: String,
ns: Option<BackupNamespace>,
- backup_type: Option<BackupType>,
- backup_id: Option<String>,
+ max_depth: Option<usize>,
_param: Value,
_info: &ApiMethod,
rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<SnapshotListItem>, Error> {
+) -> Result<proxmox_router::Stream, Error> {
+ let (sender, receiver) = tokio::sync::mpsc::channel(128);
+
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+ let user_info = CachedUserInfo::new()?;
- tokio::task::spawn_blocking(move || unsafe {
- list_snapshots_blocking(store, ns, backup_type, backup_id, auth_id)
- })
- .await
- .map_err(|err| format_err!("failed to await blocking task: {err}"))?
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
+
+ // show one level of namespaces more than we use for group/snapshot list
+ let list_max_depth = max_depth.map(|depth| depth + 1);
+ if !can_access_any_namespace_below(
+ datastore.clone(),
+ &auth_id,
+ &user_info,
+ ns.clone(),
+ list_max_depth,
+ ) {
+ proxmox_router::http_bail!(FORBIDDEN, "permission check failed");
+ }
+
+ let ns = ns.unwrap_or_default();
+
+ tokio::task::spawn_blocking(move || {
+ for ns in datastore.recursive_iter_backup_ns_ok(ns.clone(), list_max_depth)? {
+ match check_ns_privs(&store, &ns, &auth_id, NS_PRIVS_OK) {
+ Ok(_) => sender.blocking_send(Record::new(DatastoreContent::from(
+ NamespaceListItem { ns, comment: None },
+ )))?,
+ // don't disclose errors about ns privileges, since that can be an info leak
+ Err(_) => continue,
+ }
+ }
+
+ for ns in datastore.recursive_iter_backup_ns_ok(ns, max_depth)? {
+ let list_all = match check_ns_privs_full(
+ &store,
+ &ns,
+ &auth_id,
+ PRIV_DATASTORE_AUDIT,
+ PRIV_DATASTORE_BACKUP,
+ ) {
+ Ok(requires_owner) => !requires_owner,
+ // don't disclose errors about ns privileges, since that can be an info leak
+ Err(_) => continue,
+ };
+ for group in datastore.iter_backup_groups(ns.clone())? {
+ let group = match group {
+ Ok(group) => group,
+ Err(err) => {
+ sender.blocking_send(Record::error(err))?;
+ continue;
+ }
+ };
+ let group = backup_group_to_group_list_item(
+ datastore.clone(),
+ group,
+ &ns,
+ &auth_id,
+ list_all,
+ );
+
+ if let Some(group) = group {
+ sender
+ .blocking_send(Record::new(DatastoreContent::from((ns.clone(), group))))?;
+ }
+ }
+
+ for group in datastore.iter_backup_groups(ns.clone())? {
+ let group = match group {
+ Ok(group) => group,
+ Err(err) => {
+ sender.blocking_send(Record::error(err))?;
+ continue;
+ }
+ };
+ let owner = match get_group_owner(&store, &ns, &group) {
+ Some(auth_id) => auth_id,
+ None => continue,
+ };
+ for backup_dir in group.iter_snapshots()? {
+ let backup_info = match backup_dir {
+ Ok(snap) => BackupInfo::new(snap)?,
+ Err(err) => {
+ sender.blocking_send(Record::error(err))?;
+ continue;
+ }
+ };
+ let snapshot = backup_info_to_snapshot_list_item(&backup_info, &owner);
+ sender.blocking_send(Record::new(DatastoreContent::from((
+ ns.clone(),
+ snapshot,
+ ))))?;
+ }
+ }
+ }
+ Ok::<_, Error>(())
+ });
+
+ Ok(ReceiverStream::new(receiver).into())
}
fn get_group_owner(
@@ -514,6 +603,51 @@ unsafe fn list_snapshots_blocking(
})
}
+#[api(
+ serializing: true,
+ input: {
+ properties: {
+ store: { schema: DATASTORE_SCHEMA },
+ ns: {
+ type: BackupNamespace,
+ optional: true,
+ },
+ "backup-type": {
+ optional: true,
+ type: BackupType,
+ },
+ "backup-id": {
+ optional: true,
+ schema: BACKUP_ID_SCHEMA,
+ },
+ },
+ },
+ returns: pbs_api_types::ADMIN_DATASTORE_LIST_SNAPSHOTS_RETURN_TYPE,
+ access: {
+ permission: &Permission::Anybody,
+ description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_AUDIT for any \
+ or DATASTORE_BACKUP and being the owner of the group",
+ },
+)]
+/// List backup snapshots.
+pub async fn list_snapshots(
+ store: String,
+ ns: Option<BackupNamespace>,
+ backup_type: Option<BackupType>,
+ backup_id: Option<String>,
+ _param: Value,
+ _info: &ApiMethod,
+ rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<SnapshotListItem>, Error> {
+ let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
+ tokio::task::spawn_blocking(move || unsafe {
+ list_snapshots_blocking(store, ns, backup_type, backup_id, auth_id)
+ })
+ .await
+ .map_err(|err| format_err!("failed to await blocking task: {err}"))?
+}
+
async fn get_snapshots_count(
store: &Arc<DataStore>,
owner: Option<&Authid>,
@@ -2764,6 +2898,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
"change-owner",
&Router::new().post(&API_METHOD_SET_BACKUP_OWNER),
),
+ ("content", &Router::new().get(&API_METHOD_LIST_CONTENT)),
(
"download",
&Router::new().download(&API_METHOD_DOWNLOAD_FILE),
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v2 6/6] api: admin: datastore: implement streaming content api call
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 6/6] api: admin: datastore: implement streaming content " Dominik Csapak
@ 2025-10-08 19:49 ` Thomas Lamprecht
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Lamprecht @ 2025-10-08 19:49 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
Am 08.10.25 um 15:43 schrieb Dominik Csapak:
> this is a new api call that utilizes `proxmox_router::Stream` to provide
> a streaming interface to querying the datastore content.
>
> This can be done when a client requests this api call with the
> `application/json-seq` Accept header.
>
> In contrast to the existing api calls, this one
> * returns all types of content items (namespaces, groups, snapshots; can
> be filtered with a parameter)
> * iterates over them recursively (with the range that is given with the
> parameter)
>
> The api call returns the data in the following order:
> * first all visible namespaces
> * then for each ns in order
> * each group
> * each snapshot
>
> This is done so that we can have a good way of building a tree view in
> the ui.
I guess you did not get around to test some more performance / memory
usage here? Might be nice to have whatever stats you did compare encoded
in the commit message here.
I.e. that part of you and my text from patch 6/6 from the v1:
Am 03.10.25 um 13:55 schrieb Thomas Lamprecht:
> Am 03.10.25 um 10:51 schrieb Dominik Csapak:
>> interesting side node, in my rather large setup with ~600 groups and ~1000
>> snapshosts per group, streaming this is faster than using the current
>> `snapshot` api (by a lot):
>> * `snapshot` api -> ~3 min
>> * `content` api with streaming -> ~2:11 min
>> * `content` api without streaming -> ~3 min
>>
>> It seems that either collecting such a 'large' api response (~200MiB)
>> is expensive. My guesses what happens here are either:
>> * frequent (re)allocation of the resulting vec
>> * or serde's serializing code
>
> You could compare peak (RSS) memory usage of the daemon as side-effect,
> and/or also use bpftrace to log bigger allocations. While I did use bpftrace
> lots of times, I did not try this specifically to rust, but I found a
> shorth'ish article that describes doing just that for rust, and looks like
> it would not be _that_ much work (and could be a nice tool to have in the
> belt in the future):
>
> https://readyset.io/blog/tracing-large-memory-allocations-in-rust-with-bpftrace
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 14+ messages in thread