* [pbs-devel] [RFC PATCH proxmox-backup 1/5] pbs-datastore: add protection info to BackupInfo
2021-09-01 8:25 [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots Dominik Csapak
@ 2021-09-01 8:25 ` Dominik Csapak
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backup 2/5] pbs-datastore: skip protected backups in pruning Dominik Csapak
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2021-09-01 8:25 UTC (permalink / raw)
To: pbs-devel
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 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 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
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [RFC PATCH proxmox-backup 2/5] pbs-datastore: skip protected backups in pruning
2021-09-01 8:25 [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots Dominik Csapak
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backup 1/5] pbs-datastore: add protection info to BackupInfo Dominik Csapak
@ 2021-09-01 8:25 ` Dominik Csapak
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backup 3/5] add protected info of snapshots to api and task logs Dominik Csapak
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2021-09-01 8:25 UTC (permalink / raw)
To: pbs-devel
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 4605e26f..6b76e9d7 100644
--- a/pbs-datastore/src/prune.rs
+++ b/pbs-datastore/src/prune.rs
@@ -17,7 +17,7 @@ use pbs_api_types::{
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>,
@@ -40,6 +40,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
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [RFC PATCH proxmox-backup 3/5] add protected info of snapshots to api and task logs
2021-09-01 8:25 [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots Dominik Csapak
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backup 1/5] pbs-datastore: add protection info to BackupInfo Dominik Csapak
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backup 2/5] pbs-datastore: skip protected backups in pruning Dominik Csapak
@ 2021-09-01 8:25 ` Dominik Csapak
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backup 4/5] api2/admin/datastore: add get/set_protection Dominik Csapak
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2021-09-01 8:25 UTC (permalink / raw)
To: pbs-devel
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/lib.rs | 2 ++
pbs-datastore/src/prune.rs | 15 ++++++++-------
src/api2/admin/datastore.rs | 11 ++++++++---
src/server/prune_job.rs | 4 ++--
4 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 024181ee..4e581358 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -387,6 +387,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 6b76e9d7..3d4eb18e 100644
--- a/pbs-datastore/src/prune.rs
+++ b/pbs-datastore/src/prune.rs
@@ -225,7 +225,7 @@ impl PruneOptions {
pub fn compute_prune_info(
mut list: Vec<BackupInfo>,
options: &PruneOptions,
-) -> Result<Vec<(BackupInfo, bool)>, Error> {
+) -> Result<Vec<(BackupInfo, bool, bool)>, Error> {
let mut mark = HashMap::new();
@@ -273,15 +273,16 @@ pub fn compute_prune_info(
})?;
}
- let prune_info: Vec<(BackupInfo, bool)> = list.into_iter()
+ let prune_info: Vec<(BackupInfo, bool, bool)> = 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 (keep, protected) = match mark.get(&backup_id) {
+ Some(PruneMark::Keep) => (true, false),
+ Some(PruneMark::KeepPartial) => (true, false),
+ Some(PruneMark::Protected) => (true, true),
+ _ => (false, false),
};
- (info, keep)
+ (info, keep, protected)
})
.collect();
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 5470de73..f49cdbee 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -444,6 +444,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)) => {
@@ -482,6 +483,7 @@ pub fn list_snapshots (
files,
size,
owner,
+ protected,
}
},
Err(err) => {
@@ -506,6 +508,7 @@ pub fn list_snapshots (
files,
size: None,
owner,
+ protected,
}
},
}
@@ -849,7 +852,7 @@ pub fn prune(
let keep_all = !prune_options.keeps_something();
if dry_run {
- for (info, mut keep) in prune_info {
+ for (info, mut keep, protected) in prune_info {
if keep_all { keep = true; }
let backup_time = info.backup_dir.backup_time();
@@ -860,6 +863,7 @@ pub fn prune(
"backup-id": group.backup_id(),
"backup-time": backup_time,
"keep": keep,
+ "protected": protected,
}));
}
return Ok(json!(prune_result));
@@ -877,7 +881,7 @@ pub fn prune(
store, backup_type, backup_id));
}
- for (info, mut keep) in prune_info {
+ for (info, mut keep, protected) in prune_info {
if keep_all { keep = true; }
let backup_time = info.backup_dir.backup_time();
@@ -890,7 +894,7 @@ pub fn prune(
group.backup_type(),
group.backup_id(),
timestamp,
- if keep { "keep" } else { "remove" },
+ if keep { if protected { "keep (protected)" } else { "keep" } } else { "remove" },
);
worker.log(msg);
@@ -900,6 +904,7 @@ pub fn prune(
"backup-id": group.backup_id(),
"backup-time": backup_time,
"keep": keep,
+ "protected": protected,
}));
if !(dry_run || keep) {
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index 2ea8b713..bfe2bd9e 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -66,12 +66,12 @@ pub fn prune_datastore(
group.backup_id()
);
- for (info, mut keep) in prune_info {
+ for (info, mut keep, protected) in prune_info {
if keep_all { keep = true; }
task_log!(
worker,
"{} {}/{}/{}",
- if keep { "keep" } else { "remove" },
+ if keep { if protected { "keep (protected)" } else { "keep" } } else { "remove" },
group.backup_type(),
group.backup_id(),
info.backup_dir.backup_time_string()
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [RFC PATCH proxmox-backup 4/5] api2/admin/datastore: add get/set_protection
2021-09-01 8:25 [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots Dominik Csapak
` (2 preceding siblings ...)
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backup 3/5] add protected info of snapshots to api and task logs Dominik Csapak
@ 2021-09-01 8:25 ` Dominik Csapak
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backcup 5/5] ui: PruneInputPanel: add keepReason 'protected' for protected backups Dominik Csapak
2021-09-02 6:25 ` [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots Thomas Lamprecht
5 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2021-09-01 8:25 UTC (permalink / raw)
To: pbs-devel
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 | 101 ++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index f49cdbee..483bf63f 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1749,6 +1749,101 @@ 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)?;
+
+ let protected_path = backup_dir.protected_file(datastore.base_path());
+ if protected {
+ std::fs::File::create(protected_path)
+ .map_err(|err| format_err!("could not create protection file: {}", err))?;
+ } else {
+ std::fs::remove_file(protected_path)
+ .map_err(|err| format_err!("could not remove protection file: {}", err))?;
+ }
+
+ Ok(())
+}
+
#[api(
input: {
properties: {
@@ -1897,6 +1992,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()
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [RFC PATCH proxmox-backcup 5/5] ui: PruneInputPanel: add keepReason 'protected' for protected backups
2021-09-01 8:25 [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots Dominik Csapak
` (3 preceding siblings ...)
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backup 4/5] api2/admin/datastore: add get/set_protection Dominik Csapak
@ 2021-09-01 8:25 ` Dominik Csapak
2021-09-02 6:25 ` [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots Thomas Lamprecht
5 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2021-09-01 8:25 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots
2021-09-01 8:25 [pbs-devel] [RFC PATCH proxmox-backup 0/5] add 'protected' setting for snapshots Dominik Csapak
` (4 preceding siblings ...)
2021-09-01 8:25 ` [pbs-devel] [RFC PATCH proxmox-backcup 5/5] ui: PruneInputPanel: add keepReason 'protected' for protected backups Dominik Csapak
@ 2021-09-02 6:25 ` Thomas Lamprecht
5 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2021-09-02 6:25 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
On 01.09.21 10:25, Dominik Csapak wrote:
> add the means to 'protect' a snapshot against pruning by
> adding a file '.protected' in the snapshot folder
>
> sending as RFC because i am not sure about a couple things, and there
> is no gui (yet):
>
> * does it make sense to protect a snapshot this way? or would it be
> better to have a 'central' protected list somewhere?
> (sounds not right though..)
I find the flag file quite ok, other approach could be to either make the index
immutable, which is probably not a good idea as the trouble-maker FS like NFS surely
make this a PITA. We could also safe it in the unprotected part of the manifest,
similar to how we do with comments, would reduce the file count and often we have
that info already anyway, but if not it may be a bit more expensive to check
than a stat on a separate file.
> * would we want to protect also against manual removal ? or
> 'remove-vanished' on sync?
Would make sense as long as we allow to "unprotect" it, that's how we do it for guests
in PVE too. IMO it's weird/unexpected to mark it protected and allow some API mechanisms
to still remove it.
> * how should the ui look? do we want *another* button in the content
> view?
Why not? We explicitly choose the actions columns for that reason, and the row for
a specific backup has only verify + delete anyway.
Alternatives could be:
- add protection column and add an edit button there, similar to how I did the
edit pencil for the comments
- make a more general edit button for the snapshots, for now this could open a
dialogue for owner, protection and comment.
Just throwing out things here.
> * would it make sense to specify this flag on backup creation too?
IMO that could make sense, depends on how we see the actual use case, i.e., reasons
for setting it.
If I want to make an safety backup before starting a dist upgrade or the like in my
guest in PVE, I'd probably like to specify this flag immediately and not wait until
the backup is done then login into PBS and set it there.
>
> the finished series would fix #3602
please add that to the commit subject, at least of the GUI and API patches, i.e., those
that actually enable this feature for (non-local-root) users.
>
> Dominik Csapak (5):
> pbs-datastore: add protection info to BackupInfo
> pbs-datastore: skip protected backups in pruning
> add protected info of snapshots to api and task logs
> api2/admin/datastore: add get/set_protection
> ui: PruneInputPanel: add keepReason 'protected' for protected backups
>
> pbs-api-types/src/lib.rs | 2 +
> pbs-datastore/src/backup_info.rs | 20 +++++-
> pbs-datastore/src/prune.rs | 21 +++---
> src/api2/admin/datastore.rs | 112 ++++++++++++++++++++++++++++++-
> src/server/prune_job.rs | 4 +-
> www/datastore/Prune.js | 4 ++
> 6 files changed, 148 insertions(+), 15 deletions(-)
>
^ permalink raw reply [flat|nested] 7+ messages in thread