From 556609984982cb34e4dafbb67f249a580168361d Mon Sep 17 00:00:00 2001
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Date: Wed, 11 May 2022 18:39:05 +0200
Subject: [PATCH] datastore: move cleanup_unreferenced_files to BackupDir impl
 and fix NS awareness

sync failed on cleanup due to always trying to do so in the root NS

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 38 ++++++++++++++++++++++-
 pbs-datastore/src/datastore.rs   | 53 +-------------------------------
 src/server/pull.rs               |  5 +--
 3 files changed, 39 insertions(+), 57 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index bffd26ed..c9aa72b8 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -13,7 +13,9 @@ use pbs_api_types::{
 };
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
-use crate::manifest::{BackupManifest, MANIFEST_BLOB_NAME, MANIFEST_LOCK_NAME};
+use crate::manifest::{
+    BackupManifest, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MANIFEST_LOCK_NAME,
+};
 use crate::{DataBlob, DataStore};
 
 /// BackupGroup is a directory containing a list of BackupDir
@@ -493,6 +495,40 @@ impl BackupDir {
     ) -> Result<(), Error> {
         self.store.update_manifest(self, update_fn)
     }
+
+    /// Cleans up the backup directory by removing any file not mentioned in the manifest.
+    pub fn cleanup_unreferenced_files(&self, manifest: &BackupManifest) -> Result<(), Error> {
+        let full_path = self.full_path();
+
+        let mut wanted_files = std::collections::HashSet::new();
+        wanted_files.insert(MANIFEST_BLOB_NAME.to_string());
+        wanted_files.insert(CLIENT_LOG_BLOB_NAME.to_string());
+        manifest.files().iter().for_each(|item| {
+            wanted_files.insert(item.filename.clone());
+        });
+
+        for item in proxmox_sys::fs::read_subdir(libc::AT_FDCWD, &full_path)?.flatten() {
+            if let Some(file_type) = item.file_type() {
+                if file_type != nix::dir::Type::File {
+                    continue;
+                }
+            }
+            let file_name = item.file_name().to_bytes();
+            if file_name == b"." || file_name == b".." {
+                continue;
+            };
+            if let Ok(name) = std::str::from_utf8(file_name) {
+                if wanted_files.contains(name) {
+                    continue;
+                }
+            }
+            println!("remove unused file {:?}", item.file_name());
+            let dirfd = item.parent_fd();
+            let _res = unsafe { libc::unlinkat(dirfd, item.file_name().as_ptr(), 0) };
+        }
+
+        Ok(())
+    }
 }
 
 impl AsRef<pbs_api_types::BackupNamespace> for BackupDir {
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 57f995d4..9a63a9e0 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -28,9 +28,7 @@ use crate::chunk_store::ChunkStore;
 use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
 use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
 use crate::index::IndexFile;
-use crate::manifest::{
-    archive_type, ArchiveType, BackupManifest, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME,
-};
+use crate::manifest::{archive_type, ArchiveType, BackupManifest, MANIFEST_BLOB_NAME};
 use crate::task_tracking::update_active_operations;
 use crate::DataBlob;
 
@@ -360,55 +358,6 @@ impl DataStore {
         path
     }
 
-    /// Cleanup a backup directory
-    ///
-    /// Removes all files not mentioned in the manifest.
-    pub fn cleanup_backup_dir(
-        &self,
-        backup_dir: impl AsRef<pbs_api_types::BackupDir>,
-        manifest: &BackupManifest,
-    ) -> Result<(), Error> {
-        self.cleanup_backup_dir_do(backup_dir.as_ref(), manifest)
-    }
-
-    fn cleanup_backup_dir_do(
-        &self,
-        backup_dir: &pbs_api_types::BackupDir,
-        manifest: &BackupManifest,
-    ) -> Result<(), Error> {
-        let mut full_path = self.base_path();
-        full_path.push(backup_dir.to_string());
-
-        let mut wanted_files = HashSet::new();
-        wanted_files.insert(MANIFEST_BLOB_NAME.to_string());
-        wanted_files.insert(CLIENT_LOG_BLOB_NAME.to_string());
-        manifest.files().iter().for_each(|item| {
-            wanted_files.insert(item.filename.clone());
-        });
-
-        for item in proxmox_sys::fs::read_subdir(libc::AT_FDCWD, &full_path)?.flatten() {
-            if let Some(file_type) = item.file_type() {
-                if file_type != nix::dir::Type::File {
-                    continue;
-                }
-            }
-            let file_name = item.file_name().to_bytes();
-            if file_name == b"." || file_name == b".." {
-                continue;
-            };
-            if let Ok(name) = std::str::from_utf8(file_name) {
-                if wanted_files.contains(name) {
-                    continue;
-                }
-            }
-            println!("remove unused file {:?}", item.file_name());
-            let dirfd = item.parent_fd();
-            let _res = unsafe { libc::unlinkat(dirfd, item.file_name().as_ptr(), 0) };
-        }
-
-        Ok(())
-    }
-
     /// Returns the absolute path for a backup_group
     pub fn group_path(
         &self,
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 19b1df8d..ae84d134 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -506,10 +506,7 @@ async fn pull_snapshot(
         try_client_log_download(worker, reader, &client_log_name).await?;
     }
 
-    // cleanup - remove stale files
-    snapshot
-        .datastore()
-        .cleanup_backup_dir(snapshot, &manifest)?;
+    snapshot.cleanup_unreferenced_files(&manifest)?;
 
     Ok(())
 }