[pbs-devel] [PATCH proxmox-backup v2 03/12] chunk store: add unsafe signature to cache remove method

[Christian Ebner]

Removing a chunk file from the local datastore cache is rather unsafe
as several preconditions have to be met:
- The chunk store mutex guard has to be held, in order to avoid
  concurrent operations on the chunk file
- It must be assured that the chunk to be removed is not referenced
  by any visible index file.
- It must be assured that the chunk is not being indexed by an active
  index writer (ongoing backup).
- It must be assured that the chunk is not being indexed by an active
  index writer in an old process, still active after service reload
  (ongoing backup in old process).

Add the unsafe signature to `LocalDatastoreLRUCache::remove()` to
signal these preconditions and limit the scope to be crate only.

Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
 pbs-datastore/src/datastore.rs                 |  2 +-
 pbs-datastore/src/local_datastore_lru_cache.rs | 10 +++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 7ef16c31e..acf22e9b0 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1686,7 +1686,7 @@ impl DataStore {
                         |_status| {
                             if let Some(cache) = self.cache() {
                                 // ignore errors, phase 3 will retry cleanup anyways
-                                let _ = cache.remove(&digest);
+                                let _ = unsafe { cache.remove(&digest) };
                             }
                             delete_list.push(content.key);
                             Ok(())
diff --git a/pbs-datastore/src/local_datastore_lru_cache.rs b/pbs-datastore/src/local_datastore_lru_cache.rs
index c0edd3619..12b7f0aaa 100644
--- a/pbs-datastore/src/local_datastore_lru_cache.rs
+++ b/pbs-datastore/src/local_datastore_lru_cache.rs
@@ -86,8 +86,16 @@ impl LocalDatastoreLruCache {
 
     /// Remove a chunk from the local datastore cache.
     ///
+    /// Callers to this method must assure that:
+    /// - no concurrent insert is being performed, the chunk store's mutex must be held.
+    /// - the chunk to be removed is no longer referenced by an index file.
+    /// - the chunk to be removed has not been inserted by an active writer (atime newer than
+    ///   writer start time).
+    /// - there is no active writer in an old process, which could have inserted the chunk to be
+    ///   deleted.
+    ///
     /// Fails if the chunk cannot be deleted successfully.
-    pub fn remove(&self, digest: &[u8; 32]) -> Result<(), Error> {
+    pub(crate) unsafe fn remove(&self, digest: &[u8; 32]) -> Result<(), Error> {
         self.cache.remove(*digest);
         let (path, _digest_str) = self.store.chunk_path(digest);
         std::fs::remove_file(path).map_err(Error::from)
-- 
2.47.3