[pbs-devel] [PATCH v3 proxmox-backup 06/20] datastore: allow filtering of backups by their trash state

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

Extends the BackupGroup::list_backups method by an enum parameter to
filter backup snapshots based on their trash state.

This allows to reuse the same logic for listing snapshots including
trashed, excluding trashed or only trashed snapshots.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 34 +++++++++++++++++++++++++++-----
 pbs-datastore/src/datastore.rs   |  4 ++--
 src/api2/admin/datastore.rs      | 15 ++++++++------
 src/api2/tape/backup.rs          |  4 ++--
 src/backup/verify.rs             |  6 +++---
 src/server/prune_job.rs          |  6 +++---
 src/server/pull.rs               |  8 ++++----
 tests/prune.rs                   |  1 +
 8 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index ec6d97790..53acc5baf 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -12,8 +12,8 @@ use proxmox_sys::fs::{lock_dir_noblock, lock_dir_noblock_shared, replace_file, C
 use proxmox_systemd::escape_unit;
 
 use pbs_api_types::{
-    Authid, BackupGroupDeleteStats, BackupNamespace, BackupType, GroupFilter, VerifyState,
-    BACKUP_DATE_REGEX, BACKUP_FILE_REGEX, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME,
+    Authid, BackupGroupDeleteStats, BackupNamespace, BackupType, GroupFilter, TrashStateFilter,
+    VerifyState, BACKUP_DATE_REGEX, BACKUP_FILE_REGEX, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME,
 };
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
@@ -106,7 +106,7 @@ impl BackupGroup {
         path.exists()
     }
 
-    pub fn list_backups(&self) -> Result<Vec<BackupInfo>, Error> {
+    pub fn list_backups(&self, filter: TrashStateFilter) -> Result<Vec<BackupInfo>, Error> {
         let mut list = vec![];
 
         let path = self.full_group_path();
@@ -124,11 +124,26 @@ impl BackupGroup {
                 let files = list_backup_files(l2_fd, backup_time)?;
 
                 let protected = backup_dir.is_protected();
+                let trash = backup_dir.is_trash();
+                match filter {
+                    TrashStateFilter::IncludeTrash => (),
+                    TrashStateFilter::OnlyTrash => {
+                        if !trash {
+                            return Ok(());
+                        }
+                    }
+                    TrashStateFilter::ExcludeTrash => {
+                        if trash {
+                            return Ok(());
+                        }
+                    }
+                }
 
                 list.push(BackupInfo {
                     backup_dir,
                     files,
                     protected,
+                    trash,
                 });
 
                 Ok(())
@@ -139,7 +154,7 @@ impl BackupGroup {
 
     /// Finds the latest backup inside a backup group
     pub fn last_backup(&self, only_finished: bool) -> Result<Option<BackupInfo>, Error> {
-        let backups = self.list_backups()?;
+        let backups = self.list_backups(TrashStateFilter::ExcludeTrash)?;
         Ok(backups
             .into_iter()
             .filter(|item| !only_finished || item.is_finished())
@@ -651,7 +666,12 @@ impl BackupDir {
             //
             // Do not error out, as we have already removed the snapshot, there is nothing a user could
             // do to rectify the situation.
-            if guard.is_ok() && group.list_backups()?.is_empty() && !*OLD_LOCKING {
+            if guard.is_ok()
+                && group
+                    .list_backups(TrashStateFilter::ExcludeTrash)?
+                    .is_empty()
+                && !*OLD_LOCKING
+            {
                 group.remove_group_dir()?;
             } else if let Err(err) = guard {
                 log::debug!("{err:#}");
@@ -801,6 +821,8 @@ pub struct BackupInfo {
     pub files: Vec<String>,
     /// Protection Status
     pub protected: bool,
+    /// Trash state
+    pub trash: bool,
 }
 
 impl BackupInfo {
@@ -809,11 +831,13 @@ impl BackupInfo {
 
         let files = list_backup_files(libc::AT_FDCWD, &path)?;
         let protected = backup_dir.is_protected();
+        let trash = backup_dir.is_trash();
 
         Ok(BackupInfo {
             backup_dir,
             files,
             protected,
+            trash,
         })
     }
 
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index e546bc532..dc4059789 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -24,7 +24,7 @@ use proxmox_worker_task::WorkerTaskContext;
 use pbs_api_types::{
     ArchiveType, Authid, BackupGroupDeleteStats, BackupNamespace, BackupType, ChunkOrder,
     DataStoreConfig, DatastoreFSyncLevel, DatastoreTuning, GarbageCollectionStatus,
-    MaintenanceMode, MaintenanceType, Operation, UPID,
+    MaintenanceMode, MaintenanceType, Operation, TrashStateFilter, UPID,
 };
 use pbs_config::BackupLockGuard;
 
@@ -1158,7 +1158,7 @@ impl DataStore {
                         _ => bail!("exhausted retries and unexpected counter overrun"),
                     };
 
-                    let mut snapshots = match group.list_backups() {
+                    let mut snapshots = match group.list_backups(TrashStateFilter::IncludeTrash) {
                         Ok(snapshots) => snapshots,
                         Err(err) => {
                             if group.exists() {
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index aa9202ed5..a59e39abe 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -42,8 +42,8 @@ use pbs_api_types::{
     DataStoreConfig, DataStoreListItem, DataStoreMountStatus, DataStoreStatus,
     GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, MaintenanceMode,
     MaintenanceType, Operation, PruneJobOptions, SnapshotListItem, SnapshotVerifyState,
-    BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
-    BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
+    TrashStateFilter, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
+    BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
     IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
     PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
     PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA,
@@ -223,7 +223,7 @@ pub fn list_groups(
                 return Ok(group_info);
             }
 
-            let snapshots = match group.list_backups() {
+            let snapshots = match group.list_backups(TrashStateFilter::ExcludeTrash) {
                 Ok(snapshots) => snapshots,
                 Err(_) => return Ok(group_info),
             };
@@ -542,6 +542,7 @@ unsafe fn list_snapshots_blocking(
             time: info.backup_dir.backup_time(),
         };
         let protected = info.protected;
+        let trash = if info.trash { Some(info.trash) } else { None };
 
         match get_all_snapshot_files(&info) {
             Ok((manifest, files)) => {
@@ -578,6 +579,7 @@ unsafe fn list_snapshots_blocking(
                     size,
                     owner,
                     protected,
+                    trash,
                 }
             }
             Err(err) => {
@@ -601,6 +603,7 @@ unsafe fn list_snapshots_blocking(
                     size: None,
                     owner,
                     protected,
+                    trash,
                 }
             }
         }
@@ -624,7 +627,7 @@ unsafe fn list_snapshots_blocking(
             return Ok(snapshots);
         }
 
-        let group_backups = group.list_backups()?;
+        let group_backups = group.list_backups(TrashStateFilter::ExcludeTrash)?;
 
         snapshots.extend(
             group_backups
@@ -657,7 +660,7 @@ async fn get_snapshots_count(
                 Ok(group) => group,
                 Err(_) => return Ok(counts), // TODO: add this as error counts?
             };
-            let snapshot_count = group.list_backups()?.len() as u64;
+            let snapshot_count = group.list_backups(TrashStateFilter::ExcludeTrash)?.len() as u64;
 
             // only include groups with snapshots, counting/displaying empty groups can confuse
             if snapshot_count > 0 {
@@ -1042,7 +1045,7 @@ pub fn prune(
     }
     let mut prune_result: Vec<PruneResult> = Vec::new();
 
-    let list = group.list_backups()?;
+    let list = group.list_backups(TrashStateFilter::ExcludeTrash)?;
 
     let mut prune_info = compute_prune_info(list, &keep_options)?;
 
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 31293a9a9..158905990 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -12,7 +12,7 @@ use proxmox_worker_task::WorkerTaskContext;
 
 use pbs_api_types::{
     print_ns_and_snapshot, print_store_and_ns, Authid, MediaPoolConfig, Operation,
-    TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, JOB_ID_SCHEMA,
+    TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, TrashStateFilter, JOB_ID_SCHEMA,
     PRIV_DATASTORE_READ, PRIV_TAPE_AUDIT, PRIV_TAPE_WRITE, UPID_SCHEMA,
 };
 
@@ -433,7 +433,7 @@ fn backup_worker(
         progress.done_snapshots = 0;
         progress.group_snapshots = 0;
 
-        let snapshot_list = group.list_backups()?;
+        let snapshot_list = group.list_backups(TrashStateFilter::ExcludeTrash)?;
 
         // filter out unfinished backups
         let mut snapshot_list: Vec<_> = snapshot_list
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 3d2cba8ac..bf7affe09 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -11,8 +11,8 @@ use proxmox_worker_task::WorkerTaskContext;
 
 use pbs_api_types::{
     print_ns_and_snapshot, print_store_and_ns, ArchiveType, Authid, BackupNamespace, BackupType,
-    CryptMode, SnapshotVerifyState, VerifyState, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_VERIFY,
-    UPID,
+    CryptMode, SnapshotVerifyState, TrashStateFilter, VerifyState, PRIV_DATASTORE_BACKUP,
+    PRIV_DATASTORE_VERIFY, UPID,
 };
 use pbs_datastore::backup_info::{BackupDir, BackupGroup, BackupInfo};
 use pbs_datastore::index::IndexFile;
@@ -411,7 +411,7 @@ pub fn verify_backup_group(
     filter: Option<&dyn Fn(&BackupManifest) -> bool>,
 ) -> Result<Vec<String>, Error> {
     let mut errors = Vec::new();
-    let mut list = match group.list_backups() {
+    let mut list = match group.list_backups(TrashStateFilter::ExcludeTrash) {
         Ok(list) => list,
         Err(err) => {
             info!(
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index 1c86647a0..24359efc7 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -4,8 +4,8 @@ use anyhow::Error;
 use tracing::{info, warn};
 
 use pbs_api_types::{
-    print_store_and_ns, Authid, KeepOptions, Operation, PruneJobOptions, MAX_NAMESPACE_DEPTH,
-    PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
+    print_store_and_ns, Authid, KeepOptions, Operation, PruneJobOptions, TrashStateFilter,
+    MAX_NAMESPACE_DEPTH, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
 };
 use pbs_datastore::prune::compute_prune_info;
 use pbs_datastore::DataStore;
@@ -54,7 +54,7 @@ pub fn prune_datastore(
     )? {
         let group = group?;
         let ns = group.backup_ns();
-        let list = group.list_backups()?;
+        let list = group.list_backups(TrashStateFilter::ExcludeTrash)?;
 
         let mut prune_info = compute_prune_info(list, &prune_options.keep)?;
         prune_info.reverse(); // delete older snapshots first
diff --git a/src/server/pull.rs b/src/server/pull.rs
index b1724c142..7aeb2bd56 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, TrashStateFilter,
+    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;
@@ -660,7 +660,7 @@ async fn pull_group(
             .target
             .store
             .backup_group(target_ns.clone(), group.clone());
-        let local_list = group.list_backups()?;
+        let local_list = group.list_backups(TrashStateFilter::ExcludeTrash)?;
         for info in local_list {
             let snapshot = info.backup_dir;
             if source_snapshots.contains(&snapshot.backup_time()) {
diff --git a/tests/prune.rs b/tests/prune.rs
index b11449ca0..02e9bc200 100644
--- a/tests/prune.rs
+++ b/tests/prune.rs
@@ -40,6 +40,7 @@ fn create_info(snapshot: &str, partial: bool) -> BackupInfo {
         backup_dir,
         files,
         protected: false,
+        trash: false,
     }
 }
 
-- 
2.39.5



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