public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [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
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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal