all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API
@ 2020-11-12 10:30 Fabian Grünbichler
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 1/5] improve group/snapshot listing Fabian Grünbichler
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-12 10:30 UTC (permalink / raw)
  To: pbs-devel

mainly relevant for non-Datastore.Audit users

- reduce number of file accesses when listing datastore contents
- make status call as issued by PVE less expensive
- filter snapshot counts for unpriv. users

Fabian Grünbichler (5):
  improve group/snapshot listing
  api: make expensive parts of datastore status opt-in
  api: filter snapshot counts
  drop now unused BackupInfo::list_backups
  api: include store in invalid owner errors

 src/api2/admin/datastore.rs | 305 +++++++++++++++++++++---------------
 src/api2/types/mod.rs       |  18 ++-
 src/backup/backup_info.rs   |  16 +-
 www/datastore/Summary.js    |   2 +-
 4 files changed, 195 insertions(+), 146 deletions(-)

-- 
2.20.1





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 1/5] improve group/snapshot listing
  2020-11-12 10:30 [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API Fabian Grünbichler
@ 2020-11-12 10:30 ` Fabian Grünbichler
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 2/5] api: make expensive parts of datastore status opt-in Fabian Grünbichler
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-12 10:30 UTC (permalink / raw)
  To: pbs-devel

by listing groups first, then filtering, then listing group snapshots.

this cuts down the number of openat/getdirents calls for users that just
have a partial view of the datastore.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    error behaviour should remain the same, unless I missed something..

 src/api2/admin/datastore.rs | 200 ++++++++++++++++++++----------------
 src/backup/backup_info.rs   |  28 +++--
 2 files changed, 131 insertions(+), 97 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index e76867c7..df666156 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1,4 +1,4 @@
-use std::collections::{HashSet, HashMap};
+use std::collections::HashSet;
 use std::ffi::OsStr;
 use std::os::unix::ffi::OsStrExt;
 use std::sync::{Arc, Mutex};
@@ -125,19 +125,6 @@ fn get_all_snapshot_files(
     Ok((manifest, files))
 }
 
-fn group_backups(backup_list: Vec<BackupInfo>) -> HashMap<String, Vec<BackupInfo>> {
-
-    let mut group_hash = HashMap::new();
-
-    for info in backup_list {
-        let group_id = info.backup_dir.group().group_path().to_str().unwrap().to_owned();
-        let time_list = group_hash.entry(group_id).or_insert(vec![]);
-        time_list.push(info);
-    }
-
-    group_hash
-}
-
 #[api(
     input: {
         properties: {
@@ -171,45 +158,61 @@ fn list_groups(
     let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
 
     let datastore = DataStore::lookup_datastore(&store)?;
+    let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
+
+    let backup_groups = BackupInfo::list_backup_groups(&datastore.base_path())?;
+
+    let group_info = backup_groups
+        .into_iter()
+        .fold(Vec::new(), |mut group_info, group| {
+            let owner = match datastore.get_owner(&group) {
+                Ok(auth_id) => auth_id,
+                Err(err) => {
+                    println!("Failed to get owner of group '{}' - {}", group, err);
+                    return group_info;
+                },
+            };
+            if !list_all && check_backup_owner(&owner, &auth_id).is_err() {
+                return group_info;
+            }
 
-    let backup_list = BackupInfo::list_backups(&datastore.base_path())?;
-
-    let group_hash = group_backups(backup_list);
-
-    let mut groups = Vec::new();
-
-    for (_group_id, mut list) in group_hash {
-
-        BackupInfo::sort_list(&mut list, false);
-
-        let info = &list[0];
+            let snapshots = match group.list_backups(&datastore.base_path()) {
+                Ok(snapshots) => snapshots,
+                Err(_) => {
+                    return group_info;
+                },
+            };
 
-        let group = info.backup_dir.group();
+            let backup_count: u64 = snapshots.len() as u64;
+            if backup_count == 0 {
+                return group_info;
+            }
 
-        let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
-        let owner = match datastore.get_owner(group) {
-            Ok(auth_id) => auth_id,
-            Err(err) => {
-                println!("Failed to get owner of group '{}' - {}", group, err);
-                continue;
-            },
-        };
-        if !list_all && check_backup_owner(&owner, &auth_id).is_err() {
-            continue;
-        }
+            let last_backup = snapshots
+                .iter()
+                .fold(&snapshots[0], |last, curr| {
+                    if curr.is_finished()
+                        && curr.backup_dir.backup_time() > last.backup_dir.backup_time() {
+                        curr
+                    } else {
+                        last
+                    }
+                })
+                .to_owned();
+
+            group_info.push(GroupListItem {
+                backup_type: group.backup_type().to_string(),
+                backup_id: group.backup_id().to_string(),
+                last_backup: last_backup.backup_dir.backup_time(),
+                owner: Some(owner),
+                backup_count,
+                files: last_backup.files,
+            });
 
-        let result_item = GroupListItem {
-            backup_type: group.backup_type().to_string(),
-            backup_id: group.backup_id().to_string(),
-            last_backup: info.backup_dir.backup_time(),
-            backup_count: list.len() as u64,
-            files: info.files.clone(),
-            owner: Some(owner),
-        };
-        groups.push(result_item);
-    }
+            group_info
+        });
 
-    Ok(groups)
+    Ok(group_info)
 }
 
 #[api(
@@ -357,41 +360,38 @@ pub fn list_snapshots (
     let user_info = CachedUserInfo::new()?;
     let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
 
+    let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
+
     let datastore = DataStore::lookup_datastore(&store)?;
 
     let base_path = datastore.base_path();
 
-    let backup_list = BackupInfo::list_backups(&base_path)?;
-
-    let mut snapshots = vec![];
-
-    for info in backup_list {
-        let group = info.backup_dir.group();
-        if let Some(ref backup_type) = backup_type {
-            if backup_type != group.backup_type() { continue; }
-        }
-        if let Some(ref backup_id) = backup_id {
-            if backup_id != group.backup_id() { continue; }
-        }
-
-        let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
-        let owner = match datastore.get_owner(group) {
-            Ok(auth_id) => auth_id,
-            Err(err) => {
-                println!("Failed to get owner of group '{}' - {}", group, err);
-                continue;
-            },
-        };
-
-        if !list_all && check_backup_owner(&owner, &auth_id).is_err() {
-            continue;
-        }
+    let groups = match (backup_type, backup_id) {
+        (Some(backup_type), Some(backup_id)) => {
+            let mut groups = Vec::with_capacity(1);
+            groups.push(BackupGroup::new(backup_type, backup_id));
+            groups
+        },
+        (Some(backup_type), None) => {
+            BackupInfo::list_backup_groups(&base_path)?
+                .into_iter()
+                .filter(|group| group.backup_type() == backup_type)
+                .collect()
+        },
+        (None, Some(backup_id)) => {
+            BackupInfo::list_backup_groups(&base_path)?
+                .into_iter()
+                .filter(|group| group.backup_id() == backup_id)
+                .collect()
+        },
+        _ => BackupInfo::list_backup_groups(&base_path)?,
+    };
 
-        let mut size = None;
+    let info_to_snapshot_list_item = |group: &BackupGroup, owner, info: BackupInfo| {
+        let backup_time = info.backup_dir.backup_time();
 
-        let (comment, verification, files) = match get_all_snapshot_files(&datastore, &info) {
+        let (comment, verification, files, size) = match get_all_snapshot_files(&datastore, &info) {
             Ok((manifest, files)) => {
-                size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum());
                 // extract the first line from notes
                 let comment: Option<String> = manifest.unprotected["notes"]
                     .as_str()
@@ -407,7 +407,9 @@ pub fn list_snapshots (
                     }
                 };
 
-                (comment, verify, files)
+                let size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum());
+
+                (comment, verify, files, size)
             },
             Err(err) => {
                 eprintln!("error during snapshot file listing: '{}'", err);
@@ -416,32 +418,58 @@ pub fn list_snapshots (
                     None,
                     info
                         .files
-                        .iter()
+                        .into_iter()
                         .map(|x| BackupContent {
                             filename: x.to_string(),
                             size: None,
                             crypt_mode: None,
                         })
-                        .collect()
+                        .collect(),
+                    None,
                 )
             },
         };
 
-        let result_item = SnapshotListItem {
+        SnapshotListItem {
             backup_type: group.backup_type().to_string(),
             backup_id: group.backup_id().to_string(),
-            backup_time: info.backup_dir.backup_time(),
+            backup_time,
             comment,
             verification,
             files,
             size,
-            owner: Some(owner),
-        };
+            owner,
+        }
+    };
 
-        snapshots.push(result_item);
-    }
+    groups
+        .iter()
+        .try_fold(Vec::new(), |mut snapshots, group| {
+            let owner = match datastore.get_owner(group) {
+                Ok(auth_id) => auth_id,
+                Err(err) => {
+                    eprintln!("Failed to get owner of group '{}/{}' - {}",
+                              &store,
+                              group,
+                              err);
+                    return Ok(snapshots);
+                },
+            };
+
+            if !list_all && check_backup_owner(&owner, &auth_id).is_err() {
+                return Ok(snapshots);
+            }
+
+            let group_backups = group.list_backups(&datastore.base_path())?;
+
+            snapshots.extend(
+                group_backups
+                    .into_iter()
+                    .map(|info| info_to_snapshot_list_item(&group, Some(owner.clone()), info))
+            );
 
-    Ok(snapshots)
+            Ok(snapshots)
+        })
 }
 
 fn get_snapshots_count(store: &DataStore) -> Result<Counts, Error> {
diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs
index 59849848..ebb5325b 100644
--- a/src/backup/backup_info.rs
+++ b/src/backup/backup_info.rs
@@ -327,29 +327,35 @@ impl BackupInfo {
         Ok(files)
     }
 
-    pub fn list_backups(base_path: &Path) -> Result<Vec<BackupInfo>, Error> {
+    pub fn list_backup_groups(base_path: &Path) -> Result<Vec<BackupGroup>, Error> {
         let mut list = Vec::new();
 
         tools::scandir(libc::AT_FDCWD, base_path, &BACKUP_TYPE_REGEX, |l0_fd, backup_type, file_type| {
             if file_type != nix::dir::Type::Directory { return Ok(()); }
-            tools::scandir(l0_fd, backup_type, &BACKUP_ID_REGEX, |l1_fd, backup_id, file_type| {
+            tools::scandir(l0_fd, backup_type, &BACKUP_ID_REGEX, |_, backup_id, file_type| {
                 if file_type != nix::dir::Type::Directory { return Ok(()); }
-                tools::scandir(l1_fd, backup_id, &BACKUP_DATE_REGEX, |l2_fd, backup_time_string, file_type| {
-                    if file_type != nix::dir::Type::Directory { return Ok(()); }
-
-                    let backup_dir = BackupDir::with_rfc3339(backup_type, backup_id, backup_time_string)?;
-
-                    let files = list_backup_files(l2_fd, backup_time_string)?;
 
-                    list.push(BackupInfo { backup_dir, files });
+                list.push(BackupGroup::new(backup_type, backup_id));
 
-                    Ok(())
-                })
+                Ok(())
             })
         })?;
+
         Ok(list)
     }
 
+    pub fn list_backups(base_path: &Path) -> Result<Vec<BackupInfo>, Error> {
+        let groups = BackupInfo::list_backup_groups(base_path)?;
+
+        groups
+            .into_iter()
+            .try_fold(Vec::new(), |mut snapshots, group| {
+                let group_snapshots = group.list_backups(base_path)?;
+                snapshots.extend(group_snapshots);
+                Ok(snapshots)
+            })
+    }
+
     pub fn is_finished(&self) -> bool {
         // backup is considered unfinished if there is no manifest
         self.files.iter().any(|name| name == super::MANIFEST_BLOB_NAME)
-- 
2.20.1





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 2/5] api: make expensive parts of datastore status opt-in
  2020-11-12 10:30 [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API Fabian Grünbichler
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 1/5] improve group/snapshot listing Fabian Grünbichler
@ 2020-11-12 10:30 ` Fabian Grünbichler
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 3/5] api: filter snapshot counts Fabian Grünbichler
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-12 10:30 UTC (permalink / raw)
  To: pbs-devel

