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 C6225610C2 for ; Tue, 20 Oct 2020 10:08:38 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B9EE0B625 for ; Tue, 20 Oct 2020 10:08:38 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id C3791B603 for ; Tue, 20 Oct 2020 10:08:37 +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 8DFBC45E39 for ; Tue, 20 Oct 2020 10:08:37 +0200 (CEST) From: Stefan Reiter To: pbs-devel@lists.proxmox.com Date: Tue, 20 Oct 2020 10:08:25 +0200 Message-Id: <20201020080825.8451-2-s.reiter@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20201020080825.8451-1-s.reiter@proxmox.com> References: <20201020080825.8451-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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [backup.rs, datastore.rs, environment.rs] Subject: [pbs-devel] [PATCH v2 proxmox-backup 2/2] 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: Tue, 20 Oct 2020 08:08:38 -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 --- v2: * call ensure_finished to be safe * don't drop lock and instead pass it on to verify_backup_dir_with_lock * rebase on build fix (sorry for that...) and drop applied patches src/api2/backup.rs | 16 +++++++++-- src/api2/backup/environment.rs | 51 +++++++++++++++++++++++++++++++++- src/backup/datastore.rs | 12 ++++++-- src/config/datastore.rs | 7 +++++ 4 files changed, 81 insertions(+), 5 deletions(-) diff --git a/src/api2/backup.rs b/src/api2/backup.rs index 626c603f..ca7af3c8 100644 --- a/src/api2/backup.rs +++ b/src/api2/backup.rs @@ -149,7 +149,7 @@ async move { None }; - let (path, is_new, _snap_guard) = datastore.create_locked_backup_dir(&backup_dir)?; + let (path, is_new, snap_guard) = datastore.create_locked_backup_dir(&backup_dir)?; if !is_new { bail!("backup directory already exists."); } @@ -191,7 +191,7 @@ async move { async move { // keep flock until task ends let _group_guard = _group_guard; - let _snap_guard = _snap_guard; + let snap_guard = snap_guard; let _last_guard = _last_guard; let res = select!{ @@ -203,14 +203,26 @@ async move { tools::runtime::block_in_place(|| env.remove_backup())?; return Ok(()); } + + let verify = |env: BackupEnvironment| { + if let Err(err) = env.verify_after_complete(snap_guard) { + 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..27497b24 100644 --- a/src/api2/backup/environment.rs +++ b/src/api2/backup/environment.rs @@ -1,6 +1,7 @@ use anyhow::{bail, format_err, Error}; use std::sync::{Arc, Mutex}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; +use nix::dir::Dir; use ::serde::{Serialize}; use serde_json::{json, Value}; @@ -494,6 +495,54 @@ impl BackupEnvironment { Ok(()) } + /// If verify-new is set on the datastore, this will run a new verify task + /// for the backup. If not, this will return and also drop the passed lock + /// immediately. + pub fn verify_after_complete(&self, snap_lock: Dir) -> Result<(), Error> { + self.ensure_finished()?; + + 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_with_lock( + datastore, + &backup_dir, + verified_chunks, + corrupt_chunks, + worker.clone(), + worker.upid().clone(), + snap_lock, + )? { + 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 d8e7a794..2389cc6c 100644 --- a/src/backup/datastore.rs +++ b/src/backup/datastore.rs @@ -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()); } } @@ -65,7 +68,7 @@ impl DataStore { Ok(datastore) } - fn open_with_path(store_name: &str, path: &Path, _config: DataStoreConfig) -> Result { + fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result { let chunk_store = ChunkStore::open(store_name, path)?; let gc_status = GarbageCollectionStatus::default(); @@ -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