* [pbs-devel] [PATCH proxmox-backup] api: tape/backup: improve behaviour for vanishing snapshots
@ 2022-06-29 12:15 Dominik Csapak
2022-06-30 8:22 ` [pbs-devel] applied: " Wolfgang Bumiller
0 siblings, 1 reply; 2+ messages in thread
From: Dominik Csapak @ 2022-06-29 12:15 UTC (permalink / raw)
To: pbs-devel
when snapshots vanish during tape backup, we skip them. Until now,
we also warned with the error and failed the task at the end.
Since deleting snapshots during tape backup does not really interfere
with it, don't fail the whole task, and only add a log line that it
was skipped.
To differentiate from different errors (e.g. permission problems),
introduce a 'SnapshotBackupResult' which is returned by 'backup_snapshot'.
Also remove the 'pub' there since we don't want to leak the
SnapshotBackupResult type and it's not used anywhere outside this file.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
We could also do a 'task_warn' here, then the task ends a 'Warning' state
instead of OK. Still better than failing the task though.
src/api2/tape/backup.rs | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 5a4008e2..8f7ee5cb 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -376,6 +376,12 @@ pub fn backup(
Ok(upid_str.into())
}
+enum SnapshotBackupResult {
+ Success,
+ Error,
+ Ignored,
+}
+
fn backup_worker(
worker: &WorkerTask,
datastore: Arc<DataStore>,
@@ -489,10 +495,11 @@ fn backup_worker(
need_catalog = true;
- if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
- errors = true;
- } else {
- summary.snapshot_list.push(rel_path);
+ match backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)?
+ {
+ SnapshotBackupResult::Success => summary.snapshot_list.push(rel_path),
+ SnapshotBackupResult::Error => errors = true,
+ SnapshotBackupResult::Ignored => {}
}
progress.done_snapshots = 1;
task_log!(worker, "percentage done: {}", progress);
@@ -514,10 +521,11 @@ fn backup_worker(
need_catalog = true;
- if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
- errors = true;
- } else {
- summary.snapshot_list.push(rel_path);
+ match backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)?
+ {
+ SnapshotBackupResult::Success => summary.snapshot_list.push(rel_path),
+ SnapshotBackupResult::Error => errors = true,
+ SnapshotBackupResult::Ignored => {}
}
progress.done_snapshots = snapshot_number as u64 + 1;
task_log!(worker, "percentage done: {}", progress);
@@ -579,26 +587,31 @@ fn update_media_online_status(drive: &str) -> Result<Option<String>, Error> {
}
}
-pub fn backup_snapshot(
+fn backup_snapshot(
worker: &WorkerTask,
pool_writer: &mut PoolWriter,
datastore: Arc<DataStore>,
snapshot: BackupDir,
-) -> Result<bool, Error> {
+) -> Result<SnapshotBackupResult, Error> {
let snapshot_path = snapshot.relative_path();
task_log!(worker, "backup snapshot {:?}", snapshot_path);
let snapshot_reader = match snapshot.locked_reader() {
Ok(reader) => reader,
Err(err) => {
- // ignore missing snapshots and continue
+ if !snapshot.full_path().exists() {
+ // we got an error and the dir does not exist,
+ // it probably just vanished, so continue
+ task_log!(worker, "snapshot {:?} vanished, skipping", snapshot_path);
+ return Ok(SnapshotBackupResult::Ignored);
+ }
task_warn!(
worker,
"failed opening snapshot {:?}: {}",
snapshot_path,
err
);
- return Ok(false);
+ return Ok(SnapshotBackupResult::Error);
}
};
@@ -666,5 +679,5 @@ pub fn backup_snapshot(
snapshot_path
);
- Ok(true)
+ Ok(SnapshotBackupResult::Success)
}
--
2.30.2
^ permalink raw reply [flat|nested] 2+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup] api: tape/backup: improve behaviour for vanishing snapshots
2022-06-29 12:15 [pbs-devel] [PATCH proxmox-backup] api: tape/backup: improve behaviour for vanishing snapshots Dominik Csapak
@ 2022-06-30 8:22 ` Wolfgang Bumiller
0 siblings, 0 replies; 2+ messages in thread
From: Wolfgang Bumiller @ 2022-06-30 8:22 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pbs-devel
applied, thanks
On Wed, Jun 29, 2022 at 02:15:44PM +0200, Dominik Csapak wrote:
> when snapshots vanish during tape backup, we skip them. Until now,
> we also warned with the error and failed the task at the end.
>
> Since deleting snapshots during tape backup does not really interfere
> with it, don't fail the whole task, and only add a log line that it
> was skipped.
>
> To differentiate from different errors (e.g. permission problems),
> introduce a 'SnapshotBackupResult' which is returned by 'backup_snapshot'.
>
> Also remove the 'pub' there since we don't want to leak the
> SnapshotBackupResult type and it's not used anywhere outside this file.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> We could also do a 'task_warn' here, then the task ends a 'Warning' state
> instead of OK. Still better than failing the task though.
>
> src/api2/tape/backup.rs | 39 ++++++++++++++++++++++++++-------------
> 1 file changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
> index 5a4008e2..8f7ee5cb 100644
> --- a/src/api2/tape/backup.rs
> +++ b/src/api2/tape/backup.rs
> @@ -376,6 +376,12 @@ pub fn backup(
> Ok(upid_str.into())
> }
>
> +enum SnapshotBackupResult {
> + Success,
> + Error,
> + Ignored,
> +}
> +
> fn backup_worker(
> worker: &WorkerTask,
> datastore: Arc<DataStore>,
> @@ -489,10 +495,11 @@ fn backup_worker(
>
> need_catalog = true;
>
> - if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
> - errors = true;
> - } else {
> - summary.snapshot_list.push(rel_path);
> + match backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)?
> + {
> + SnapshotBackupResult::Success => summary.snapshot_list.push(rel_path),
> + SnapshotBackupResult::Error => errors = true,
> + SnapshotBackupResult::Ignored => {}
> }
> progress.done_snapshots = 1;
> task_log!(worker, "percentage done: {}", progress);
> @@ -514,10 +521,11 @@ fn backup_worker(
>
> need_catalog = true;
>
> - if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
> - errors = true;
> - } else {
> - summary.snapshot_list.push(rel_path);
> + match backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)?
> + {
> + SnapshotBackupResult::Success => summary.snapshot_list.push(rel_path),
> + SnapshotBackupResult::Error => errors = true,
> + SnapshotBackupResult::Ignored => {}
> }
> progress.done_snapshots = snapshot_number as u64 + 1;
> task_log!(worker, "percentage done: {}", progress);
> @@ -579,26 +587,31 @@ fn update_media_online_status(drive: &str) -> Result<Option<String>, Error> {
> }
> }
>
> -pub fn backup_snapshot(
> +fn backup_snapshot(
> worker: &WorkerTask,
> pool_writer: &mut PoolWriter,
> datastore: Arc<DataStore>,
> snapshot: BackupDir,
> -) -> Result<bool, Error> {
> +) -> Result<SnapshotBackupResult, Error> {
> let snapshot_path = snapshot.relative_path();
> task_log!(worker, "backup snapshot {:?}", snapshot_path);
>
> let snapshot_reader = match snapshot.locked_reader() {
> Ok(reader) => reader,
> Err(err) => {
> - // ignore missing snapshots and continue
> + if !snapshot.full_path().exists() {
> + // we got an error and the dir does not exist,
> + // it probably just vanished, so continue
> + task_log!(worker, "snapshot {:?} vanished, skipping", snapshot_path);
> + return Ok(SnapshotBackupResult::Ignored);
> + }
> task_warn!(
> worker,
> "failed opening snapshot {:?}: {}",
> snapshot_path,
> err
> );
> - return Ok(false);
> + return Ok(SnapshotBackupResult::Error);
> }
> };
>
> @@ -666,5 +679,5 @@ pub fn backup_snapshot(
> snapshot_path
> );
>
> - Ok(true)
> + Ok(SnapshotBackupResult::Success)
> }
> --
> 2.30.2
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-06-30 8:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-29 12:15 [pbs-devel] [PATCH proxmox-backup] api: tape/backup: improve behaviour for vanishing snapshots Dominik Csapak
2022-06-30 8:22 ` [pbs-devel] applied: " Wolfgang Bumiller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox