[pbs-devel] [PATCH proxmox-backup 3/5] sync: source: list snapshot items instead of backup directories

[Christian Ebner <c.ebner@proxmox.com>]

The snapshot list items contain further information such as the
source verify and encryption state, so that allows filtering based on
these information after list generation already. Since this
information is already returned by the api calls in case of remote
sources and obtained by switching to `BackupGroup::list_backups`
instead of `BackupGroup::iter_snapshots` for local sources, return
the whole snapshot list item information instead of reducing it to
the subset containing the backup directory only when reading from
source.

Adapts the trait accordingly and renames `list_backup_dirs` to the
now better fitting `list_backup_snapshots`.

No functional changes intended.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/server/pull.rs | 16 +++++++++-------
 src/server/push.rs | 17 ++++++++++-------
 src/server/sync.rs | 39 ++++++++++++++++++++++-----------------
 3 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/src/server/pull.rs b/src/server/pull.rs
index b1724c142..832ee1b85 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -12,9 +12,9 @@ use tracing::info;
 
 use pbs_api_types::{
     print_store_and_ns, ArchiveType, Authid, BackupArchiveName, BackupDir, BackupGroup,
-    BackupNamespace, GroupFilter, Operation, RateLimitConfig, Remote, VerifyState,
-    CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, PRIV_DATASTORE_AUDIT,
-    PRIV_DATASTORE_BACKUP,
+    BackupNamespace, GroupFilter, Operation, RateLimitConfig, Remote, SnapshotListItem,
+    VerifyState, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH,
+    PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
 };
 use pbs_client::BackupRepository;
 use pbs_config::CachedUserInfo;
