[pbs-devel] [PATCH proxmox-backup v2 00/13] add 'protected' setting for snapshots

[pbs-devel] [PATCH proxmox-backup v2 00/13] add 'protected' setting for snapshots

[Dominik Csapak]

add the means to 'protect' a snapshot against pruning and removal by
adding a file '.protected' in the snapshot folder

in the v2, i opted against syncing the protected flag for two reasons:
* it gives the admin the choice on each datastore if he wants to keep
  the snapshot or not(if we sync it, we would have to sync it everytime?)
* it is much simpler to implement

changes from v1:
* rebase on master
* return the PruneMark enum instead of two booleans in compute_prune_info
* lock the snapshot when setting/removig the flag
* don't bail if the file does not exist on removal
* add a 'protected show' command to the client
* add a docs section to explain the protected flag

changes from rfc:
* added gui parts
* added tests
* fixed a bug (that the test uncovered)
* add pbs client command
* prevent removal (also during sync)

Dominik Csapak (13):
  pbs-datastore: add protection info to BackupInfo
  pbs-datastore: skip protected backups in pruning
  add protected info of snapshots to api and task logs
  tests/prune: add tests for protecteded backups
  backup/datastore: prevent protected snapshots to be removed
  pull_store/group: dont try remove locally protected snapshots
  api2: datastore/delete_group: throw error for partially removed group
  api2/admin/datastore: add get/set_protection
  proxmox-backup-client: add 'protected' commands
  ui: PruneInputPanel: add keepReason 'protected' for protected backups
  ui: add protected icon to snapshots
  fix #3602: ui: datastore/Content: add action to set protection status
  docs: add info about protection flag to client docs

 docs/backup-client.rst                |  19 +++++
 pbs-api-types/src/datastore.rs        |   2 +
 pbs-datastore/src/backup_info.rs      |  20 ++++-
 pbs-datastore/src/prune.rs            |  44 ++++++++--
 proxmox-backup-client/src/snapshot.rs | 113 ++++++++++++++++++++++++++
 src/api2/admin/datastore.rs           | 111 +++++++++++++++++++++++--
 src/backup/datastore.rs               |  61 +++++++++++---
 src/server/prune_job.rs               |   6 +-
 src/server/pull.rs                    |  19 ++++-
 tests/prune.rs                        |  44 +++++++++-
 www/datastore/Content.js              |  75 +++++++++++++++++
 www/datastore/Prune.js                |   4 +
 12 files changed, 481 insertions(+), 37 deletions(-)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 01/13] pbs-datastore: add protection info to BackupInfo

[Dominik Csapak]

and add necessary helper functions (protected_file/is_protected)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 20 ++++++++++++++++++--
 tests/prune.rs                   |  2 +-
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index f77098ee..032fad8b 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -92,7 +92,9 @@ impl BackupGroup {
                     BackupDir::with_rfc3339(&self.backup_type, &self.backup_id, backup_time)?;
                 let files = list_backup_files(l2_fd, backup_time)?;
 
-                list.push(BackupInfo { backup_dir, files });
+                let protected = backup_dir.is_protected(base_path.to_owned());
+
+                list.push(BackupInfo { backup_dir, files, protected });
 
                 Ok(())
             },
@@ -253,6 +255,17 @@ impl BackupDir {
         relative_path
     }
 
