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)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 67A9B60D40 for ; Mon, 19 Oct 2020 16:46:06 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 65AA12DCDF for ; Mon, 19 Oct 2020 16:46:06 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (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 id D56B62DCC1 for ; Mon, 19 Oct 2020 16:46:02 +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 99D6845E02 for ; Mon, 19 Oct 2020 16:46:02 +0200 (CEST) From: Stefan Reiter To: pbs-devel@lists.proxmox.com Date: Mon, 19 Oct 2020 16:45:23 +0200 Message-Id: <20201019144523.24840-3-s.reiter@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20201019144523.24840-1-s.reiter@proxmox.com> References: <20201019144523.24840-1-s.reiter@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.034 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup 3/3] fix #2988: allow verification after finishing a snapshot 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: Mon, 19 Oct 2020 14:46:06 -0000 To cater to the paranoid, a new datastore-wide setting "verify-new" is introduced. When set, a verify job will be spawned right after a new backup is added to the store (only verifying the added snapshot). Signed-off-by: Stefan Reiter --- Do we want this in the GUI as well? Would be easy to do ofc, but not sure we want to expose this so readily, would give the feeling that it's necessary IMO. src/api2/backup.rs | 15 ++++++++++++ src/api2/backup/environment.rs | 44 +++++++++++++++++++++++++++++++++- src/backup/datastore.rs | 12 ++++++++-- src/config/datastore.rs | 7 ++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/api2/backup.rs b/src/api2/backup.rs index 626c603f..a8a8a4b7 100644 --- a/src/api2/backup.rs +++ b/src/api2/backup.rs @@ -203,14 +203,29 @@ async move { tools::runtime::block_in_place(|| env.remove_backup())?; return Ok(()); } + + let verify = |env: BackupEnvironment| { + // we are done, so we can safely drop the snapshot guard early + // to allow the verify task to acquire it for itself + std::mem::drop(_snap_guard); + if let Err(err) = env.verify_after_complete() { + env.log(format!( + "backup finished, but starting the requested verify task failed: {}", + err + )); + } + }; + match (res, env.ensure_finished()) { (Ok(_), Ok(())) => { env.log("backup finished successfully"); + verify(env); Ok(()) }, (Err(err), Ok(())) => { // ignore errors after finish env.log(format!("backup had errors but finished: {}", err)); + verify(env); Ok(()) }, (Ok(_), Err(err)) => { diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs index c4f166df..316accdc 100644 --- a/src/api2/backup/environment.rs +++ b/src/api2/backup/environment.rs @@ -1,6 +1,6 @@ use anyhow::{bail, format_err, Error}; use std::sync::{Arc, Mutex}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use ::serde::{Serialize}; use serde_json::{json, Value}; @@ -494,6 +494,48 @@ impl BackupEnvironment { Ok(()) } + pub fn verify_after_complete(&self) -> Result<(), Error> { + if !self.datastore.verify_new() { + // no verify requested, do nothing + return Ok(()); + } + + let worker_id = format!("{}_{}_{}_{:08X}", + self.datastore.name(), + self.backup_dir.group().backup_type(), + self.backup_dir.group().backup_id(), + self.backup_dir.backup_time()); + + let datastore = self.datastore.clone(); + let backup_dir = self.backup_dir.clone(); + + WorkerTask::new_thread( + "verify", + Some(worker_id), + self.user.clone(), + false, + move |worker| { + worker.log("Automatically verifying newly added snapshot"); + + let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024*16))); + let corrupt_chunks = Arc::new(Mutex::new(HashSet::with_capacity(64))); + + if !verify_backup_dir( + datastore, + &backup_dir, + verified_chunks, + corrupt_chunks, + worker.clone(), + worker.upid().clone(), + )? { + bail!("verification failed - please check the log for details"); + } + + Ok(()) + }, + ).map(|_| ()) + } + pub fn log>(&self, msg: S) { self.worker.log(msg); } diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs index a856e2f4..2389cc6c 100644 --- a/src/backup/datastore.rs +++ b/src/backup/datastore.rs @@ -18,7 +18,7 @@ use super::fixed_index::{FixedIndexReader, FixedIndexWriter}; use super::manifest::{MANIFEST_BLOB_NAME, MANIFEST_LOCK_NAME, CLIENT_LOG_BLOB_NAME, BackupManifest}; use super::index::*; use super::{DataBlob, ArchiveType, archive_type}; -use crate::config::datastore; +use crate::config::datastore::{self, DataStoreConfig}; use crate::task::TaskState; use crate::tools; use crate::tools::format::HumanByte; @@ -38,6 +38,7 @@ pub struct DataStore { chunk_store: Arc, gc_mutex: Mutex, last_gc_status: Mutex, + verify_new: bool, } impl DataStore { @@ -52,7 +53,9 @@ impl DataStore { if let Some(datastore) = map.get(name) { // Compare Config - if changed, create new Datastore object! - if datastore.chunk_store.base == path { + if datastore.chunk_store.base == path && + datastore.verify_new == config.verify_new.unwrap_or(false) + { return Ok(datastore.clone()); } } @@ -74,6 +77,7 @@ impl DataStore { chunk_store: Arc::new(chunk_store), gc_mutex: Mutex::new(false), last_gc_status: Mutex::new(gc_status), + verify_new: config.verify_new.unwrap_or(false), }) } @@ -680,4 +684,8 @@ impl DataStore { Ok(()) } + + pub fn verify_new(&self) -> bool { + self.verify_new + } } diff --git a/src/config/datastore.rs b/src/config/datastore.rs index aaf977a7..943364fd 100644 --- a/src/config/datastore.rs +++ b/src/config/datastore.rs @@ -72,6 +72,10 @@ pub const DIR_NAME_SCHEMA: Schema = StringSchema::new("Directory name").schema() optional: true, schema: PRUNE_SCHEMA_KEEP_YEARLY, }, + "verify-new": { + optional: true, + type: bool, + }, } )] #[serde(rename_all="kebab-case")] @@ -100,6 +104,9 @@ pub struct DataStoreConfig { pub keep_monthly: Option, #[serde(skip_serializing_if="Option::is_none")] pub keep_yearly: Option, + /// If enabled, all backups will be verified right after completion. + #[serde(skip_serializing_if="Option::is_none")] + pub verify_new: Option, } fn init() -> SectionConfig { -- 2.20.1