From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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 ; 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 ; Wed, 29 Jun 2022 14:15:48 +0200 (CEST) From: Dominik Csapak 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 --- 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, @@ -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, Error> { } } -pub fn backup_snapshot( +fn backup_snapshot( worker: &WorkerTask, pool_writer: &mut PoolWriter, datastore: Arc, snapshot: BackupDir, -) -> Result { +) -> Result { 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