+    pub fn protected_file(&self, mut path: PathBuf) -> PathBuf {
+        path.push(self.relative_path());
+        path.push(".protected");
+        path
+    }
+
+    pub fn is_protected(&self, base_path: PathBuf) -> bool {
+        let path = self.protected_file(base_path);
+        path.exists()
+    }
+
     pub fn backup_time_to_string(backup_time: i64) -> Result<String, Error> {
         // fixme: can this fail? (avoid unwrap)
         proxmox::tools::time::epoch_to_rfc3339_utc(backup_time)
@@ -293,6 +306,8 @@ pub struct BackupInfo {
     pub backup_dir: BackupDir,
     /// List of data files
     pub files: Vec<String>,
+    /// Protection Status
+    pub protected: bool,
 }
 
 impl BackupInfo {
@@ -301,8 +316,9 @@ impl BackupInfo {
         path.push(backup_dir.relative_path());
 
         let files = list_backup_files(libc::AT_FDCWD, &path)?;
+        let protected = backup_dir.is_protected(base_path.to_owned());
 
-        Ok(BackupInfo { backup_dir, files })
+        Ok(BackupInfo { backup_dir, files, protected })
     }
 
     /// Finds the latest backup inside a backup group
diff --git a/tests/prune.rs b/tests/prune.rs
index 35b40ad7..a733ff82 100644
--- a/tests/prune.rs
+++ b/tests/prune.rs
@@ -41,7 +41,7 @@ fn create_info(
         files.push(String::from(MANIFEST_BLOB_NAME));
     }
 
-    BackupInfo { backup_dir, files }
+    BackupInfo { backup_dir, files, protected: false }
 }
 
 #[test]
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 02/13] pbs-datastore: skip protected backups in pruning

[Dominik Csapak]

as a separate keep reason so it will not be calculated for the other reasons

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-datastore/src/prune.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/pbs-datastore/src/prune.rs b/pbs-datastore/src/prune.rs
index 3b4cf4f2..1eb4208f 100644
--- a/pbs-datastore/src/prune.rs
+++ b/pbs-datastore/src/prune.rs
@@ -7,7 +7,7 @@ use pbs_api_types::PruneOptions;
 
 use super::BackupInfo;
 
