From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 3C10A78AC8
 for <pbs-devel@lists.proxmox.com>; Wed, 29 Jun 2022 14:16:19 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 29366252FE
 for <pbs-devel@lists.proxmox.com>; Wed, 29 Jun 2022 14:15:49 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pbs-devel@lists.proxmox.com>; Wed, 29 Jun 2022 14:15:48 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 1E34443D67
 for <pbs-devel@lists.proxmox.com>; Wed, 29 Jun 2022 14:15:48 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Wed, 29 Jun 2022 14:15:44 +0200
Message-Id: <20220629121544.3277589-1-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.30.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.100 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
Subject: [pbs-devel] [PATCH proxmox-backup] api: tape/backup: improve
 behaviour for vanishing snapshots
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 29 Jun 2022 12:16:19 -0000

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