From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id C49B71FF165
	for <inbox@lore.proxmox.com>; Thu,  8 May 2025 15:05:58 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 52F25F993;
	Thu,  8 May 2025 15:06:11 +0200 (CEST)
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Thu,  8 May 2025 15:05:36 +0200
Message-Id: <20250508130555.494782-3-c.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250508130555.494782-1-c.ebner@proxmox.com>
References: <20250508130555.494782-1-c.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.028 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [RFC v2 proxmox-backup 02/21] datastore: mark groups as
 trash on destroy
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

In order to implement the trash can functionality, mark all the
snapshots of the group and the group itself as trash instead of
deleting them right away. Cleanup of the group is deferred to the
garbage collection.

Groups and snapshots are marked by the trash marker file. New backups
to this group will check for the marker file (see subsequent
commits), clearing the whole group and all of the snapshots to
create a new snapshot within that group. Otherwise ownership
conflicts could arise. This implies that a new backup clears the
whole trashed group.

Snapshots already marked as trash within the same backup group will
be cleared as well when the group is requested to be destroyed with
skip trash.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 19 ++++++++++++++++---
 pbs-datastore/src/datastore.rs   |  4 ++--
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 76bcd15f5..9ce4cb0f8 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -215,7 +215,7 @@ impl BackupGroup {
     ///
     /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots
     /// and number of protected snaphsots, which therefore were not removed.
-    pub fn destroy(&self) -> Result<BackupGroupDeleteStats, Error> {
+    pub fn destroy(&self, skip_trash: bool) -> Result<BackupGroupDeleteStats, Error> {
         let _guard = self
             .lock()
             .with_context(|| format!("while destroying group '{self:?}'"))?;
@@ -229,14 +229,20 @@ impl BackupGroup {
                 delete_stats.increment_protected_snapshots();
                 continue;
             }
-            snap.destroy(false, false)?;
+            snap.destroy(false, skip_trash)?;
             delete_stats.increment_removed_snapshots();
         }
 
         // Note: make sure the old locking mechanism isn't used as `remove_dir_all` is not safe in
         // that case
         if delete_stats.all_removed() && !*OLD_LOCKING {
-            self.remove_group_dir()?;
+            if skip_trash {
+                self.remove_group_dir()?;
+            } else {
+                let path = self.full_group_path().join(TRASH_MARKER_FILENAME);
+                let _trash_file =
+                    std::fs::File::create(path).context("failed to set trash file")?;
+            }
             delete_stats.increment_removed_groups();
         }
 
@@ -245,6 +251,13 @@ impl BackupGroup {
 
     /// Helper function, assumes that no more snapshots are present in the group.
     fn remove_group_dir(&self) -> Result<(), Error> {
+        let trash_path = self.full_group_path().join(TRASH_MARKER_FILENAME);
+        if let Err(err) = std::fs::remove_file(&trash_path) {
+            if err.kind() != std::io::ErrorKind::NotFound {
+                bail!("removing the trash file '{trash_path:?}' failed - {err}")
+            }
+        }
+
         let owner_path = self.store.owner_path(&self.ns, &self.group);
 
         std::fs::remove_file(&owner_path).map_err(|err| {
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 6df26e825..e546bc532 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -581,7 +581,7 @@ impl DataStore {
         let mut stats = BackupGroupDeleteStats::default();
 
         for group in self.iter_backup_groups(ns.to_owned())? {
-            let delete_stats = group?.destroy()?;
+            let delete_stats = group?.destroy(true)?;
             stats.add(&delete_stats);
             removed_all_groups = removed_all_groups && delete_stats.all_removed();
         }
@@ -674,7 +674,7 @@ impl DataStore {
     ) -> Result<BackupGroupDeleteStats, Error> {
         let backup_group = self.backup_group(ns.clone(), backup_group.clone());
 
-        backup_group.destroy()
+        backup_group.destroy(true)
     }
 
     /// Remove a backup directory including all content
-- 
2.39.5



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