-enum PruneMark { Keep, KeepPartial, Remove }
+enum PruneMark { Protected, Keep, KeepPartial, Remove }
 
 fn mark_selections<F: Fn(&BackupInfo) -> Result<String, Error>> (
     mark: &mut HashMap<PathBuf, PruneMark>,
@@ -30,6 +30,10 @@ fn mark_selections<F: Fn(&BackupInfo) -> Result<String, Error>> (
     for info in list {
         let backup_id = info.backup_dir.relative_path();
         if mark.get(&backup_id).is_some() { continue; }
+        if info.protected {
+            mark.insert(backup_id, PruneMark::Protected);
+            continue;
+        }
         let sel_id: String = select_id(&info)?;
 
         if already_included.contains(&sel_id) { continue; }
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 03/13] add protected info of snapshots to api and task logs

[Dominik Csapak]

adds the info that a snapshot is protected to:
* snapshot list
* manual pruning (also dry-run)
* prune jobs

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-api-types/src/datastore.rs |  2 ++
 pbs-datastore/src/prune.rs     | 40 +++++++++++++++++++++++++++-------
 src/api2/admin/datastore.rs    | 15 ++++++++-----
 src/server/prune_job.rs        |  6 ++---
 tests/prune.rs                 |  4 ++--
 5 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 75f82ea4..31b4d483 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -393,6 +393,8 @@ pub struct SnapshotListItem {
     /// The owner of the snapshots group
     #[serde(skip_serializing_if = "Option::is_none")]
     pub owner: Option<Authid>,
+    /// Protection from prunes
+    pub protected: bool,
 }
 
 #[api(
diff --git a/pbs-datastore/src/prune.rs b/pbs-datastore/src/prune.rs
index 1eb4208f..7614999c 100644
--- a/pbs-datastore/src/prune.rs
+++ b/pbs-datastore/src/prune.rs
@@ -7,7 +7,30 @@ use pbs_api_types::PruneOptions;
 
 use super::BackupInfo;
 
-enum PruneMark { Protected, Keep, KeepPartial, Remove }
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub enum PruneMark { Protected, Keep, KeepPartial, Remove }
+
+impl PruneMark {
+    pub fn keep(&self) -> bool {
+        *self != PruneMark::Remove
+    }
+
+    pub fn protected(&self) -> bool {
+        *self == PruneMark::Protected
+    }
+}
+
+impl std::fmt::Display for PruneMark {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let txt = match self {
+            PruneMark::Protected => "protected",
+            PruneMark::Keep => "keep",
+            PruneMark::KeepPartial => "keep-partial",
+            PruneMark::Remove => "remove",
+        };
+        write!(f, "{}", txt)
+    }
+}
 
 fn mark_selections<F: Fn(&BackupInfo) -> Result<String, Error>> (
     mark: &mut HashMap<PathBuf, PruneMark>,
@@ -125,7 +148,7 @@ pub fn cli_options_string(options: &PruneOptions) -> String {
 pub fn compute_prune_info(
     mut list: Vec<BackupInfo>,
     options: &PruneOptions,
-) -> Result<Vec<(BackupInfo, bool)>, Error> {
+) -> Result<Vec<(BackupInfo, PruneMark)>, Error> {
 
     let mut mark = HashMap::new();
 
@@ -173,15 +196,16 @@ pub fn compute_prune_info(
         })?;
     }
 
-    let prune_info: Vec<(BackupInfo, bool)> = list.into_iter()
+    let prune_info: Vec<(BackupInfo, PruneMark)> = list.into_iter()
         .map(|info| {
             let backup_id = info.backup_dir.relative_path();
-            let keep = match mark.get(&backup_id) {
-                Some(PruneMark::Keep) => true,
-                Some(PruneMark::KeepPartial) => true,
-               _ => false,
+            let mark = if info.protected {
+                PruneMark::Protected
+            } else {
+                *mark.get(&backup_id).unwrap_or(&PruneMark::Remove)
             };
-            (info, keep)
+
+            (info, mark)
         })
         .collect();
 
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 0b14dfbf..7390a62f 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -439,6 +439,7 @@ pub fn list_snapshots (
         let backup_type = group.backup_type().to_string();
         let backup_id = group.backup_id().to_string();
         let backup_time = info.backup_dir.backup_time();
+        let protected = info.backup_dir.is_protected(base_path.clone());
 
         match get_all_snapshot_files(&datastore, &info) {
             Ok((manifest, files)) => {
@@ -477,6 +478,7 @@ pub fn list_snapshots (
                     files,
                     size,
                     owner,
+                    protected,
                 }
             },
             Err(err) => {
@@ -501,6 +503,7 @@ pub fn list_snapshots (
                     files,
                     size: None,
                     owner,
+                    protected,
                 }
             },
         }
@@ -844,8 +847,8 @@ pub fn prune(
     let keep_all = !pbs_datastore::prune::keeps_something(&prune_options);
 
     if dry_run {
-        for (info, mut keep) in prune_info {
-            if keep_all { keep = true; }
+        for (info, mark) in prune_info {
+            let keep = keep_all || mark.keep();
 
             let backup_time = info.backup_dir.backup_time();
             let group = info.backup_dir.group();
@@ -855,6 +858,7 @@ pub fn prune(
                 "backup-id": group.backup_id(),
                 "backup-time": backup_time,
                 "keep": keep,
+                "protected": mark.protected(),
             }));
         }
         return Ok(json!(prune_result));
@@ -872,8 +876,8 @@ pub fn prune(
                             store, backup_type, backup_id));
     }
 
-    for (info, mut keep) in prune_info {
-        if keep_all { keep = true; }
+    for (info, mark) in prune_info {
+        let keep = keep_all || mark.keep();
 
         let backup_time = info.backup_dir.backup_time();
         let timestamp = info.backup_dir.backup_time_string();
@@ -885,7 +889,7 @@ pub fn prune(
             group.backup_type(),
             group.backup_id(),
             timestamp,
-            if keep { "keep" } else { "remove" },
+            mark,
         );
 
         worker.log(msg);
@@ -895,6 +899,7 @@ pub fn prune(
             "backup-id": group.backup_id(),
             "backup-time": backup_time,
             "keep": keep,
+            "protected": mark.protected(),
         }));
 
         if !(dry_run || keep) {
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index f298a74c..42c806a1 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -65,12 +65,12 @@ pub fn prune_datastore(
             group.backup_id()
         );
 
-        for (info, mut keep) in prune_info {
-            if keep_all { keep = true; }
+        for (info, mark) in prune_info {
+            let keep = keep_all || mark.keep();
             task_log!(
                 worker,
                 "{} {}/{}/{}",
-                if keep { "keep" } else { "remove" },
+                mark,
                 group.backup_type(),
                 group.backup_id(),
                 info.backup_dir.backup_time_string()
diff --git a/tests/prune.rs b/tests/prune.rs
index a733ff82..14ecc887 100644
--- a/tests/prune.rs
+++ b/tests/prune.rs
@@ -18,8 +18,8 @@ fn get_prune_list(
 
     prune_info
         .iter()
-        .filter_map(|(info, keep)| {
-            if *keep != return_kept {
+        .filter_map(|(info, mark)| {
+            if mark.keep() != return_kept {
                 None
             } else {
                 Some(info.backup_dir.relative_path())
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 04/13] tests/prune: add tests for protecteded backups

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 tests/prune.rs | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tests/prune.rs b/tests/prune.rs
index 14ecc887..24bd10cd 100644
--- a/tests/prune.rs
+++ b/tests/prune.rs
@@ -44,6 +44,44 @@ fn create_info(
     BackupInfo { backup_dir, files, protected: false }
 }
 
+fn create_info_protected(
+    snapshot: &str,
+    partial: bool,
+) -> BackupInfo {
+    let mut info = create_info(snapshot, partial);
+    info.protected = true;
+    info
+}
+
+#[test]
+fn test_prune_protected() -> Result<(), Error> {
+    let mut orig_list = Vec::new();
+
+    orig_list.push(create_info_protected("host/elsa/2019-11-15T09:39:15Z", false));
+    orig_list.push(create_info("host/elsa/2019-11-15T10:39:15Z", false));
+    orig_list.push(create_info("host/elsa/2019-11-15T10:49:15Z", false));
+    orig_list.push(create_info_protected("host/elsa/2019-11-15T10:59:15Z", false));
+
+    eprintln!("{:?}", orig_list);
+
+    let mut options = PruneOptions::default();
+    options.keep_last = Some(1);
+    let remove_list = get_prune_list(orig_list.clone(), false, &options);
+    let expect: Vec<PathBuf> = vec![
+        PathBuf::from("host/elsa/2019-11-15T10:39:15Z"),
+    ];
+    assert_eq!(remove_list, expect);
+
+    let mut options = PruneOptions::default();
+    options.keep_hourly = Some(1);
+    let remove_list = get_prune_list(orig_list.clone(), false, &options);
+    let expect: Vec<PathBuf> = vec![
+        PathBuf::from("host/elsa/2019-11-15T10:39:15Z"),
+    ];
+    assert_eq!(remove_list, expect);
+    Ok(())
+}
+
 #[test]
 fn test_prune_hourly() -> Result<(), Error> {
 
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 05/13] backup/datastore: prevent protected snapshots to be removed

[Dominik Csapak]

by throwing an error for remove_backup_dir, and skipping for
remove_backup_group

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/backup/datastore.rs | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index df8d46b6..a791807f 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -267,8 +267,9 @@ impl DataStore {
         full_path
     }
 
-    /// Remove a complete backup group including all snapshots
-    pub fn remove_backup_group(&self, backup_group: &BackupGroup) ->  Result<(), Error> {
+    /// Remove a complete backup group including all snapshots, returns true
+    /// if all snapshots were removed, and false if some were protected
+    pub fn remove_backup_group(&self, backup_group: &BackupGroup) ->  Result<bool, Error> {
 
         let full_path = self.group_path(backup_group);
 
@@ -276,22 +277,30 @@ impl DataStore {
 
         log::info!("removing backup group {:?}", full_path);
 
+        let mut removed_all = true;
+
         // remove all individual backup dirs first to ensure nothing is using them
         for snap in backup_group.list_backups(&self.base_path())? {
+            if snap.backup_dir.is_protected(self.base_path()) {
+                removed_all = false;
+                continue;
+            }
             self.remove_backup_dir(&snap.backup_dir, false)?;
         }
 
-        // no snapshots left, we can now safely remove the empty folder
-        std::fs::remove_dir_all(&full_path)
-            .map_err(|err| {
-                format_err!(
-                    "removing backup group directory {:?} failed - {}",
-                    full_path,
-                    err,
-                )
-            })?;
+        if removed_all {
+            // no snapshots left, we can now safely remove the empty folder
+            std::fs::remove_dir_all(&full_path)
+                .map_err(|err| {
+                    format_err!(
+                        "removing backup group directory {:?} failed - {}",
+                        full_path,
+                        err,
+                    )
+                })?;
+        }
 
-        Ok(())
+        Ok(removed_all)
     }
 
     /// Remove a backup directory including all content
@@ -305,6 +314,10 @@ impl DataStore {
             _manifest_guard = self.lock_manifest(backup_dir)?;
         }
 
+        if backup_dir.is_protected(self.base_path()) {
+            bail!("cannot remove protected snapshot");
+        }
+
         log::info!("removing backup snapshot {:?}", full_path);
         std::fs::remove_dir_all(&full_path)
             .map_err(|err| {
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 06/13] pull_store/group: dont try remove locally protected snapshots

[Dominik Csapak]

and log if a vanished groups could not be completely deleted if it
contains protected snapshots

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/server/pull.rs | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/server/pull.rs b/src/server/pull.rs
index 5214a218..910ba03b 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -609,6 +609,13 @@ pub async fn pull_group(
             if remote_snapshots.contains(&backup_time) {
                 continue;
             }
+            if info.backup_dir.is_protected(tgt_store.base_path()) {
+                worker.log(format!(
+                        "don't delete vanished snapshot {:?} (protected)",
+                        info.backup_dir.relative_path()
+                ));
+                continue;
+            }
             worker.log(format!(
                 "delete vanished snapshot {:?}",
                 info.backup_dir.relative_path()
@@ -722,9 +729,15 @@ pub async fn pull_store(
                     local_group.backup_type(),
                     local_group.backup_id()
                 ));
-                if let Err(err) = tgt_store.remove_backup_group(&local_group) {
-                    worker.log(err.to_string());
-                    errors = true;
+                match tgt_store.remove_backup_group(&local_group) {
+                    Ok(true) => {},
+                    Ok(false) => {
+                        task_log!(worker, "kept some protected snapshots of group '{}'", local_group);
+                    },
+                    Err(err) => {
+                        task_log!(worker, "{}", err);
+                        errors = true;
+                    }
                 }
             }
             Ok(())
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 07/13] api2: datastore/delete_group: throw error for partially removed group

[Dominik Csapak]

when a group could not be completely removed due to protected snapshot,
throw an error

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/admin/datastore.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 7390a62f..04320f9c 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -270,7 +270,9 @@ pub fn delete_group(
 
     check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?;
 
-    datastore.remove_backup_group(&group)?;
+    if !datastore.remove_backup_group(&group)? {
+        bail!("did not delete whole group because of protected snapthots");
+    }
 
     Ok(Value::Null)
 }
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 08/13] api2/admin/datastore: add get/set_protection

[Dominik Csapak]

for gettin/setting the protected flag for snapshots (akin to notes)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/admin/datastore.rs | 92 +++++++++++++++++++++++++++++++++++++
 src/backup/datastore.rs     | 24 ++++++++++
 2 files changed, 116 insertions(+)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 04320f9c..a92efc02 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1748,6 +1748,92 @@ pub fn set_notes(
     Ok(())
 }
 
+#[api(
+    input: {
+        properties: {
+            store: {
+                schema: DATASTORE_SCHEMA,
+            },
+            "backup-type": {
+                schema: BACKUP_TYPE_SCHEMA,
+            },
+            "backup-id": {
+                schema: BACKUP_ID_SCHEMA,
+            },
+            "backup-time": {
+                schema: BACKUP_TIME_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP, true),
+    },
+)]
+/// Query protection for a specific backup
+pub fn get_protection(
+    store: String,
+    backup_type: String,
+    backup_id: String,
+    backup_time: i64,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<bool, Error> {
+    let datastore = DataStore::lookup_datastore(&store)?;
+
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
+
+    check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_AUDIT)?;
+
+    let protected_path = backup_dir.protected_file(datastore.base_path());
+
+    Ok(protected_path.exists())
+}
+
+#[api(
+    input: {
+        properties: {
+            store: {
+                schema: DATASTORE_SCHEMA,
+            },
+            "backup-type": {
+                schema: BACKUP_TYPE_SCHEMA,
+            },
+            "backup-id": {
+                schema: BACKUP_ID_SCHEMA,
+            },
+            "backup-time": {
+                schema: BACKUP_TIME_SCHEMA,
+            },
+            protected: {
+                description: "Enable/disable protection.",
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["datastore", "{store}"],
+                                           PRIV_DATASTORE_MODIFY | PRIV_DATASTORE_BACKUP,
+                                           true),
+    },
+)]
+/// En- or disable protection for a specific backup
+pub fn set_protection(
+    store: String,
+    backup_type: String,
+    backup_id: String,
+    backup_time: i64,
+    protected: bool,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let datastore = DataStore::lookup_datastore(&store)?;
+
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
+
+    check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_MODIFY)?;
+
+    datastore.update_protection(&backup_dir, protected)
+}
+
 #[api(
     input: {
         properties: {
@@ -1896,6 +1982,12 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
             .get(&API_METHOD_GET_NOTES)
             .put(&API_METHOD_SET_NOTES)
     ),
+    (
+        "protected",
+        &Router::new()
+            .get(&API_METHOD_GET_PROTECTION)
+            .put(&API_METHOD_SET_PROTECTION)
+    ),
     (
         "prune",
         &Router::new()
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index a791807f..2ab0e289 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -860,6 +860,30 @@ impl DataStore {
         Ok(())
     }
 
+    /// Updates the protection status of the specified snapshot.
+    pub fn update_protection(
+        &self,
+        backup_dir: &BackupDir,
+        protection: bool
+    ) -> Result<(), Error> {
+        let full_path = self.snapshot_path(backup_dir);
+
+        let _guard = lock_dir_noblock(&full_path, "snapshot", "possibly running or in use")?;
+
+        let protected_path = backup_dir.protected_file(self.base_path());
+        if protection {
+            std::fs::File::create(protected_path)
+                .map_err(|err| format_err!("could not create protection file: {}", err))?;
+        } else if let Err(err) = std::fs::remove_file(protected_path) {
+            // ignore error for non-existing file
+            if err.kind() != std::io::ErrorKind::NotFound {
+                bail!("could not remove protection file: {}", err);
+            }
+        }
+
+        Ok(())
+    }
+
     pub fn verify_new(&self) -> bool {
         self.verify_new
     }
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 09/13] proxmox-backup-client: add 'protected' commands

[Dominik Csapak]

includes 'update' and 'show' similar to the notes commands

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 proxmox-backup-client/src/snapshot.rs | 113 ++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/proxmox-backup-client/src/snapshot.rs b/proxmox-backup-client/src/snapshot.rs
index 4465f69a..37db5579 100644
--- a/proxmox-backup-client/src/snapshot.rs
+++ b/proxmox-backup-client/src/snapshot.rs
@@ -359,6 +359,118 @@ async fn update_notes(param: Value) -> Result<Value, Error> {
     Ok(Value::Null)
 }
 
+#[api(
+    input: {
+        properties: {
+            repository: {
+                schema: REPO_URL_SCHEMA,
+                optional: true,
+            },
+            snapshot: {
+                type: String,
+                description: "Snapshot path.",
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Show protection status of the specified snapshot
+async fn show_protection(param: Value) -> Result<Value, Error> {
+    let repo = extract_repository_from_value(&param)?;
+    let path = required_string_param(&param, "snapshot")?;
+
+    let snapshot: BackupDir = path.parse()?;
+    let client = connect(&repo)?;
+
+    let path = format!("api2/json/admin/datastore/{}/protected", repo.store());
+
+    let args = json!({
+        "backup-type": snapshot.group().backup_type(),
+        "backup-id": snapshot.group().backup_id(),
+        "backup-time": snapshot.backup_time(),
+    });
+
+    let output_format = get_output_format(&param);
+
+    let mut result = client.get(&path, Some(args)).await?;
+
+    let protected = result["data"].take();
+
+    if output_format == "text" {
+        if let Some(protected) = protected.as_bool() {
+            println!("{}", protected);
+        }
+    } else {
+        format_and_print_result(
+            &json!({
+                "protected": protected,
+            }),
+            &output_format,
+        );
+    }
+
+    Ok(Value::Null)
+}
+
+#[api(
+    input: {
+        properties: {
+            repository: {
+                schema: REPO_URL_SCHEMA,
+                optional: true,
+            },
+            snapshot: {
+                type: String,
+                description: "Snapshot path.",
+            },
+            protected: {
+                type: bool,
+                description: "The protection status.",
+            },
+        }
+    }
+)]
+/// Update Protection Status of a snapshot
+async fn update_protection(protected: bool, param: Value) -> Result<Value, Error> {
+    let repo = extract_repository_from_value(&param)?;
+    let path = required_string_param(&param, "snapshot")?;
+
+    let snapshot: BackupDir = path.parse()?;
+    let mut client = connect(&repo)?;
+
+    let path = format!("api2/json/admin/datastore/{}/protected", repo.store());
+
+    let args = json!({
+        "backup-type": snapshot.group().backup_type(),
+        "backup-id": snapshot.group().backup_id(),
+        "backup-time": snapshot.backup_time(),
+        "protected": protected,
+    });
+
+    client.put(&path, Some(args)).await?;
+
+    Ok(Value::Null)
+}
+
+fn protected_cli() -> CliCommandMap {
+    CliCommandMap::new()
+        .insert(
+            "show",
+            CliCommand::new(&API_METHOD_SHOW_PROTECTION)
+                .arg_param(&["snapshot"])
+                .completion_cb("snapshot", complete_backup_snapshot),
+        )
+        .insert(
+            "update",
+            CliCommand::new(&API_METHOD_UPDATE_PROTECTION)
+                .arg_param(&["snapshot", "protected"])
+                .completion_cb("snapshot", complete_backup_snapshot),
+        )
+}
+
 fn notes_cli() -> CliCommandMap {
     CliCommandMap::new()
         .insert(
@@ -378,6 +490,7 @@ fn notes_cli() -> CliCommandMap {
 pub fn snapshot_mgtm_cli() -> CliCommandMap {
     CliCommandMap::new()
         .insert("notes", notes_cli())
+        .insert("protected", protected_cli())
         .insert(
             "list",
             CliCommand::new(&API_METHOD_LIST_SNAPSHOTS)
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 10/13] ui: PruneInputPanel: add keepReason 'protected' for protected backups

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/datastore/Prune.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/www/datastore/Prune.js b/www/datastore/Prune.js
index 42ea21bf..e4db4b6c 100644
--- a/www/datastore/Prune.js
+++ b/www/datastore/Prune.js
@@ -103,6 +103,10 @@ Ext.define('PBS.Datastore.PruneInputPanel', {
 		let rule = nextRule();
 		for (let backup of backups) {
 		    if (backup.keep) {
+			if (backup.protected) {
+			    backup.keepReason = 'protected';
+			    continue;
+			}
 			counter[rule]++;
 			if (rule !== 'keep-all') {
 			    backup.keepReason = rule + ': ' + counter[rule];
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 11/13] ui: add protected icon to snapshots

[Dominik Csapak]

if they are protected

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/datastore/Content.js | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index 57693785..fbc3e12f 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -640,6 +640,13 @@ Ext.define('PBS.DataStoreContent', {
 	    xtype: 'treecolumn',
 	    header: gettext("Backup Group"),
 	    dataIndex: 'text',
+	    renderer: (value, meta, record) => {
+		let protect = "";
+		if (record.data.protected) {
+		    protect = ` <i class="fa fa-shield"></i>`;
+		}
+		return value + protect;
+	    },
 	    flex: 1,
 	},
 	{
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 12/13] fix #3602: ui: datastore/Content: add action to set protection status

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/datastore/Content.js | 68 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index fbc3e12f..6bb29c2a 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -491,6 +491,68 @@ Ext.define('PBS.DataStoreContent', {
 	    });
 	},
 
+	onProtectionChange: function(view, rI, cI, item, e, rec) {
+	    let me = this;
+	    view = this.getView();
+
+	    if (!(rec && rec.data)) return;
+	    let data = rec.data;
+	    if (!view.datastore) return;
+
+	    let type = data["backup-type"];
+	    let id = data["backup-id"];
+	    let time = (data["backup-time"].getTime()/1000).toFixed(0);
+
+	    let params = {
+		'backup-type': type,
+		'backup-id': id,
+		'backup-time': time,
+	    };
+
+	    let url = `/api2/extjs/admin/datastore/${view.datastore}/protected`;
+
+	    Ext.create('Proxmox.window.Edit', {
+		subject: gettext('Protection') + ` - ${data.text}`,
+		width: 400,
+
+		method: 'PUT',
+		autoShow: true,
+		isCreate: false,
+		autoLoad: true,
+
+		loadUrl: `${url}?${Ext.Object.toQueryString(params)}`,
+		url,
+
+		items: [
+		    {
+			xtype: 'hidden',
+			name: 'backup-type',
+			value: type,
+		    },
+		    {
+			xtype: 'hidden',
+			name: 'backup-id',
+			value: id,
+		    },
+		    {
+			xtype: 'hidden',
+			name: 'backup-time',
+			value: time,
+		    },
+		    {
+			xtype: 'proxmoxcheckbox',
+			fieldLabel: gettext('Protected'),
+			uncheckedValue: 0,
+			name: 'protected',
+			value: data.protected,
+		    },
+		],
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    });
+	},
+
 	onForget: function(view, rI, cI, item, e, rec) {
 	    let me = this;
 	    view = this.getView();
@@ -716,6 +778,12 @@ Ext.define('PBS.DataStoreContent', {
 		    getClass: (v, m, rec) => rec.parentNode.id ==='root' ? 'fa fa-scissors' : 'pmx-hidden',
 		    isActionDisabled: (v, r, c, i, rec) => rec.parentNode.id !=='root',
 		},
+		{
+		    handler: 'onProtectionChange',
+		    getTip: (v, m, rec) => Ext.String.format(gettext("Change protection of '{0}'"), v),
+		    getClass: (v, m, rec) => !rec.data.leaf && rec.parentNode.id !== 'root' ? 'fa fa-shield' : 'pmx-hidden',
+		    isActionDisabled: (v, r, c, i, rec) => !!rec.data.leaf || rec.parentNode.id === 'root',
+		},
 		{
 		    handler: 'onForget',
 		    getTip: (v, m, rec) => rec.parentNode.id !=='root'
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 13/13] docs: add info about protection flag to client docs

[Dominik Csapak]

and mention that sync/pull does not sync the protected flag

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 docs/backup-client.rst | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/docs/backup-client.rst b/docs/backup-client.rst
index 02dae100..f18651ef 100644
--- a/docs/backup-client.rst
+++ b/docs/backup-client.rst
@@ -652,6 +652,25 @@ shows the list of existing snapshots and what actions prune would take.
    in the chunk-store. The chunk-store still contains the data blocks. To free
    space you need to perform :ref:`client_garbage-collection`.
 
+It is also possible to protect single snapshots from being pruned or deleted:
+
+.. code-block:: console
+
+  # proxmox-backup-client snapshot protected update <snapshot> true
+
+This will set the protected flag on the snapshot and prevent pruning or manual
+deletion of this snapshot untilt he flag is removed again with:
+
+.. code-block:: console
+
+  # proxmox-backup-client snapshot protected update <snapshot> false
+
+When a group is with a protected snapshot is deleted, only the non-protected
+ones are removed and the group will remain.
+
+.. note:: This flag will not be synced when using pull or sync jobs. If you
+   want to protect a synced snapshot, you have to manually to this again on
+   the target :ref:`backup server.
 
 .. _client_garbage-collection:
 
-- 
2.30.2