used in the PBS GUI, but also for PVE usage queries which don't need all
the extra expensive information..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/admin/datastore.rs | 16 ++++++++++++++--
 src/api2/types/mod.rs       | 16 ++++++++++++----
 www/datastore/Summary.js    |  2 +-
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index df666156..857fb903 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -525,7 +525,14 @@ fn get_snapshots_count(store: &DataStore) -> Result<Counts, Error> {
             store: {
                 schema: DATASTORE_SCHEMA,
             },
+            verbose: {
+                type: bool,
+                default: false,
+                optional: true,
+                description: "Include additional information like snapshot counts and GC status.",
+            },
         },
+
     },
     returns: {
         type: DataStoreStatus,
@@ -537,13 +544,18 @@ fn get_snapshots_count(store: &DataStore) -> Result<Counts, Error> {
 /// Get datastore status.
 pub fn status(
     store: String,
+    verbose: bool,
     _info: &ApiMethod,
     _rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<DataStoreStatus, Error> {
     let datastore = DataStore::lookup_datastore(&store)?;
     let storage = crate::tools::disks::disk_usage(&datastore.base_path())?;
-    let counts = get_snapshots_count(&datastore)?;
-    let gc_status = datastore.last_gc_status();
+    let (counts, gc_status) = match verbose {
+        true => {
+            (Some(get_snapshots_count(&datastore)?), Some(datastore.last_gc_status()))
+        },
+        false => (None, None),
+    };
 
     Ok(DataStoreStatus {
         total: storage.total,
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 59323327..44cfef94 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -707,8 +707,14 @@ pub struct Counts {
 
 #[api(
     properties: {
-        "gc-status": { type: GarbageCollectionStatus, },
-        counts: { type: Counts, }
+        "gc-status": {
+            type: GarbageCollectionStatus,
+            optional: true,
+        },
+        counts: {
+            type: Counts,
+            optional: true,
+        },
     },
 )]
 #[derive(Serialize, Deserialize)]
@@ -722,9 +728,11 @@ pub struct DataStoreStatus {
     /// Available space (bytes).
     pub avail: u64,
     /// Status of last GC
-    pub gc_status: GarbageCollectionStatus,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub gc_status: Option<GarbageCollectionStatus>,
     /// Group/Snapshot counts
-    pub counts: Counts,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub counts: Option<Counts>,
 }
 
 #[api(
diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index 41fd7c85..5e7cfede 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -78,7 +78,7 @@ Ext.define('PBS.DataStoreInfo', {
 	    let datastore = encodeURIComponent(view.datastore);
 	    me.store = Ext.create('Proxmox.data.ObjectStore', {
 		interval: 5*1000,
-		url: `/api2/json/admin/datastore/${datastore}/status`,
+		url: `/api2/json/admin/datastore/${datastore}/status/?verbose=true`,
 	    });
 	    me.store.on('load', me.onLoad, me);
 	},
-- 
2.20.1





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 3/5] api: filter snapshot counts
  2020-11-12 10:30 [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API Fabian Grünbichler
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 1/5] improve group/snapshot listing Fabian Grünbichler
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 2/5] api: make expensive parts of datastore status opt-in Fabian Grünbichler
@ 2020-11-12 10:30 ` Fabian Grünbichler
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 4/5] drop now unused BackupInfo::list_backups Fabian Grünbichler
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-12 10:30 UTC (permalink / raw)
  To: pbs-devel

unprivileged users should only see the counts related to their part of
the datastore.

while we're at it, switch to a list groups, filter groups, count
snapshots approach (like list_snapshots) to speedup calls to this
endpoint when many unprivileged users share a datastore.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/admin/datastore.rs | 93 +++++++++++++++++++------------------
 src/api2/types/mod.rs       |  2 +-
 2 files changed, 48 insertions(+), 47 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 857fb903..6d9de4e4 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -472,51 +472,40 @@ pub fn list_snapshots (
         })
 }
 
-fn get_snapshots_count(store: &DataStore) -> Result<Counts, Error> {
+fn get_snapshots_count(store: &DataStore, filter_owner: Option<&Authid>) -> Result<Counts, Error> {
     let base_path = store.base_path();
-    let backup_list = BackupInfo::list_backups(&base_path)?;
-    let mut groups = HashSet::new();
-
-    let mut result = Counts {
-        ct: None,
-        host: None,
-        vm: None,
-        other: None,
-    };
-
-    for info in backup_list {
-        let group = info.backup_dir.group();
-
-        let id = group.backup_id();
-        let backup_type = group.backup_type();
-
-        let mut new_id = false;
-
-        if groups.insert(format!("{}-{}", &backup_type, &id)) {
-            new_id = true;
-        }
+    let groups = BackupInfo::list_backup_groups(&base_path)?;
 
-        let mut counts = match backup_type {
-            "ct" => result.ct.take().unwrap_or(Default::default()),
-            "host" => result.host.take().unwrap_or(Default::default()),
-            "vm" => result.vm.take().unwrap_or(Default::default()),
-            _ => result.other.take().unwrap_or(Default::default()),
-        };
+    groups.iter()
+        .filter(|group| {
+            let owner = match store.get_owner(&group) {
+                Ok(owner) => owner,
+                Err(err) => {
+                    eprintln!("Failed to get owner of group '{}' - {}", group, err);
+                    return false;
+                },
+            };
 
-        counts.snapshots += 1;
-        if new_id {
-            counts.groups +=1;
-        }
+            match filter_owner {
+                Some(filter) => check_backup_owner(&owner, filter).is_ok(),
+                None => true,
+            }
+        })
+        .try_fold(Counts::default(), |mut counts, group| {
+            let snapshot_count = group.list_backups(&base_path)?.len() as u64;
+
+            let type_count = match group.backup_type() {
+                "ct" => counts.ct.get_or_insert(Default::default()),
+                "vm" => counts.vm.get_or_insert(Default::default()),
+                "host" => counts.host.get_or_insert(Default::default()),
+                _ => counts.other.get_or_insert(Default::default()),
+            };
 
-        match backup_type {
-            "ct" => result.ct = Some(counts),
-            "host" => result.host = Some(counts),
-            "vm" => result.vm = Some(counts),
-            _ => result.other = Some(counts),
-        }
-    }
+            type_count.groups += 1;
+            type_count.snapshots += snapshot_count;
 
-    Ok(result)
+            Ok(counts)
+        })
 }
 
 #[api(
@@ -546,15 +535,27 @@ pub fn status(
     store: String,
     verbose: bool,
     _info: &ApiMethod,
-    _rpcenv: &mut dyn RpcEnvironment,
+    rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<DataStoreStatus, Error> {
     let datastore = DataStore::lookup_datastore(&store)?;
     let storage = crate::tools::disks::disk_usage(&datastore.base_path())?;
-    let (counts, gc_status) = match verbose {
-        true => {
-            (Some(get_snapshots_count(&datastore)?), Some(datastore.last_gc_status()))
-        },
-        false => (None, None),
+    let (counts, gc_status) = if verbose {
+        let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+        let user_info = CachedUserInfo::new()?;
+
+        let store_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
+        let filter_owner = if store_privs & PRIV_DATASTORE_AUDIT != 0 {
+            None
+        } else {
+            Some(&auth_id)
+        };
+
+        let counts = Some(get_snapshots_count(&datastore, filter_owner)?);
+        let gc_status = Some(datastore.last_gc_status());
+
+        (counts, gc_status)
+    } else {
+        (None, None)
     };
 
     Ok(DataStoreStatus {
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 44cfef94..d7d5a8af 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -692,7 +692,7 @@ pub struct TypeCounts {
         },
     },
 )]
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Default)]
 /// Counts of groups/snapshots per BackupType.
 pub struct Counts {
     /// The counts for CT backups
-- 
2.20.1





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 4/5] drop now unused BackupInfo::list_backups
  2020-11-12 10:30 [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API Fabian Grünbichler
                   ` (2 preceding siblings ...)
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 3/5] api: filter snapshot counts Fabian Grünbichler
@ 2020-11-12 10:30 ` Fabian Grünbichler
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 5/5] api: include store in invalid owner errors Fabian Grünbichler
  2020-11-18 10:14 ` [pbs-devel] applied series: [PATCH proxmox-backup 0/5] improve listing API Wolfgang Bumiller
  5 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-12 10:30 UTC (permalink / raw)
  To: pbs-devel

all global backup listing now happens via BackupGroup

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/backup/backup_info.rs | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs
index ebb5325b..367cb8ee 100644
--- a/src/backup/backup_info.rs
+++ b/src/backup/backup_info.rs
@@ -344,18 +344,6 @@ impl BackupInfo {
         Ok(list)
     }
 
-    pub fn list_backups(base_path: &Path) -> Result<Vec<BackupInfo>, Error> {
-        let groups = BackupInfo::list_backup_groups(base_path)?;
-
-        groups
-            .into_iter()
-            .try_fold(Vec::new(), |mut snapshots, group| {
-                let group_snapshots = group.list_backups(base_path)?;
-                snapshots.extend(group_snapshots);
-                Ok(snapshots)
-            })
-    }
-
     pub fn is_finished(&self) -> bool {
         // backup is considered unfinished if there is no manifest
         self.files.iter().any(|name| name == super::MANIFEST_BLOB_NAME)
-- 
2.20.1





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 5/5] api: include store in invalid owner errors
  2020-11-12 10:30 [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API Fabian Grünbichler
                   ` (3 preceding siblings ...)
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 4/5] drop now unused BackupInfo::list_backups Fabian Grünbichler
@ 2020-11-12 10:30 ` Fabian Grünbichler
  2020-11-18 10:14 ` [pbs-devel] applied series: [PATCH proxmox-backup 0/5] improve listing API Wolfgang Bumiller
  5 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-12 10:30 UTC (permalink / raw)
  To: pbs-devel

since a group might exist in plenty stores

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/admin/datastore.rs | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 6d9de4e4..872d081e 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -168,7 +168,10 @@ fn list_groups(
             let owner = match datastore.get_owner(&group) {
                 Ok(auth_id) => auth_id,
                 Err(err) => {
-                    println!("Failed to get owner of group '{}' - {}", group, err);
+                    eprintln!("Failed to get owner of group '{}/{}' - {}",
+                             &store,
+                             group,
+                             err);
                     return group_info;
                 },
             };
@@ -481,7 +484,10 @@ fn get_snapshots_count(store: &DataStore, filter_owner: Option<&Authid>) -> Resu
             let owner = match store.get_owner(&group) {
                 Ok(owner) => owner,
                 Err(err) => {
-                    eprintln!("Failed to get owner of group '{}' - {}", group, err);
+                    eprintln!("Failed to get owner of group '{}/{}' - {}",
+                              store.name(),
+                              group,
+                              err);
                     return false;
                 },
             };
-- 
2.20.1





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] applied series: [PATCH proxmox-backup 0/5] improve listing API
  2020-11-12 10:30 [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API Fabian Grünbichler
                   ` (4 preceding siblings ...)
  2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 5/5] api: include store in invalid owner errors Fabian Grünbichler
@ 2020-11-18 10:14 ` Wolfgang Bumiller
  5 siblings, 0 replies; 7+ messages in thread
From: Wolfgang Bumiller @ 2020-11-18 10:14 UTC (permalink / raw)
  To: Fabian Grünbichler; +Cc: pbs-devel

applied series

On Thu, Nov 12, 2020 at 11:30:29AM +0100, Fabian Grünbichler wrote:
> mainly relevant for non-Datastore.Audit users
> 
> - reduce number of file accesses when listing datastore contents
> - make status call as issued by PVE less expensive
> - filter snapshot counts for unpriv. users
> 
> Fabian Grünbichler (5):
>   improve group/snapshot listing
>   api: make expensive parts of datastore status opt-in
>   api: filter snapshot counts
>   drop now unused BackupInfo::list_backups
>   api: include store in invalid owner errors
> 
>  src/api2/admin/datastore.rs | 305 +++++++++++++++++++++---------------
>  src/api2/types/mod.rs       |  18 ++-
>  src/backup/backup_info.rs   |  16 +-
>  www/datastore/Summary.js    |   2 +-
>  4 files changed, 195 insertions(+), 146 deletions(-)
> 
> -- 
> 2.20.1




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-11-18 10:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-12 10:30 [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API Fabian Grünbichler
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 1/5] improve group/snapshot listing Fabian Grünbichler
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 2/5] api: make expensive parts of datastore status opt-in Fabian Grünbichler
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 3/5] api: filter snapshot counts Fabian Grünbichler
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 4/5] drop now unused BackupInfo::list_backups Fabian Grünbichler
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 5/5] api: include store in invalid owner errors Fabian Grünbichler
2020-11-18 10:14 ` [pbs-devel] applied series: [PATCH proxmox-backup 0/5] improve listing API Wolfgang Bumiller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal