[pbs-devel] [PATCH proxmox-backup] api: tape/backup: improve behaviour for vanishing snapshots

[Dominik Csapak]

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 at 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