[pbs-devel] [PATCH v3 proxmox proxmox-backup 00/20] implement trash bin functionality

[pbs-devel] [PATCH v3 proxmox proxmox-backup 00/20] implement trash bin functionality

[Christian Ebner]

This patch series implements a trash bin functionality, marking
backup snapshots and groups as trashed on prune and forget instead of
deleting them immediately. Cleanup is deferred to the garbage
collection job, allowing to recover the trashed items if removed by
accident.

Items are marked as trash by creating a `.trashed` marker file within
the respective snapshot or group directory. If group directories
contain the marker file, the whole group is considered trash. New
backups to a trashed group will remove the group trash marker file,
but only if the group is owned by the user creating the backup.
Backup creation will fail otherwise, in order to guarantee ownership.
If an backup is created to a pre-existing but trashed backup
directory, the backup directories contents are cleared and the backup
can proceed as if no such snapshot existed.

Most notably changes since the previous RFC version [0] (thanks Fabian
for feedback and discussion):
- Do not allow to mark namespaces as trash. Instead, in order to
  remove a namespace all contents (including trashed items) must be
  deleted. Drop all related patches as there is no longer the need to
  operate and filter on namespaces directly.
- Use exclusive snapshot/group locks in order to guarantee
  consistency during concurrent operations.
- Only clear backup snapshot directories in case of a new backup to the
  same backup directory. For groups, only allow to create a new backup
  to a trashed group if the ownership matches, but do not clear
  pre-existing trashed contents.
- Use pre-existing helpers for snapshot/group cleanup during garbage
  collection.
- Allow to clear only the trashed contents for a namespace or group via
  the API/WebUI, not removing all contents.
- Allow to filter for all trash states while listing snapshots,
  introducing a dedicated api type for this.
- Fix several smaller UI issues

As most of the patches changed significantly and many got dropped while
new ones added, no per-patch changes are noted.

Note: Patches 5 and onwards require patches 1 and 2 to the PBS api types
for compilation.

[0] https://lore.proxmox.com/pbs-devel/20250508130555.494782-1-c.ebner@proxmox.com/T/

proxmox:

Christian Ebner (2):
  pbs api types: add type for snapshot list filtering based on trash
    state
  pbs api types: datastore: add trash marker to snapshot list item

 pbs-api-types/src/datastore.rs | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

proxmox-backup:

Christian Ebner (18):
  datastore/api: mark snapshots as trash on destroy
  datastore: mark groups as trash on destroy
  datastore: add helpers to check if snapshot/group is trash
  datastore: allow filtering of backups by their trash state
  api: datastore: add trash state filtering for snapshot listing
  datastore: ignore trashed snapshots for last successful backup
  sync: ignore trashed snapshots/groups when reading from local source
  api: tape: check trash marker when trying to write snapshot
  datastore: clear trashed snapshot dir if re-creation requested
  datastore: recover backup group from trash for new backups
  datastore: garbage collection: clean-up trashed snapshots and groups
  client: expose skip trash flags for cli commands
  api: admin: implement endpoints to recover trashed contents
  api: admin: move backup group list generation into helper
  api: admin: add endpoint to clear trashed items from group
  ui: add recover for trashed items tab to datastore panel
  ui: drop 'permanent' in group/snapshot forget, default is to trash
  ui: mention trash items will be cleared on namespace deletion

 pbs-datastore/src/backup_info.rs      | 152 +++--
 pbs-datastore/src/datastore.rs        |  90 ++-
 proxmox-backup-client/src/group.rs    |  14 +-
 proxmox-backup-client/src/snapshot.rs |  16 +-
 src/api2/admin/datastore.rs           | 319 ++++++++--
 src/api2/backup/environment.rs        |   1 +
 src/api2/tape/backup.rs               |  12 +-
 src/backup/verify.rs                  |   6 +-
 src/server/prune_job.rs               |  10 +-
 src/server/pull.rs                    |  20 +-
 src/server/sync.rs                    |   2 +
 tests/prune.rs                        |   1 +
 www/Makefile                          |   1 +
 www/Utils.js                          |   1 +
 www/datastore/Content.js              |   4 +-
 www/datastore/Panel.js                |   8 +
 www/datastore/RecoverTrashed.js       | 805 ++++++++++++++++++++++++++
 www/window/NamespaceEdit.js           |   2 +-
 18 files changed, 1359 insertions(+), 105 deletions(-)
 create mode 100644 www/datastore/RecoverTrashed.js

-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox 1/20] pbs api types: add type for snapshot list filtering based on trash state

[Christian Ebner]

Adds a dedicated enum to allow filtering snapshots based on their
trash state. Allows to include, exclude or only show trashed
snapshots when listing.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 5bd953ac..d50d6f2b 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -1772,6 +1772,20 @@ impl BackupGroupDeleteStats {
     }
 }
 
+#[api()]
+#[derive(Serialize, Deserialize, Copy, Clone, Default, Debug, PartialOrd, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// Filter snapshots based on their trash state.
+pub enum TrashStateFilter {
+    #[default]
+    /// Exclude snapshots which are marked as trash.
+    ExcludeTrash,
+    /// Include snapshots which are marked as trash.
+    IncludeTrash,
+    /// Only include snapshots which are marked as trash.
+    OnlyTrash,
+}
+
 #[derive(Clone, PartialEq, Eq)]
 /// Allowed variants of backup archives to be contained in a snapshot's manifest
 pub enum ArchiveType {
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox 2/20] pbs api types: datastore: add trash marker to snapshot list item

[Christian Ebner]

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index d50d6f2b..ef5700d8 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -1311,6 +1311,9 @@ pub struct SnapshotListItem {
     /// Protection from prunes
     #[serde(default)]
     pub protected: bool,
+    /// Snapshot is marked as trash, only present if marked as trash
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub trash: Option<bool>,
 }
 
 #[api(
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 03/20] datastore/api: mark snapshots as trash on destroy

[Christian Ebner]

In order to implement the trash functionality, mark snapshots as
trash instead of removing them by default. However, provide a
`skip-trash` flag to opt-out and destroy the snapshot including it's
contents immediately.

Trashed snapshots are marked by creating a `.trashed` marker file
inside the snapshot folder. Actual removal of the snapshot will be
deferred to the garbage collection task.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 66 ++++++++++++++++++--------------
 pbs-datastore/src/datastore.rs   |  2 +-
 src/api2/admin/datastore.rs      | 18 ++++++++-
 3 files changed, 55 insertions(+), 31 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index d4732fdd9..76bcd15f5 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -21,6 +21,7 @@ use crate::manifest::{BackupManifest, MANIFEST_LOCK_NAME};
 use crate::{DataBlob, DataStore};
 
 pub const DATASTORE_LOCKS_DIR: &str = "/run/proxmox-backup/locks";
+pub const TRASH_MARKER_FILENAME: &str = ".trashed";
 
 // TODO: Remove with PBS 5
 // Note: The `expect()` call here will only happen if we can neither confirm nor deny the existence
@@ -228,7 +229,7 @@ impl BackupGroup {
                 delete_stats.increment_protected_snapshots();
                 continue;
             }
-            snap.destroy(false)?;
+            snap.destroy(false, false)?;
             delete_stats.increment_removed_snapshots();
         }
 
@@ -575,7 +576,8 @@ impl BackupDir {
     /// Destroy the whole snapshot, bails if it's protected
     ///
     /// Setting `force` to true skips locking and thus ignores if the backup is currently in use.
-    pub fn destroy(&self, force: bool) -> Result<(), Error> {
+    /// Setting `skip_trash` to true will remove the snapshot instead of marking it as trash.
+    pub fn destroy(&self, force: bool, skip_trash: bool) -> Result<(), Error> {
         let (_guard, _manifest_guard);
         if !force {
             _guard = self
@@ -588,37 +590,45 @@ impl BackupDir {
             bail!("cannot remove protected snapshot"); // use special error type?
         }
 
-        let full_path = self.full_path();
-        log::info!("removing backup snapshot {:?}", full_path);
-        std::fs::remove_dir_all(&full_path).map_err(|err| {
-            format_err!("removing backup snapshot {:?} failed - {}", full_path, err,)
-        })?;
+        let mut full_path = self.full_path();
+        log::info!("removing backup snapshot {full_path:?}");
+        if skip_trash {
+            std::fs::remove_dir_all(&full_path).map_err(|err| {
+                format_err!("removing backup snapshot {full_path:?} failed - {err}")
+            })?;
+        } else {
+            full_path.push(TRASH_MARKER_FILENAME);
+            let _trash_file =
+                std::fs::File::create(full_path).context("failed to set trash file")?;
+        }
 
         // remove no longer needed lock files
         let _ = std::fs::remove_file(self.manifest_lock_path()); // ignore errors
         let _ = std::fs::remove_file(self.lock_path()); // ignore errors
 
-        let group = BackupGroup::from(self);
-        let guard = group.lock().with_context(|| {
-            format!("while checking if group '{group:?}' is empty during snapshot destruction")
-        });
-
-        // Only remove the group if all of the following is true:
-        //
-        // - we can lock it: if we can't lock the group, it is still in use (either by another
-        //   backup process or a parent caller (who needs to take care that empty groups are
-        //   removed themselves).
-        // - it is now empty: if the group isn't empty, removing it will fail (to avoid removing
-        //   backups that might still be used).
-        // - the new locking mechanism is used: if the old mechanism is used, a group removal here
-        //   could lead to a race condition.
-        //
-        // 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 {
-            group.remove_group_dir()?;
-        } else if let Err(err) = guard {
-            log::debug!("{err:#}");
+        if skip_trash {
+            let group = BackupGroup::from(self);
+            let guard = group.lock().with_context(|| {
+                format!("while checking if group '{group:?}' is empty during snapshot destruction")
+            });
+
+            // Only remove the group if all of the following is true:
+            //
+            // - we can lock it: if we can't lock the group, it is still in use (either by another
+            //   backup process or a parent caller (who needs to take care that empty groups are
+            //   removed themselves).
+            // - it is now empty: if the group isn't empty, removing it will fail (to avoid removing
+            //   backups that might still be used).
+            // - the new locking mechanism is used: if the old mechanism is used, a group removal here
+            //   could lead to a race condition.
+            //
+            // 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 {
+                group.remove_group_dir()?;
+            } else if let Err(err) = guard {
+                log::debug!("{err:#}");
+            }
         }
 
         Ok(())
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index cbf78ecb6..6df26e825 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -686,7 +686,7 @@ impl DataStore {
     ) -> Result<(), Error> {
         let backup_dir = self.backup_dir(ns.clone(), backup_dir.clone())?;
 
-        backup_dir.destroy(force)
+        backup_dir.destroy(force, true)
     }
 
     /// Returns the time of the last successful backup
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 392494488..aa9202ed5 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -402,6 +402,12 @@ pub async fn list_snapshot_files(
                 type: pbs_api_types::BackupDir,
                 flatten: true,
             },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                default: false,
+                description: "Immediately remove the snapshot, not marking it as trash.",
+            },
         },
     },
     access: {
@@ -415,6 +421,7 @@ pub async fn delete_snapshot(
     store: String,
     ns: Option<BackupNamespace>,
     backup_dir: pbs_api_types::BackupDir,
+    skip_trash: bool,
     _info: &ApiMethod,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
@@ -435,7 +442,7 @@ pub async fn delete_snapshot(
 
         let snapshot = datastore.backup_dir(ns, backup_dir)?;
 
-        snapshot.destroy(false)?;
+        snapshot.destroy(false, skip_trash)?;
 
         Ok(Value::Null)
     })
@@ -979,6 +986,12 @@ pub fn verify(
                 optional: true,
                 description: "Spins up an asynchronous task that does the work.",
             },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                default: false,
+                description: "Immediately remove the group including all snapshots, not marking it as trash.",
+            },
         },
     },
     returns: pbs_api_types::ADMIN_DATASTORE_PRUNE_RETURN_TYPE,
@@ -995,6 +1008,7 @@ pub fn prune(
     keep_options: KeepOptions,
     store: String,
     ns: Option<BackupNamespace>,
+    skip_trash: bool,
     param: Value,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
@@ -1098,7 +1112,7 @@ pub fn prune(
             });
 
             if !keep {
-                if let Err(err) = backup_dir.destroy(false) {
+                if let Err(err) = backup_dir.destroy(false, skip_trash) {
                     warn!(
                         "failed to remove dir {:?}: {}",
                         backup_dir.relative_path(),
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 04/20] datastore: mark groups as trash on destroy

[Christian Ebner]

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.

The group is marked as trash in order to skip over it in listings
without trashed items and to force an owner check when re-creation is
requested by a backup task to this 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

[pbs-devel] [PATCH v3 proxmox-backup 05/20] datastore: add helpers to check if snapshot/group is trash

[Christian Ebner]

The convenience helper method check for the trash marker file being
present within the snapshot/group directory. From the absence of the
marker file it may not be inferred that the snapshot/group is present
as regular item however.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 9ce4cb0f8..ec6d97790 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -99,6 +99,13 @@ impl BackupGroup {
         self.full_group_path().exists()
     }
 
+    /// Check if the group is currently marked as trash by checking the presence of the trash
+    /// marker file in the group's directory
+    pub fn is_trash(&self) -> bool {
+        let path = self.full_group_path().join(TRASH_MARKER_FILENAME);
+        path.exists()
+    }
+
     pub fn list_backups(&self) -> Result<Vec<BackupInfo>, Error> {
         let mut list = vec![];
 
@@ -480,6 +487,13 @@ impl BackupDir {
         path.exists()
     }
 
+    /// Check if the snapshot is currently marked as trash by checking the presence of the trash
+    /// marker file in the snapshot's directory
+    pub fn is_trash(&self) -> bool {
+        let path = self.full_path().join(TRASH_MARKER_FILENAME);
+        path.exists()
+    }
+
     pub fn backup_time_to_string(backup_time: i64) -> Result<String, Error> {
         // fixme: can this fail? (avoid unwrap)
         proxmox_time::epoch_to_rfc3339_utc(backup_time)
-- 
2.39.5



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

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

[Christian Ebner]

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

[pbs-devel] [PATCH v3 proxmox-backup 07/20] api: datastore: add trash state filtering for snapshot listing

[Christian Ebner]

Allows to conditionally include, exclude or show trashed backup
snapshots only, the latter being used when displaying the contents of
the trash for given datastore.

Default to exclude snapshots marked as trash from the list.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index a59e39abe..d371a46b0 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -466,6 +466,10 @@ pub async fn delete_snapshot(
                 optional: true,
                 schema: BACKUP_ID_SCHEMA,
             },
+            "trash-state": {
+                type: TrashStateFilter,
+                optional: true,
+            },
         },
     },
     returns: pbs_api_types::ADMIN_DATASTORE_LIST_SNAPSHOTS_RETURN_TYPE,
@@ -481,14 +485,16 @@ pub async fn list_snapshots(
     ns: Option<BackupNamespace>,
     backup_type: Option<BackupType>,
     backup_id: Option<String>,
+    trash_state: Option<TrashStateFilter>,
     _param: Value,
     _info: &ApiMethod,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Vec<SnapshotListItem>, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let trash_state = trash_state.unwrap_or_default();
 
     tokio::task::spawn_blocking(move || unsafe {
-        list_snapshots_blocking(store, ns, backup_type, backup_id, auth_id)
+        list_snapshots_blocking(store, ns, backup_type, backup_id, auth_id, trash_state)
     })
     .await
     .map_err(|err| format_err!("failed to await blocking task: {err}"))?
@@ -501,6 +507,7 @@ unsafe fn list_snapshots_blocking(
     backup_type: Option<BackupType>,
     backup_id: Option<String>,
     auth_id: Authid,
+    trash_state: TrashStateFilter,
 ) -> Result<Vec<SnapshotListItem>, Error> {
     let ns = ns.unwrap_or_default();
 
@@ -627,7 +634,7 @@ unsafe fn list_snapshots_blocking(
             return Ok(snapshots);
         }
 
-        let group_backups = group.list_backups(TrashStateFilter::ExcludeTrash)?;
+        let group_backups = group.list_backups(trash_state)?;
 
         snapshots.extend(
             group_backups
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 08/20] datastore: ignore trashed snapshots for last successful backup

[Christian Ebner]

Exclude trashed snapshots from looking up the last successful backup
of the group, as trashed items are marked for deletion by the next
garbage collection run and must be considered as if not present
anymore.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 53acc5baf..ca0c4bc55 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -175,8 +175,13 @@ impl BackupGroup {
                     return Ok(());
                 }
 
-                let mut manifest_path = PathBuf::from(backup_time);
-                manifest_path.push(MANIFEST_BLOB_NAME.as_ref());
+                let path = PathBuf::from(backup_time);
+                let trash_marker_path = path.join(TRASH_MARKER_FILENAME);
+                if trash_marker_path.exists() {
+                    return Ok(());
+                }
+
+                let manifest_path = path.join(MANIFEST_BLOB_NAME.as_ref());
 
                 use nix::fcntl::{openat, OFlag};
                 match openat(
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 09/20] sync: ignore trashed snapshots/groups when reading from local source

[Christian Ebner]

Trashed snapshots and backup groups should never be synced, so filter
them out when listing the contents to be synced.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/server/sync.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/server/sync.rs b/src/server/sync.rs
index 09814ef0c..a32c914ff 100644
--- a/src/server/sync.rs
+++ b/src/server/sync.rs
@@ -447,6 +447,7 @@ impl SyncSource for LocalSource {
             Some(owner),
         )?
         .filter_map(Result::ok)
+        .filter(|backup_group| !backup_group.is_trash())
         .map(|backup_group| backup_group.group().clone())
         .collect::<Vec<pbs_api_types::BackupGroup>>())
     }
@@ -461,6 +462,7 @@ impl SyncSource for LocalSource {
             .backup_group(namespace.clone(), group.clone())
             .iter_snapshots()?
             .filter_map(Result::ok)
+            .filter(|snapshot| !snapshot.is_trash())
             .map(|snapshot| snapshot.dir().to_owned())
             .collect::<Vec<BackupDir>>())
     }
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 10/20] api: tape: check trash marker when trying to write snapshot

[Christian Ebner]

Since snapshots might be marked as trash, the snapshot directory
can still be present until cleaned up by garbage collection.

Therefore, check for the presence of the trash marker after acquiring
the locked snapshot reader and skip over marked ones.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/tape/backup.rs | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 158905990..0adac96af 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -574,7 +574,13 @@ fn backup_snapshot(
     info!("backup snapshot {snapshot_path:?}");
 
     let snapshot_reader = match snapshot.locked_reader() {
-        Ok(reader) => reader,
+        Ok(reader) => {
+            if snapshot.is_trash() {
+                info!("snapshot {snapshot_path:?} marked as trash, skipping");
+                return Ok(SnapshotBackupResult::Ignored);
+            }
+            reader
+        }
         Err(err) => {
             if !snapshot.full_path().exists() {
                 // we got an error and the dir does not exist,
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 11/20] datastore: clear trashed snapshot dir if re-creation requested

[Christian Ebner]

If a previously trashed snapshot has been requested for re-creation
(e.g. by a sync job in push direction), drop the contents of the
currently trashed snapshot.
The snapshot directory itself is already locked at that point, either
by the old locking mechanism acting on the directory itself or by the
new locking mechanism. Therefore, concurrent operations can be
excluded.

For the call site this acts as if the snapshot directory has been
newly created.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index dc4059789..6f99ff572 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -826,8 +826,9 @@ impl DataStore {
     ) -> Result<(PathBuf, bool, BackupLockGuard), Error> {
         let backup_dir = self.backup_dir(ns.clone(), backup_dir.clone())?;
         let relative_path = backup_dir.relative_path();
+        let full_path = backup_dir.full_path();
 
-        match std::fs::create_dir(backup_dir.full_path()) {
+        match std::fs::create_dir(&full_path) {
             Ok(_) => {
                 let guard = backup_dir.lock().with_context(|| {
                     format!("while creating new locked snapshot '{backup_dir:?}'")
@@ -838,6 +839,28 @@ impl DataStore {
                 let guard = backup_dir
                     .lock()
                     .with_context(|| format!("while creating locked snapshot '{backup_dir:?}'"))?;
+
+                if backup_dir.is_trash() {
+                    info!("clear trashed backup snapshot {full_path:?}");
+                    let dir_entries = std::fs::read_dir(&full_path).context(
+                        "failed to read directory contents during cleanup of trashed snapshot",
+                    )?;
+                    for entry in dir_entries {
+                        let entry = entry.context(
+                            "failed to read directory entry during clenup of trashed snapshot",
+                        )?;
+                        // Only expect regular file entries
+                        std::fs::remove_file(entry.path()).context(
+                            "failed to remove directory entry during clenup of trashed snapshot",
+                        )?;
+                    }
+
+                    // Group already untrashed by `create_locked_backup_group`, no further action
+                    // required.
+
+                    return Ok((relative_path, true, guard));
+                }
+
                 Ok((relative_path, false, guard))
             }
             Err(e) => Err(e.into()),
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 12/20] datastore: recover backup group from trash for new backups

[Christian Ebner]

A whole backup group might have been marked as trashed, including all
of the contained snapshots.

A new backup to the trashed group will only be allowed if owner and
user match, restoring the group. Otherwise, fail and do not allow
backups until the group is either removed from trash or permanently
cleared.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 6f99ff572..1bc096420 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -28,7 +28,7 @@ use pbs_api_types::{
 };
 use pbs_config::BackupLockGuard;
 
-use crate::backup_info::{BackupDir, BackupGroup, BackupInfo, OLD_LOCKING};
+use crate::backup_info::{BackupDir, BackupGroup, BackupInfo, OLD_LOCKING, TRASH_MARKER_FILENAME};
 use crate::chunk_store::ChunkStore;
 use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
 use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
@@ -809,6 +809,16 @@ impl DataStore {
                 let guard = backup_group.lock().with_context(|| {
                     format!("while creating locked backup group '{backup_group:?}'")
                 })?;
+                if backup_group.is_trash() {
+                    let owner = self.get_owner(ns, backup_group.group())?;
+                    check_backup_owner(&owner, auth_id)?;
+                    info!("remove trash marker for backup group {full_path:?}");
+                    let trash_path = full_path.join(TRASH_MARKER_FILENAME);
+                    std::fs::remove_file(trash_path)
+                        .context("failed to remove trash marker file")?;
+                    return Ok((owner, guard));
+                }
+
                 let owner = self.get_owner(ns, backup_group.group())?; // just to be sure
                 Ok((owner, guard))
             }
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 13/20] datastore: garbage collection: clean-up trashed snapshots and groups

[Christian Ebner]

Cleanup trashed items during phase 1 of garbage collection. If
encountered, index files located within trashed snapshots are still
touched.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs |  2 +-
 pbs-datastore/src/datastore.rs   | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index ca0c4bc55..f334600c7 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -277,7 +277,7 @@ impl BackupGroup {
     }
 
     /// Helper function, assumes that no more snapshots are present in the group.
-    fn remove_group_dir(&self) -> Result<(), Error> {
+    pub(crate) 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 {
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 1bc096420..fd6eaadbb 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1254,10 +1254,38 @@ impl DataStore {
                             }
                             processed_index_files += 1;
                         }
+
+                        // Only try to lock trashed snapshots and continue if that is not possible,
+                        // as then most likely this is in the process of being untrashed.
+                        // Check trash state before and after locking to avoid otherwise possible
+                        // races.
+                        if snapshot.backup_dir.is_trash() {
+                            if let Ok(_lock) = snapshot.backup_dir.lock() {
+                                if snapshot.backup_dir.is_trash() {
+                                    snapshot.backup_dir.destroy(true, true)?;
+                                }
+                            } else {
+                                let path = snapshot.backup_dir.full_path();
+                                warn!("failed to lock trashed backup snapshot {path:?}, ignore");
+                            }
+                        }
                     }
 
                     break;
                 }
+                if group.is_trash() {
+                    if let Ok(_lock) = group.lock() {
+                        if group.is_trash() {
+                            if let Err(err) = group.remove_group_dir() {
+                                let path = group.full_group_path();
+                                warn!("failed to remove trashed backup group {path:?} - {err}");
+                            }
+                        } else {
+                            let path = group.full_group_path();
+                            warn!("failed to lock trashed backup group {path:?}");
+                        }
+                    }
+                }
             }
         }
 
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 14/20] client: expose skip trash flags for cli commands

[Christian Ebner]

Allows to explicitly set/clear the `skip-trash` flag when pruning
groups or snapshots via the client cli command.

Set defaults for `skip-trash` to false in order to use the trash.

Further, never add the flag to the api call parameters in the client
if not explicitly set, in order to keep backwards compatibility to
older PBS instances.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs        |  6 ++++--
 proxmox-backup-client/src/group.rs    | 14 +++++++++++++-
 proxmox-backup-client/src/snapshot.rs | 16 ++++++++++++----
 src/api2/admin/datastore.rs           | 11 +++++++++--
 src/api2/backup/environment.rs        |  1 +
 src/server/prune_job.rs               |  4 +++-
 src/server/pull.rs                    | 12 +++++++-----
 7 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index fd6eaadbb..574d6ec26 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -671,10 +671,11 @@ impl DataStore {
         self: &Arc<Self>,
         ns: &BackupNamespace,
         backup_group: &pbs_api_types::BackupGroup,
+        skip_trash: bool,
     ) -> Result<BackupGroupDeleteStats, Error> {
         let backup_group = self.backup_group(ns.clone(), backup_group.clone());
 
-        backup_group.destroy(true)
+        backup_group.destroy(skip_trash)
     }
 
     /// Remove a backup directory including all content
@@ -683,10 +684,11 @@ impl DataStore {
         ns: &BackupNamespace,
         backup_dir: &pbs_api_types::BackupDir,
         force: bool,
+        skip_trash: bool,
     ) -> Result<(), Error> {
         let backup_dir = self.backup_dir(ns.clone(), backup_dir.clone())?;
 
-        backup_dir.destroy(force, true)
+        backup_dir.destroy(force, skip_trash)
     }
 
     /// Returns the time of the last successful backup
diff --git a/proxmox-backup-client/src/group.rs b/proxmox-backup-client/src/group.rs
index 67f26e261..42f8e1e61 100644
--- a/proxmox-backup-client/src/group.rs
+++ b/proxmox-backup-client/src/group.rs
@@ -37,11 +37,20 @@ pub fn group_mgmt_cli() -> CliCommandMap {
                 type: BackupNamespace,
                 optional: true,
             },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                description: "Immediately remove the group, not marking contents as trash.",
+            },
         }
     }
 )]
 /// Forget (remove) backup snapshots.
-async fn forget_group(group: String, mut param: Value) -> Result<(), Error> {
+async fn forget_group(
+    group: String,
+    skip_trash: Option<bool>,
+    mut param: Value,
+) -> Result<(), Error> {
     let backup_group: BackupGroup = group.parse()?;
     let repo = remove_repository_from_value(&mut param)?;
     let client = connect(&repo)?;
@@ -63,6 +72,9 @@ async fn forget_group(group: String, mut param: Value) -> Result<(), Error> {
     )?;
     if confirmation.is_yes() {
         let path = format!("api2/json/admin/datastore/{}/groups", repo.store());
+        if let Some(skip_trash) = skip_trash {
+            api_param["skip-trash"] = Value::from(skip_trash);
+        }
         if let Err(err) = client.delete(&path, Some(api_param)).await {
             // "ENOENT: No such file or directory" is part of the error returned when the group
             // has not been found. The full error contains the full datastore path and we would
diff --git a/proxmox-backup-client/src/snapshot.rs b/proxmox-backup-client/src/snapshot.rs
index f195c23b7..a9b46726a 100644
--- a/proxmox-backup-client/src/snapshot.rs
+++ b/proxmox-backup-client/src/snapshot.rs
@@ -173,11 +173,16 @@ async fn list_snapshot_files(param: Value) -> Result<Value, Error> {
                 type: String,
                 description: "Snapshot path.",
             },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                description: "Immediately remove the snapshot, not marking it as trash.",
+            },
         }
     }
 )]
 /// Forget (remove) backup snapshots.
-async fn forget_snapshots(param: Value) -> Result<(), Error> {
+async fn forget_snapshots(skip_trash: Option<bool>, param: Value) -> Result<(), Error> {
     let repo = extract_repository_from_value(&param)?;
 
     let backup_ns = optional_ns_param(&param)?;
@@ -188,9 +193,12 @@ async fn forget_snapshots(param: Value) -> Result<(), Error> {
 
     let path = format!("api2/json/admin/datastore/{}/snapshots", repo.store());
 
-    client
-        .delete(&path, Some(snapshot_args(&backup_ns, &snapshot)?))
-        .await?;
+    let mut args = snapshot_args(&backup_ns, &snapshot)?;
+    if let Some(skip_trash) = skip_trash {
+        args["skip-trash"] = Value::from(skip_trash);
+    }
+
+    client.delete(&path, Some(args)).await?;
 
     record_repository(&repo);
 
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index d371a46b0..3f68edf24 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -277,7 +277,13 @@ pub fn list_groups(
                 optional: true,
                 default: true,
                 description: "Return error when group cannot be deleted because of protected snapshots",
-            }
+            },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                default: false,
+                description: "Immediately remove the group including all snapshots, not marking it as trash.",
+            },
         },
     },
     returns: {
@@ -295,6 +301,7 @@ pub async fn delete_group(
     ns: Option<BackupNamespace>,
     error_on_protected: bool,
     group: pbs_api_types::BackupGroup,
+    skip_trash: bool,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<BackupGroupDeleteStats, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -312,7 +319,7 @@ pub async fn delete_group(
             &group,
         )?;
 
-        let delete_stats = datastore.remove_backup_group(&ns, &group)?;
+        let delete_stats = datastore.remove_backup_group(&ns, &group, skip_trash)?;
 
         let error_msg = if datastore.old_locking() {
             "could not remove empty groups directories due to old locking mechanism.\n\
diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 3d541b461..b5619eb8c 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -730,6 +730,7 @@ impl BackupEnvironment {
             self.backup_dir.backup_ns(),
             self.backup_dir.as_ref(),
             true,
+            true,
         )?;
 
         Ok(())
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index 24359efc7..3b6e168d3 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -75,7 +75,9 @@ pub fn prune_datastore(
                 info.backup_dir.backup_time_string()
             );
             if !keep && !dry_run {
-                if let Err(err) = datastore.remove_backup_dir(ns, info.backup_dir.as_ref(), false) {
+                if let Err(err) =
+                    datastore.remove_backup_dir(ns, info.backup_dir.as_ref(), false, true)
+                {
                     let path = info.backup_dir.relative_path();
                     warn!("failed to remove dir {path:?}: {err}");
                 }
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 7aeb2bd56..623419884 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -503,6 +503,7 @@ async fn pull_snapshot_from<'a>(
                     snapshot.backup_ns(),
                     snapshot.as_ref(),
                     true,
+                    true,
                 ) {
                     info!("cleanup error - {cleanup_err}");
                 }
@@ -677,7 +678,7 @@ async fn pull_group(
             params
                 .target
                 .store
-                .remove_backup_dir(&target_ns, snapshot.as_ref(), false)?;
+                .remove_backup_dir(&target_ns, snapshot.as_ref(), false, true)?;
             sync_stats.add(SyncStats::from(RemovedVanishedStats {
                 snapshots: 1,
                 groups: 0,
@@ -992,10 +993,11 @@ pub(crate) async fn pull_ns(
                     continue;
                 }
                 info!("delete vanished group '{local_group}'");
-                let delete_stats_result = params
-                    .target
-                    .store
-                    .remove_backup_group(&target_ns, local_group);
+                let delete_stats_result =
+                    params
+                        .target
+                        .store
+                        .remove_backup_group(&target_ns, local_group, false);
 
                 match delete_stats_result {
                     Ok(stats) => {
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 15/20] api: admin: implement endpoints to recover trashed contents

[Christian Ebner]

Implements the api endpoints to recover trashed contents contained
within backup groups or individual snapshots.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 143 +++++++++++++++++++++++++++++++++++-
 1 file changed, 142 insertions(+), 1 deletion(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 3f68edf24..3ea5b19f1 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -51,7 +51,7 @@ use pbs_api_types::{
 };
 use pbs_client::pxar::{create_tar, create_zip};
 use pbs_config::CachedUserInfo;
-use pbs_datastore::backup_info::BackupInfo;
+use pbs_datastore::backup_info::{BackupInfo, TRASH_MARKER_FILENAME};
 use pbs_datastore::cached_chunk_reader::CachedChunkReader;
 use pbs_datastore::catalog::{ArchiveEntry, CatalogReader};
 use pbs_datastore::data_blob::DataBlob;
@@ -2724,6 +2724,139 @@ pub async fn unmount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<V
     Ok(json!(upid))
 }
 
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            group: {
+                type: pbs_api_types::BackupGroup,
+                flatten: true,
+            },
+            ns: {
+                type: BackupNamespace,
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_MODIFY for any \
+            or DATASTORE_BACKUP and being the owner of the group",
+    },
+)]
+/// Recover trashed contents of a backup group.
+pub fn recover_group(
+    store: String,
+    group: pbs_api_types::BackupGroup,
+    ns: Option<BackupNamespace>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let ns = ns.unwrap_or_default();
+    let datastore = check_privs_and_load_store(
+        &store,
+        &ns,
+        &auth_id,
+        PRIV_DATASTORE_MODIFY,
+        PRIV_DATASTORE_BACKUP,
+        Some(Operation::Write),
+        &group,
+    )?;
+
+    let backup_group = datastore.backup_group(ns, group);
+    do_recover_group(&backup_group)?;
+
+    Ok(())
+}
+
+fn do_recover_group(backup_group: &BackupGroup) -> Result<(), Error> {
+    let _exclusive_lock = backup_group
+        .lock()
+        .with_context(|| "while recovering group {backup_group:?}")?;
+    let trashed_snapshots = backup_group.list_backups(TrashStateFilter::OnlyTrash)?;
+    for snapshot in trashed_snapshots {
+        do_recover_snapshot(&snapshot.backup_dir)?;
+    }
+
+    let group_trash_path = backup_group.full_group_path().join(TRASH_MARKER_FILENAME);
+    if let Err(err) = std::fs::remove_file(&group_trash_path) {
+        if err.kind() != std::io::ErrorKind::NotFound {
+            bail!("failed to remove group trash file {group_trash_path:?} - {err}");
+        }
+    }
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            backup_dir: {
+                type: pbs_api_types::BackupDir,
+                flatten: true,
+            },
+            ns: {
+                type: BackupNamespace,
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_MODIFY for any \
+            or DATASTORE_BACKUP and being the owner of the group",
+    },
+)]
+/// Recover trashed contents of a backup snapshot.
+pub fn recover_snapshot(
+    store: String,
+    backup_dir: pbs_api_types::BackupDir,
+    ns: Option<BackupNamespace>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let ns = ns.unwrap_or_default();
+    let datastore = check_privs_and_load_store(
+        &store,
+        &ns,
+        &auth_id,
+        PRIV_DATASTORE_MODIFY,
+        PRIV_DATASTORE_BACKUP,
+        Some(Operation::Write),
+        &backup_dir.group,
+    )?;
+
+    let backup_group = datastore.backup_group(ns.clone(), backup_dir.group.clone());
+    let snapshot = datastore.backup_dir(ns, backup_dir)?;
+    let _exclusive_group_lock = backup_group
+        .lock()
+        .with_context(|| "while recovering snapshot {snapshot_dir:?}")?;
+
+    do_recover_snapshot(&snapshot)?;
+
+    let group_trash_path = backup_group.full_group_path().join(TRASH_MARKER_FILENAME);
+    if let Err(err) = std::fs::remove_file(&group_trash_path) {
+        if err.kind() != std::io::ErrorKind::NotFound {
+            bail!("failed to remove group trash file {group_trash_path:?} - {err}");
+        }
+    }
+
+    Ok(())
+}
+
+fn do_recover_snapshot(snapshot_dir: &BackupDir) -> Result<(), Error> {
+    let _exclusive_lock = snapshot_dir
+        .lock()
+        .with_context(|| "while recovering snapshot {snapshot_dir:?}")?;
+    let trash_path = snapshot_dir.full_path().join(TRASH_MARKER_FILENAME);
+    if let Err(err) = std::fs::remove_file(&trash_path) {
+        if err.kind() != std::io::ErrorKind::NotFound {
+            bail!("failed to remove trash file {trash_path:?} - {err}");
+        }
+    }
+    Ok(())
+}
+
 #[sortable]
 const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
     (
@@ -2789,6 +2922,14 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
         "pxar-file-download",
         &Router::new().download(&API_METHOD_PXAR_FILE_DOWNLOAD),
     ),
+    (
+        "recover-group",
+        &Router::new().put(&API_METHOD_RECOVER_GROUP),
+    ),
+    (
+        "recover-snapshot",
+        &Router::new().put(&API_METHOD_RECOVER_SNAPSHOT),
+    ),
     ("rrd", &Router::new().get(&API_METHOD_GET_RRD_STATS)),
     (
         "snapshots",
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 16/20] api: admin: move backup group list generation into helper

[Christian Ebner]

Move the logic to generate a backup group list given backup type or
backup_id into a helper function. This allows to reuse the same logic
for generating the list of groups for which to clear trashed items.

No functional change intended.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 53 ++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 3ea5b19f1..bc2d51612 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -507,6 +507,37 @@ pub async fn list_snapshots(
     .map_err(|err| format_err!("failed to await blocking task: {err}"))?
 }
 
+fn groups_by_type_or_id(
+    datastore: Arc<DataStore>,
+    ns: &BackupNamespace,
+    backup_type: Option<BackupType>,
+    backup_id: Option<String>,
+) -> Result<Vec<BackupGroup>, Error> {
+    // FIXME: filter also owner before collecting, for doing that nicely the owner should move into
+    // backup group and provide an error free (Err -> None) accessor
+    match (backup_type, backup_id) {
+        (Some(backup_type), Some(backup_id)) => Ok(vec![datastore.backup_group_from_parts(
+            ns.clone(),
+            backup_type,
+            backup_id,
+        )]),
+        // FIXME: Recursion
+        (Some(backup_type), None) => Ok(datastore
+            .iter_backup_type_ok(ns.clone(), backup_type)?
+            .collect()),
+        // FIXME: Recursion
+        (None, Some(backup_id)) => Ok(BackupType::iter()
+            .filter_map(|backup_type| {
+                let group =
+                    datastore.backup_group_from_parts(ns.clone(), backup_type, backup_id.clone());
+                group.exists().then_some(group)
+            })
+            .collect()),
+        // FIXME: Recursion
+        (None, None) => datastore.list_backup_groups(ns.clone()),
+    }
+}
+
 /// This must not run in a main worker thread as it potentially does tons of I/O.
 unsafe fn list_snapshots_blocking(
     store: String,
@@ -528,27 +559,7 @@ unsafe fn list_snapshots_blocking(
 
     let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
 
-    // FIXME: filter also owner before collecting, for doing that nicely the owner should move into
-    // backup group and provide an error free (Err -> None) accessor
-    let groups = match (backup_type, backup_id) {
-        (Some(backup_type), Some(backup_id)) => {
-            vec![datastore.backup_group_from_parts(ns.clone(), backup_type, backup_id)]
-        }
-        // FIXME: Recursion
-        (Some(backup_type), None) => datastore
-            .iter_backup_type_ok(ns.clone(), backup_type)?
-            .collect(),
-        // FIXME: Recursion
-        (None, Some(backup_id)) => BackupType::iter()
-            .filter_map(|backup_type| {
-                let group =
-                    datastore.backup_group_from_parts(ns.clone(), backup_type, backup_id.clone());
-                group.exists().then_some(group)
-            })
-            .collect(),
-        // FIXME: Recursion
-        (None, None) => datastore.list_backup_groups(ns.clone())?,
-    };
+    let groups = groups_by_type_or_id(datastore, &ns, backup_type, backup_id)?;
 
     let info_to_snapshot_list_item = |group: &BackupGroup, owner, info: BackupInfo| {
         let backup = pbs_api_types::BackupDir {
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 17/20] api: admin: add endpoint to clear trashed items from group

[Christian Ebner]

Allows to remove only the trashed snapshot items of a backup group,
including the backup group itself if all the contents have been
cleared. Instead of using the backup group delete stats to determine
whether the group directory should be cleaned up or not, use a local
variable instead, as the removed trash is otherwise not correctly
accounted for.

This allows to manually clear trashed groups from the UI for
convenience.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 14 ++++++-
 pbs-datastore/src/datastore.rs   | 17 +++++++-
 src/api2/admin/datastore.rs      | 70 ++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index f334600c7..85d856888 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -242,7 +242,11 @@ impl BackupGroup {
     ///
     /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots
     /// and number of protected snaphsots, which therefore were not removed.
-    pub fn destroy(&self, skip_trash: bool) -> Result<BackupGroupDeleteStats, Error> {
+    pub fn destroy(
+        &self,
+        skip_trash: bool,
+        trash_only: bool,
+    ) -> Result<BackupGroupDeleteStats, Error> {
         let _guard = self
             .lock()
             .with_context(|| format!("while destroying group '{self:?}'"))?;
@@ -250,10 +254,16 @@ impl BackupGroup {
 
         log::info!("removing backup group {:?}", path);
         let mut delete_stats = BackupGroupDeleteStats::default();
+        let mut cleanup_group_dir = true;
         for snap in self.iter_snapshots()? {
             let snap = snap?;
             if snap.is_protected() {
                 delete_stats.increment_protected_snapshots();
+                cleanup_group_dir = false;
+                continue;
+            }
+            if trash_only && !snap.is_trash() {
+                cleanup_group_dir = false;
                 continue;
             }
             snap.destroy(false, skip_trash)?;
@@ -262,7 +272,7 @@ impl BackupGroup {
 
         // 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 {
+        if cleanup_group_dir && !*OLD_LOCKING {
             if skip_trash {
                 self.remove_group_dir()?;
             } else {
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 574d6ec26..fde0096bf 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(true)?;
+            let delete_stats = group?.destroy(true, false)?;
             stats.add(&delete_stats);
             removed_all_groups = removed_all_groups && delete_stats.all_removed();
         }
@@ -675,7 +675,20 @@ impl DataStore {
     ) -> Result<BackupGroupDeleteStats, Error> {
         let backup_group = self.backup_group(ns.clone(), backup_group.clone());
 
-        backup_group.destroy(skip_trash)
+        backup_group.destroy(skip_trash, false)
+    }
+
+    /// Remove snapshots marked as trash from a backup group, including the group if it is empty
+    /// afterwards.
+    ///
+    /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots.
+    pub fn clear_backup_group(
+        self: &Arc<Self>,
+        ns: &BackupNamespace,
+        backup_group: &pbs_api_types::BackupGroup,
+    ) -> Result<BackupGroupDeleteStats, Error> {
+        let backup_group = self.backup_group(ns.clone(), backup_group.clone());
+        backup_group.destroy(true, true)
     }
 
     /// Remove a backup directory including all content
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index bc2d51612..f97aeb5cb 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2868,6 +2868,72 @@ fn do_recover_snapshot(snapshot_dir: &BackupDir) -> Result<(), Error> {
     Ok(())
 }
 
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            ns: {
+                type: BackupNamespace,
+                optional: true,
+            },
+            "backup-type": {
+                optional: true,
+                type: BackupType,
+            },
+            "backup-id": {
+                optional: true,
+                schema: BACKUP_ID_SCHEMA,
+            },
+        },
+    },
+    returns: {
+        type: BackupGroupDeleteStats,
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_MODIFY for any \
+            or DATASTORE_PRUNE and being the owner of the group",
+    },
+)]
+/// Clear trash items in a namespace or backup group including the group itself it is marked as trash.
+pub async fn clear_trash(
+    store: String,
+    ns: Option<BackupNamespace>,
+    backup_type: Option<BackupType>,
+    backup_id: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<BackupGroupDeleteStats, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
+    tokio::task::spawn_blocking(move || {
+        let ns = ns.unwrap_or_default();
+        let limited = check_ns_privs_full(
+            &store,
+            &ns,
+            &auth_id,
+            PRIV_DATASTORE_MODIFY,
+            PRIV_DATASTORE_PRUNE,
+        )?;
+        let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
+
+        let groups = groups_by_type_or_id(datastore.clone(), &ns, backup_type, backup_id)?;
+        let mut delete_stats = BackupGroupDeleteStats::default();
+        for group in groups {
+            if limited {
+                let owner = datastore.get_owner(&ns, group.group())?;
+                if check_backup_owner(&owner, &auth_id).is_err() {
+                    continue;
+                }
+            }
+            let stats = datastore.clear_backup_group(&ns, group.group())?;
+            delete_stats.add(&stats);
+        }
+
+        Ok(delete_stats)
+    })
+    .await?
+}
+
 #[sortable]
 const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
     (
@@ -2879,6 +2945,10 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
         "change-owner",
         &Router::new().post(&API_METHOD_SET_BACKUP_OWNER),
     ),
+    (
+        "clear-trash",
+        &Router::new().delete(&API_METHOD_CLEAR_TRASH),
+    ),
     (
         "download",
         &Router::new().download(&API_METHOD_DOWNLOAD_FILE),
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 18/20] ui: add recover for trashed items tab to datastore panel

[Christian Ebner]

Display a dedicated recover trashed tab which allows to inspect and
recover trashed items.

This is based on the pre-existing contents tab but  drops any actions
which make no sense for the given context, such as editing of group
ownership, notes, verification, ecc.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/Makefile                    |   1 +
 www/Utils.js                    |   1 +
 www/datastore/Panel.js          |   8 +
 www/datastore/RecoverTrashed.js | 805 ++++++++++++++++++++++++++++++++
 4 files changed, 815 insertions(+)
 create mode 100644 www/datastore/RecoverTrashed.js

diff --git a/www/Makefile b/www/Makefile
index 44c5fa133..aa8955460 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -115,6 +115,7 @@ JSSRC=							\
 	datastore/Panel.js				\
 	datastore/DataStoreListSummary.js		\
 	datastore/DataStoreList.js			\
+	datastore/RecoverTrashed.js			\
 	ServerStatus.js					\
 	ServerAdministration.js				\
 	NodeNotes.js				        \
diff --git a/www/Utils.js b/www/Utils.js
index 9dcde6941..37523deb0 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -404,6 +404,7 @@ Ext.define('PBS.Utils', {
 	    backup: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Backup')),
 	    'barcode-label-media': [gettext('Drive'), gettext('Barcode-Label Media')],
 	    'catalog-media': [gettext('Drive'), gettext('Catalog Media')],
+	    "clear-trash": [gettext('Trash'), gettext('Clear Trashed Items of Group')],
 	    'delete-datastore': [gettext('Datastore'), gettext('Remove Datastore')],
 	    'delete-namespace': [gettext('Namespace'), gettext('Remove Namespace')],
 	    dircreate: [gettext('Directory Storage'), gettext('Create')],
diff --git a/www/datastore/Panel.js b/www/datastore/Panel.js
index ad9fc10fe..386b62284 100644
--- a/www/datastore/Panel.js
+++ b/www/datastore/Panel.js
@@ -99,6 +99,14 @@ Ext.define('PBS.DataStorePanel', {
 		datastore: '{datastore}',
 	    },
 	},
+	{
+	    xtype: 'pbsDataStoreRecoverTrashed',
+	    itemId: 'trashed',
+	    iconCls: 'fa fa-rotate-left',
+	    cbind: {
+		datastore: '{datastore}',
+	    },
+	},
     ],
 
     initComponent: function() {
diff --git a/www/datastore/RecoverTrashed.js b/www/datastore/RecoverTrashed.js
new file mode 100644
index 000000000..b730f6fd5
--- /dev/null
+++ b/www/datastore/RecoverTrashed.js
@@ -0,0 +1,805 @@
+Ext.define('PBS.DataStoreRecoverTrashed', {
+    extend: 'Ext.tree.Panel',
+    alias: 'widget.pbsDataStoreRecoverTrashed',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    rootVisible: false,
+
+    title: gettext('Recover Trashed'),
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	init: function(view) {
+	    if (!view.datastore) {
+		throw "no datastore specified";
+	    }
+
+	    this.store = Ext.create('Ext.data.Store', {
+		model: 'pbs-data-store-snapshots',
+		groupField: 'backup-group',
+	    });
+	    this.store.on('load', this.onLoad, this);
+
+	    view.getStore().setSorters([
+		'sortWeight',
+		'text',
+		'backup-time',
+	    ]);
+	},
+
+	control: {
+	    '#': { // view
+		rowdblclick: 'rowDoubleClicked',
+	    },
+	    'pbsNamespaceSelector': {
+		change: 'nsChange',
+	    },
+	},
+
+	rowDoubleClicked: function(table, rec, el, rowId, ev) {
+	    if (rec?.data?.ty === 'ns' && !rec.data.root) {
+		this.nsChange(null, rec.data.ns);
+	    }
+	},
+
+	nsChange: function(field, value) {
+	    let view = this.getView();
+	    if (field === null) {
+		field = view.down('pbsNamespaceSelector');
+		field.setValue(value);
+		return;
+	    }
+	    view.namespace = value;
+	    this.reload();
+	},
+
+	reload: function() {
+	    let view = this.getView();
+
+	    if (!view.store || !this.store) {
+		console.warn('cannot reload, no store(s)');
+		return;
+	    }
+
+	    let url = `/api2/json/admin/datastore/${view.datastore}/snapshots?trash-state=only-trash`;
+	    if (view.namespace && view.namespace !== '') {
+		url += `&ns=${encodeURIComponent(view.namespace)}`;
+	    }
+	    this.store.setProxy({
+		type: 'proxmox',
+		timeout: 5*60*1000,
+		url: url,
+	    });
+
+	    this.store.load();
+	},
+
+	getRecordGroups: function(records) {
+	    let groups = {};
+
+	    for (const item of records) {
+		var btype = item.data["backup-type"];
+		let group = btype + "/" + item.data["backup-id"];
+
+		if (groups[group] !== undefined) {
+		    continue;
+		}
+
+		var cls = PBS.Utils.get_type_icon_cls(btype);
+		if (cls === "") {
+		    console.warn(`got unknown backup-type '${btype}'`);
+		    continue;
+		}
+
+		groups[group] = {
+		    text: group,
+		    leaf: false,
+		    iconCls: "fa " + cls,
+		    expanded: false,
+		    backup_type: item.data["backup-type"],
+		    backup_id: item.data["backup-id"],
+		    children: [],
+		};
+	    }
+
+	    return groups;
+	},
+
+	loadNamespaceFromSameLevel: async function() {
+	    let view = this.getView();
+	    try {
+		let url = `/api2/extjs/admin/datastore/${view.datastore}/namespace?max-depth=1`;
+		if (view.namespace && view.namespace !== '') {
+		    url += `&parent=${encodeURIComponent(view.namespace)}`;
+		}
+		let { result: { data: ns } } = await Proxmox.Async.api2({ url });
+		return ns;
+	    } catch (err) {
+		console.debug(err);
+	    }
+	    return [];
+	},
+
+	onLoad: async function(store, records, success, operation) {
+	    let me = this;
+	    let view = this.getView();
+
+	    let namespaces = await me.loadNamespaceFromSameLevel();
+
+	    if (!success) {
+		if (namespaces.length === 0) {
+		    let error = Proxmox.Utils.getResponseErrorMessage(operation.getError());
+		    Proxmox.Utils.setErrorMask(view.down('treeview'), error);
+		    return;
+		} else {
+		    records = [];
+		}
+	    } else {
+		Proxmox.Utils.setErrorMask(view.down('treeview'));
+	    }
+
+	    let groups = this.getRecordGroups(records);
+
+	    let selected;
+	    let expanded = {};
+
+	    view.getSelection().some(function(item) {
+		let id = item.data.text;
+		if (item.data.leaf) {
+		    id = item.parentNode.data.text + id;
+		}
+		selected = id;
+		return true;
+	    });
+
+	    view.getRootNode().cascadeBy({
+		before: item => {
+		    if (item.isExpanded() && !item.data.leaf) {
+			let id = item.data.text;
+			expanded[id] = true;
+			return true;
+		    }
+		    return false;
+		},
+		after: Ext.emptyFn,
+	    });
+
+	    for (const item of records) {
+		let group = item.data["backup-type"] + "/" + item.data["backup-id"];
+		let children = groups[group].children;
+
+		let data = item.data;
+
+		data.text = group + '/' + PBS.Utils.render_datetime_utc(data["backup-time"]);
+		data.leaf = false;
+		data.cls = 'no-leaf-icons';
+		data.matchesFilter = true;
+		data.ty = 'dir';
+
+		data.expanded = !!expanded[data.text];
+
+		data.children = [];
+		for (const file of data.files) {
+		    file.text = file.filename;
+		    file['crypt-mode'] = PBS.Utils.cryptmap.indexOf(file['crypt-mode']);
+		    file.fingerprint = data.fingerprint;
+		    file.leaf = true;
+		    file.matchesFilter = true;
+		    file.ty = 'file';
+
+		    data.children.push(file);
+		}
+
+		children.push(data);
+	    }
+
+	    let children = [];
+	    for (const [name, group] of Object.entries(groups)) {
+		let last_backup = 0;
+		let crypt = {
+		    none: 0,
+		    mixed: 0,
+		    'sign-only': 0,
+		    encrypt: 0,
+		};
+		for (let item of group.children) {
+		    crypt[PBS.Utils.cryptmap[item['crypt-mode']]]++;
+		    if (item["backup-time"] > last_backup && item.size !== null) {
+			last_backup = item["backup-time"];
+			group["backup-time"] = last_backup;
+			group["last-comment"] = item.comment;
+			group.files = item.files;
+			group.size = item.size;
+			group.owner = item.owner;
+		    }
+		}
+		group.count = group.children.length;
+		group.matchesFilter = true;
+		crypt.count = group.count;
+		group['crypt-mode'] = PBS.Utils.calculateCryptMode(crypt);
+		group.expanded = !!expanded[name];
+		group.sortWeight = 0;
+		group.ty = 'group';
+		children.push(group);
+	    }
+
+	    for (const item of namespaces) {
+		if (item.ns === view.namespace || (!view.namespace && item.ns === '')) {
+		    continue;
+		}
+		children.push({
+		    text: item.ns,
+		    iconCls: 'fa fa-object-group',
+		    expanded: true,
+		    expandable: false,
+		    ns: (view.namespaces ?? '') !== '' ? `/${item.ns}` : item.ns,
+		    ty: 'ns',
+		    sortWeight: 10,
+		    leaf: true,
+		});
+	    }
+
+	    let isRootNS = !view.namespace || view.namespace === '';
+	    let rootText = isRootNS
+		? gettext('Root Namespace')
+		: Ext.String.format(gettext("Namespace '{0}'"), view.namespace);
+
+	    let topNodes = [];
+	    if (!isRootNS) {
+		let parentNS = view.namespace.split('/').slice(0, -1).join('/');
+		topNodes.push({
+		    text: `.. (${parentNS === '' ? gettext('Root') : parentNS})`,
+		    iconCls: 'fa fa-level-up',
+		    ty: 'ns',
+		    ns: parentNS,
+		    sortWeight: -10,
+		    leaf: true,
+		});
+	    }
+	    topNodes.push({
+		text: rootText,
+		iconCls: "fa fa-" + (isRootNS ? 'database' : 'object-group'),
+		expanded: true,
+		expandable: false,
+		sortWeight: -5,
+		root: true, // fake root
+		isRootNS,
+		ty: 'ns',
+		children: children,
+	    });
+
+	    view.setRootNode({
+		expanded: true,
+		children: topNodes,
+	    });
+
+	    if (!children.length) {
+		view.setEmptyText(Ext.String.format(
+		    gettext('No accessible snapshots found in namespace {0}'),
+		    view.namespace && view.namespace !== '' ? `'${view.namespace}'`: gettext('Root'),
+		));
+	    }
+
+	    if (selected !== undefined) {
+		let selection = view.getRootNode().findChildBy(function(item) {
+		    let id = item.data.text;
+		    if (item.data.leaf) {
+			id = item.parentNode.data.text + id;
+		    }
+		    return selected === id;
+		}, undefined, true);
+		if (selection) {
+		    view.setSelection(selection);
+		    view.getView().focusRow(selection);
+		}
+	    }
+
+	    Proxmox.Utils.setErrorMask(view, false);
+	    if (view.getStore().getFilters().length > 0) {
+		let searchBox = me.lookup("searchbox");
+		let searchvalue = searchBox.getValue();
+		me.search(searchBox, searchvalue);
+	    }
+	},
+
+	recoverGroup: function(data) {
+	    let me = this;
+	    let view = me.getView();
+
+	    let params = {
+		"backup-type": data.backup_type,
+		"backup-id": data.backup_id,
+	    };
+	    if (view.namespace && view.namespace !== '') {
+		params.ns = view.namespace;
+	    }
+
+	    Ext.Msg.show({
+		title: gettext('Confirm'),
+		icon: Ext.Msg.WARNING,
+		message: Ext.String.format(gettext('Are you sure you want to recover group {0}'), `'${data.text}'`),
+		buttons: Ext.Msg.YESNO,
+		defaultFocus: 'yes',
+		callback: function(btn) {
+		    if (btn !== 'yes') {
+		        return;
+		    }
+
+		    Proxmox.Utils.API2Request({
+			url: `/admin/datastore/${view.datastore}/recover-group`,
+			params,
+			method: 'PUT',
+			waitMsgTarget: view,
+			failure: function(response, opts) {
+			    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+			},
+			callback: me.reload.bind(me),
+		    });
+		},
+	    });
+	},
+
+	forgetGroup: function(data) {
+	    let me = this;
+	    let view = me.getView();
+
+	    let params = {
+		"backup-type": data.backup_type,
+		"backup-id": data.backup_id,
+	    };
+	    if (view.namespace && view.namespace !== '') {
+		params.ns = view.namespace;
+	    }
+
+	    Ext.create('Proxmox.window.SafeDestroy', {
+		url: `/admin/datastore/${view.datastore}/clear-trash`,
+		params,
+		item: {
+		    id: data.text,
+		},
+		autoShow: true,
+		taskName: 'clear-trash',
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    });
+	},
+
+	recoverSnapshot: function(data) {
+	    let me = this;
+	    let view = me.getView();
+
+	    Ext.Msg.show({
+		title: gettext('Confirm'),
+		icon: Ext.Msg.WARNING,
+		message: Ext.String.format(gettext('Are you sure you want to recover snapshot {0}'), `'${data.text}'`),
+		buttons: Ext.Msg.YESNO,
+		defaultFocus: 'yes',
+		callback: function(btn) {
+		    if (btn !== 'yes') {
+		        return;
+		    }
+		    let params = {
+			"backup-type": data["backup-type"],
+			"backup-id": data["backup-id"],
+			"backup-time": (data['backup-time'].getTime()/1000).toFixed(0),
+		    };
+		    if (view.namespace && view.namespace !== '') {
+			params.ns = view.namespace;
+		    }
+
+		    //TODO adapt to recover api endpoint
+		    Proxmox.Utils.API2Request({
+			url: `/admin/datastore/${view.datastore}/recover-snapshot`,
+			params,
+			method: 'PUT',
+			waitMsgTarget: view,
+			failure: function(response, opts) {
+			    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+			},
+			callback: me.reload.bind(me),
+		    });
+		},
+	    });
+	},
+
+	forgetSnapshot: function(data) {
+	    let me = this;
+	    let view = me.getView();
+
+	    Ext.Msg.show({
+		title: gettext('Confirm'),
+		icon: Ext.Msg.WARNING,
+		message: Ext.String.format(gettext('Are you sure you want to remove snapshot {0}'), `'${data.text}'`),
+		buttons: Ext.Msg.YESNO,
+		defaultFocus: 'no',
+		callback: function(btn) {
+		    if (btn !== 'yes') {
+		        return;
+		    }
+		    let params = {
+			"backup-type": data["backup-type"],
+			"backup-id": data["backup-id"],
+			"backup-time": (data['backup-time'].getTime()/1000).toFixed(0),
+			"skip-trash": true,
+		    };
+		    if (view.namespace && view.namespace !== '') {
+			params.ns = view.namespace;
+		    }
+
+		    Proxmox.Utils.API2Request({
+			url: `/admin/datastore/${view.datastore}/snapshots`,
+			params,
+			method: 'DELETE',
+			waitMsgTarget: view,
+			failure: function(response, opts) {
+			    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+			},
+			callback: me.reload.bind(me),
+		    });
+		},
+	    });
+	},
+
+	onRecover: function(table, rI, cI, item, e, { data }) {
+	    let me = this;
+	    let view = this.getView();
+	    if ((data.ty !== 'group' && data.ty !== 'dir') || !view.datastore) {
+		return;
+	    }
+
+	    if (data.ty === 'dir') {
+		me.recoverSnapshot(data);
+	    } else {
+		me.recoverGroup(data);
+	    }
+	},
+
+	onForget: function(table, rI, cI, item, e, { data }) {
+	    let me = this;
+	    let view = this.getView();
+	    if ((data.ty !== 'group' && data.ty !== 'dir') || !view.datastore) {
+		return;
+	    }
+
+	    if (data.ty === 'dir') {
+		me.forgetSnapshot(data);
+	    } else {
+		me.forgetGroup(data);
+	    }
+	},
+
+	// opens a namespace browser
+	openBrowser: function(tv, rI, Ci, item, e, rec) {
+	    let me = this;
+	    if (rec.data.ty === 'ns') {
+		me.nsChange(null, rec.data.ns);
+	    }
+	},
+
+	filter: function(item, value) {
+	    if (item.data.text.indexOf(value) !== -1) {
+		return true;
+	    }
+
+	    if (item.data.owner && item.data.owner.indexOf(value) !== -1) {
+		return true;
+	    }
+
+	    return false;
+	},
+
+	search: function(tf, value) {
+	    let me = this;
+	    let view = me.getView();
+	    let store = view.getStore();
+	    if (!value && value !== 0) {
+		store.clearFilter();
+		// only collapse the children below our toplevel namespace "root"
+		store.getRoot().lastChild.collapseChildren(true);
+		tf.triggers.clear.setVisible(false);
+		return;
+	    }
+	    tf.triggers.clear.setVisible(true);
+	    if (value.length < 2) return;
+	    Proxmox.Utils.setErrorMask(view, true);
+	    // we do it a little bit later for the error mask to work
+	    setTimeout(function() {
+		store.clearFilter();
+		store.getRoot().collapseChildren(true);
+
+		store.beginUpdate();
+		store.getRoot().cascadeBy({
+		    before: function(item) {
+			if (me.filter(item, value)) {
+			    item.set('matchesFilter', true);
+			    if (item.parentNode && item.parentNode.id !== 'root') {
+				item.parentNode.childmatches = true;
+			    }
+			    return false;
+			}
+			return true;
+		    },
+		    after: function(item) {
+			if (me.filter(item, value) || item.id === 'root' || item.childmatches) {
+			    item.set('matchesFilter', true);
+			    if (item.parentNode && item.parentNode.id !== 'root') {
+				item.parentNode.childmatches = true;
+			    }
+			    if (item.childmatches) {
+				item.expand();
+			    }
+			} else {
+			    item.set('matchesFilter', false);
+			}
+			delete item.childmatches;
+		    },
+		});
+		store.endUpdate();
+
+		store.filter((item) => !!item.get('matchesFilter'));
+		Proxmox.Utils.setErrorMask(view, false);
+	    }, 10);
+	},
+    },
+
+    listeners: {
+	activate: function() {
+	    let me = this;
+	    me.getController().reload();
+	},
+	itemcontextmenu: function(panel, record, item, index, event) {
+	    event.stopEvent();
+	    let title;
+	    let view = panel.up('pbsDataStoreRecoverTrashed');
+	    let controller = view.getController();
+	    let createControllerCallback = function(name) {
+		return function() {
+		    controller[name](view, undefined, undefined, undefined, undefined, record);
+		};
+	    };
+	    if (record.data.ty === 'group') {
+		title = gettext('Group');
+	    } else if (record.data.ty === 'dir') {
+		title = gettext('Snapshot');
+	    }
+	    if (title) {
+		let menu = Ext.create('PBS.datastore.RecoverTrashedContextMenu', {
+		    title: title,
+		    onRecover: createControllerCallback('onRecover'),
+		    onForget: createControllerCallback('onForget'),
+		});
+		menu.showAt(event.getXY());
+	    }
+	},
+    },
+
+    columns: [
+	{
+	    xtype: 'treecolumn',
+	    header: gettext("Backup Group"),
+	    dataIndex: 'text',
+	    renderer: (value, meta, record) => {
+		if (record.data.protected) {
+		    return `${value} (${gettext('protected')})`;
+		}
+		return value;
+	    },
+	    flex: 1,
+	},
+	{
+	    text: gettext('Comment'),
+	    dataIndex: 'comment',
+	    flex: 1,
+	    renderer: (v, meta, record) => {
+		let data = record.data;
+		if (!data || data.leaf || data.root) {
+		    return '';
+		}
+
+		let additionalClasses = "";
+		if (!v) {
+		    if (!data.expanded) {
+			v = data['last-comment'] ?? '';
+			additionalClasses = 'pmx-opacity-75';
+		    } else {
+			v = '';
+		    }
+		}
+		v = Ext.String.htmlEncode(v);
+		return `<span class="snapshot-comment-column ${additionalClasses}">${v}</span>`;
+	    },
+	},
+	{
+	    header: gettext('Actions'),
+	    xtype: 'actioncolumn',
+	    dataIndex: 'text',
+	    width: 100,
+	    items: [
+		{
+		    handler: 'onRecover',
+		    getTip: (v, m, { data }) => {
+			let tip = '{0}';
+			if (data.ty === 'dir') {
+			    tip = gettext("Recover trashed snapshot '{0}'");
+			} else if (data.ty === 'group') {
+			    tip = gettext("Recover trashed items of group '{0}'");
+			}
+			return Ext.String.format(tip, v);
+		    },
+		    getClass: (v, m, { data }) =>
+			data.ty === 'group' || data.ty === 'dir'
+		        ? 'fa fa-rotate-left'
+		        : 'pmx-hidden',
+		    isActionDisabled: (v, r, c, i, { data }) => false,
+		},
+		'->',
+		{
+		    handler: 'onForget',
+		    getTip: (v, m, { data }) => {
+			let tip = '{0}';
+			if (data.ty === 'dir') {
+			    tip = gettext("Permanently forget trashed snapshot '{0}'");
+			} else if (data.ty === 'group') {
+			    tip = gettext("Permanently forget trashed items of group '{0}'");
+			}
+			return Ext.String.format(tip, v);
+		    },
+		    getClass: (v, m, { data }) =>
+		       data.ty === 'group' || data.ty === 'dir'
+		        ? 'fa critical fa-trash-o'
+		        : 'pmx-hidden',
+		    isActionDisabled: (v, r, c, i, { data }) => false,
+		},
+		{
+		    handler: 'openBrowser',
+		    tooltip: gettext('Browse'),
+		    getClass: (v, m, { data }) =>
+			data.ty === 'ns' && !data.root
+			? 'fa fa-folder-open-o'
+			: 'pmx-hidden',
+		    isActionDisabled: (v, r, c, i, { data }) => data.ty !== 'ns',
+		},
+	    ],
+	},
+	{
+	    xtype: 'datecolumn',
+	    header: gettext('Backup Time'),
+	    sortable: true,
+	    dataIndex: 'backup-time',
+	    format: 'Y-m-d H:i:s',
+	    width: 150,
+	},
+	{
+	    header: gettext("Size"),
+	    sortable: true,
+	    dataIndex: 'size',
+	    renderer: (v, meta, { data }) => {
+		if ((data.text === 'client.log.blob' && v === undefined) || (data.ty !== 'dir' && data.ty !== 'file')) {
+		    return '';
+		}
+		if (v === undefined || v === null) {
+		    meta.tdCls = "x-grid-row-loading";
+		    return '';
+		}
+		return Proxmox.Utils.format_size(v);
+	    },
+	},
+	{
+	    xtype: 'numbercolumn',
+	    format: '0',
+	    header: gettext("Count"),
+	    sortable: true,
+	    width: 75,
+	    align: 'right',
+	    dataIndex: 'count',
+	},
+	{
+	    header: gettext("Owner"),
+	    sortable: true,
+	    dataIndex: 'owner',
+	},
+	{
+	    header: gettext('Encrypted'),
+	    dataIndex: 'crypt-mode',
+	    renderer: (v, meta, record) => {
+		if (record.data.size === undefined || record.data.size === null) {
+		    return '';
+		}
+		if (v === -1) {
+		    return '';
+		}
+		let iconCls = PBS.Utils.cryptIconCls[v] || '';
+		let iconTxt = "";
+		if (iconCls) {
+		    iconTxt = `<i class="fa fa-fw fa-${iconCls}"></i> `;
+		}
+		let tip;
+		if (v !== PBS.Utils.cryptmap.indexOf('none') && record.data.fingerprint !== undefined) {
+		    tip = "Key: " + PBS.Utils.renderKeyID(record.data.fingerprint);
+		}
+		let txt = (iconTxt + PBS.Utils.cryptText[v]) || Proxmox.Utils.unknownText;
+		if (record.data.ty === 'group' || tip === undefined) {
+		    return txt;
+		} else {
+		    return `<span data-qtip="${tip}">${txt}</span>`;
+		}
+	    },
+	},
+    ],
+
+    tbar: [
+	{
+	    text: gettext('Reload'),
+	    iconCls: 'fa fa-refresh',
+	    handler: 'reload',
+	},
+	'->',
+	{
+	    xtype: 'tbtext',
+	    html: gettext('Namespace') + ':',
+	},
+	{
+	    xtype: 'pbsNamespaceSelector',
+	    width: 200,
+	    cbind: {
+		datastore: '{datastore}',
+	    },
+	},
+	'-',
+	{
+	    xtype: 'tbtext',
+	    html: gettext('Search'),
+	},
+	{
+	    xtype: 'textfield',
+	    reference: 'searchbox',
+	    emptyText: gettext('group, date or owner'),
+	    triggers: {
+		clear: {
+		    cls: 'pmx-clear-trigger',
+		    weight: -1,
+		    hidden: true,
+		    handler: function() {
+			this.triggers.clear.setVisible(false);
+			this.setValue('');
+		    },
+		},
+	    },
+	    listeners: {
+		change: {
+		    fn: 'search',
+		    buffer: 500,
+		},
+	    },
+	},
+    ],
+});
+
+Ext.define('PBS.datastore.RecoverTrashedContextMenu', {
+    extend: 'Ext.menu.Menu',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    onRecover: undefined,
+    onForget: undefined,
+
+    items: [
+	{
+	    text: gettext('Recover'),
+	    iconCls: 'fa fa-rotate-left',
+	    handler: function() { this.up('menu').onRecover(); },
+	    cbind: {
+		hidden: '{!onRecover}',
+	    },
+	},
+	{
+	    text: gettext('Remove'),
+	    iconCls: 'fa critical fa-trash-o',
+	    handler: function() { this.up('menu').onForget(); },
+	    cbind: {
+		hidden: '{!onForget}',
+	    },
+	},
+    ],
+});
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 19/20] ui: drop 'permanent' in group/snapshot forget, default is to trash

[Christian Ebner]

Soften the message as the snapshots and groups will not be deleted
immediately anymore, but rather moved to the trash, from where they
still can be restored until the next garbage collection run.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/datastore/Content.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index fffd8c160..a6e28a773 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -1029,9 +1029,9 @@ Ext.define('PBS.DataStoreContent', {
 			if (data.ty === 'ns') {
 			    tip = gettext("Remove namespace '{0}'");
 			} else if (data.ty === 'dir') {
-			    tip = gettext("Permanently forget snapshot '{0}'");
+			    tip = gettext("Forget snapshot '{0}'");
 			} else if (data.ty === 'group') {
-			    tip = gettext("Permanently forget group '{0}'");
+			    tip = gettext("Forget group '{0}'");
 			}
 			return Ext.String.format(tip, v);
 		    },
-- 
2.39.5



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

[pbs-devel] [PATCH v3 proxmox-backup 20/20] ui: mention trash items will be cleared on namespace deletion

[Christian Ebner]

Include the information that removing backup groups will include
tashed items as well.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/window/NamespaceEdit.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/window/NamespaceEdit.js b/www/window/NamespaceEdit.js
index a9a440bbf..0f950e0d5 100644
--- a/www/window/NamespaceEdit.js
+++ b/www/window/NamespaceEdit.js
@@ -70,7 +70,7 @@ Ext.define('PBS.window.NamespaceDelete', {
 	    xtype: 'proxmoxcheckbox',
 	    name: 'delete-groups',
 	    reference: 'rmGroups',
-	    boxLabel: gettext('Delete all Backup Groups'),
+	    boxLabel: gettext('Delete all Backup Groups (including trash items)'),
 	    value: false,
 	    listeners: {
 		change: function(field, value) {
-- 
2.39.5



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