@@ -541,11 +541,11 @@ async fn pull_group(
     let mut already_synced_skip_info = SkipInfo::new(SkipReason::AlreadySynced);
     let mut transfer_last_skip_info = SkipInfo::new(SkipReason::TransferLast);
 
-    let mut raw_list: Vec<BackupDir> = params
+    let mut raw_list: Vec<SnapshotListItem> = params
         .source
-        .list_backup_dirs(source_namespace, group)
+        .list_backup_snapshots(source_namespace, group)
         .await?;
-    raw_list.sort_unstable_by(|a, b| a.time.cmp(&b.time));
+    raw_list.sort_unstable_by(|a, b| a.backup.time.cmp(&b.backup.time));
 
     let total_amount = raw_list.len();
 
@@ -568,7 +568,9 @@ async fn pull_group(
     let list: Vec<(BackupDir, bool)> = raw_list
         .into_iter()
         .enumerate()
-        .filter_map(|(pos, dir)| {
+        .filter_map(|(pos, item)| {
+            let dir = item.backup;
+
             source_snapshots.insert(dir.time);
             // If resync_corrupt is set, check if the corresponding local snapshot failed to
             // verification
diff --git a/src/server/push.rs b/src/server/push.rs
index e71012ed8..fdfed1b5a 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -672,8 +672,11 @@ pub(crate) async fn push_group(
     let mut already_synced_skip_info = SkipInfo::new(SkipReason::AlreadySynced);
     let mut transfer_last_skip_info = SkipInfo::new(SkipReason::TransferLast);
 
-    let mut snapshots: Vec<BackupDir> = params.source.list_backup_dirs(namespace, group).await?;
-    snapshots.sort_unstable_by(|a, b| a.time.cmp(&b.time));
+    let mut snapshots: Vec<SnapshotListItem> = params
+        .source
+        .list_backup_snapshots(namespace, group)
+        .await?;
+    snapshots.sort_unstable_by(|a, b| a.backup.time.cmp(&b.backup.time));
 
     if snapshots.is_empty() {
         info!("Group '{group}' contains no snapshots to sync to remote");
@@ -698,19 +701,19 @@ pub(crate) async fn push_group(
     let snapshots: Vec<BackupDir> = snapshots
         .into_iter()
         .enumerate()
-        .filter(|&(pos, ref snapshot)| {
+        .filter_map(|(pos, item)| {
+            let snapshot = item.backup;
             source_snapshots.insert(snapshot.time);
             if last_snapshot_time >= snapshot.time {
                 already_synced_skip_info.update(snapshot.time);
-                return false;
+                return None;
             }
             if pos < cutoff {
                 transfer_last_skip_info.update(snapshot.time);
-                return false;
+                return None;
             }
-            true
+            Some(snapshot)
         })
-        .map(|(_, dir)| dir)
         .collect();
 
     if already_synced_skip_info.count > 0 {
diff --git a/src/server/sync.rs b/src/server/sync.rs
index 09814ef0c..155a0cb4b 100644
--- a/src/server/sync.rs
+++ b/src/server/sync.rs
@@ -32,6 +32,7 @@ use crate::backup::ListAccessibleBackupGroups;
 use crate::server::jobstate::Job;
 use crate::server::pull::{pull_store, PullParameters};
 use crate::server::push::{push_store, PushParameters};
+use crate::tools::backup_info_to_snapshot_list_item;
 
 #[derive(Default)]
 pub(crate) struct RemovedVanishedStats {
@@ -244,11 +245,11 @@ pub(crate) trait SyncSource: Send + Sync {
     ) -> Result<Vec<BackupGroup>, Error>;
 
     /// Lists backup directories for a specific group within a specific namespace from the source.
-    async fn list_backup_dirs(
+    async fn list_backup_snapshots(
         &self,
         namespace: &BackupNamespace,
         group: &BackupGroup,
-    ) -> Result<Vec<BackupDir>, Error>;
+    ) -> Result<Vec<SnapshotListItem>, Error>;
     fn get_ns(&self) -> BackupNamespace;
     fn get_store(&self) -> &str;
 
@@ -357,11 +358,11 @@ impl SyncSource for RemoteSource {
         )
     }
 
-    async fn list_backup_dirs(
+    async fn list_backup_snapshots(
         &self,
         namespace: &BackupNamespace,
         group: &BackupGroup,
-    ) -> Result<Vec<BackupDir>, Error> {
+    ) -> Result<Vec<SnapshotListItem>, Error> {
         let path = format!("api2/json/admin/datastore/{}/snapshots", self.repo.store());
 
         let mut args = json!({
@@ -380,16 +381,15 @@ impl SyncSource for RemoteSource {
         Ok(snapshot_list
             .into_iter()
             .filter_map(|item: SnapshotListItem| {
-                let snapshot = item.backup;
                 // in-progress backups can't be synced
                 if item.size.is_none() {
-                    info!("skipping snapshot {snapshot} - in-progress backup");
+                    info!("skipping snapshot {} - in-progress backup", item.backup);
                     return None;
                 }
 
-                Some(snapshot)
+                Some(item)
             })
-            .collect::<Vec<BackupDir>>())
+            .collect::<Vec<SnapshotListItem>>())
     }
 
     fn get_ns(&self) -> BackupNamespace {
@@ -451,18 +451,23 @@ impl SyncSource for LocalSource {
         .collect::<Vec<pbs_api_types::BackupGroup>>())
     }
 
-    async fn list_backup_dirs(
+    async fn list_backup_snapshots(
         &self,
         namespace: &BackupNamespace,
         group: &BackupGroup,
-    ) -> Result<Vec<BackupDir>, Error> {
-        Ok(self
-            .store
-            .backup_group(namespace.clone(), group.clone())
-            .iter_snapshots()?
-            .filter_map(Result::ok)
-            .map(|snapshot| snapshot.dir().to_owned())
-            .collect::<Vec<BackupDir>>())
+    ) -> Result<Vec<SnapshotListItem>, Error> {
+        let backup_group = self.store.backup_group(namespace.clone(), group.clone());
+        Ok(backup_group
+            .list_backups()?
+            .iter()
+            .filter_map(|info| {
+                let owner = match backup_group.get_owner() {
+                    Ok(owner) => owner,
+                    Err(_) => return None,
+                };
+                Some(backup_info_to_snapshot_list_item(info, &owner))
+            })
+            .collect::<Vec<SnapshotListItem>>())
     }
 
     fn get_ns(&self) -> BackupNamespace {
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel