[pbs-devel] [PATCH proxmox-backup v6 1/4] snapshot: add helper function to retrieve verify_state
Gabriel Goller
g.goller at proxmox.com
Fri Nov 22 13:16:14 CET 2024
Add helper functions to retrieve the verify_state from the manifest of a
snapshot. Replaced all the manual "verify_state" parsing with the helper
function.
Suggested-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
---
pbs-datastore/src/backup_info.rs | 9 +++++++--
pbs-datastore/src/manifest.rs | 14 +++++++++++++-
src/api2/admin/datastore.rs | 16 +++++++---------
src/api2/backup/mod.rs | 18 +++++++++++-------
src/backup/verify.rs | 13 ++++++++-----
5 files changed, 46 insertions(+), 24 deletions(-)
diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 62d12b1183df..a581d75757b4 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -8,8 +8,8 @@ use anyhow::{bail, format_err, Error};
use proxmox_sys::fs::{lock_dir_noblock, replace_file, CreateOptions};
use pbs_api_types::{
- Authid, BackupGroupDeleteStats, BackupNamespace, BackupType, GroupFilter, BACKUP_DATE_REGEX,
- BACKUP_FILE_REGEX,
+ Authid, BackupGroupDeleteStats, BackupNamespace, BackupType, GroupFilter, VerifyState,
+ BACKUP_DATE_REGEX, BACKUP_FILE_REGEX,
};
use pbs_config::{open_backup_lockfile, BackupLockGuard};
@@ -555,6 +555,11 @@ impl BackupDir {
Ok(())
}
+
+ /// Load the verify state from the manifest.
+ pub fn verify_state(&self) -> Result<Option<VerifyState>, anyhow::Error> {
+ Ok(self.load_manifest()?.0.verify_state()?.map(|svs| svs.state))
+ }
}
impl AsRef<pbs_api_types::BackupNamespace> for BackupDir {
diff --git a/pbs-datastore/src/manifest.rs b/pbs-datastore/src/manifest.rs
index c3df014272a0..3013fab97221 100644
--- a/pbs-datastore/src/manifest.rs
+++ b/pbs-datastore/src/manifest.rs
@@ -5,7 +5,7 @@ use anyhow::{bail, format_err, Error};
use serde::{Deserialize, Serialize};
use serde_json::{json, Value};
-use pbs_api_types::{BackupType, CryptMode, Fingerprint};
+use pbs_api_types::{BackupType, CryptMode, Fingerprint, SnapshotVerifyState};
use pbs_tools::crypt_config::CryptConfig;
pub const MANIFEST_BLOB_NAME: &str = "index.json.blob";
@@ -242,6 +242,18 @@ impl BackupManifest {
let manifest: BackupManifest = serde_json::from_value(json)?;
Ok(manifest)
}
+
+ /// Get the verify state of the snapshot
+ ///
+ /// Note: New snapshots, which have not been verified yet, do not have a status and this
+ /// function will return `Ok(None)`.
+ pub fn verify_state(&self) -> Result<Option<SnapshotVerifyState>, anyhow::Error> {
+ let verify = self.unprotected["verify_state"].clone();
+ if verify.is_null() {
+ return Ok(None);
+ }
+ Ok(Some(serde_json::from_value::<SnapshotVerifyState>(verify)?))
+ }
}
impl TryFrom<super::DataBlob> for BackupManifest {
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 99b579f02c50..3624dba41199 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -537,15 +537,13 @@ unsafe fn list_snapshots_blocking(
}
};
- let verification = manifest.unprotected["verify_state"].clone();
- let verification: Option<SnapshotVerifyState> =
- match serde_json::from_value(verification) {
- Ok(verify) => verify,
- Err(err) => {
- eprintln!("error parsing verification state : '{}'", err);
- None
- }
- };
+ let verification: Option<SnapshotVerifyState> = match manifest.verify_state() {
+ Ok(verify) => verify,
+ Err(err) => {
+ eprintln!("error parsing verification state : '{}'", err);
+ None
+ }
+ };
let size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum());
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index 63c49f6537f7..dce40e5318b8 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -8,6 +8,7 @@ use hyper::http::request::Parts;
use hyper::{Body, Request, Response, StatusCode};
use serde::Deserialize;
use serde_json::{json, Value};
+use tracing::warn;
use proxmox_rest_server::{H2Service, WorkerTask};
use proxmox_router::{http_err, list_subdirs_api_method};
@@ -19,9 +20,9 @@ use proxmox_sortable_macro::sortable;
use proxmox_sys::fs::lock_dir_noblock_shared;
use pbs_api_types::{
- Authid, BackupNamespace, BackupType, Operation, SnapshotVerifyState, VerifyState,
- BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
- BACKUP_TYPE_SCHEMA, CHUNK_DIGEST_SCHEMA, DATASTORE_SCHEMA, PRIV_DATASTORE_BACKUP,
+ Authid, BackupNamespace, BackupType, Operation, VerifyState, BACKUP_ARCHIVE_NAME_SCHEMA,
+ BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA,
+ CHUNK_DIGEST_SCHEMA, DATASTORE_SCHEMA, PRIV_DATASTORE_BACKUP,
};
use pbs_config::CachedUserInfo;
use pbs_datastore::index::IndexFile;
@@ -159,15 +160,18 @@ fn upgrade_to_backup_protocol(
let info = backup_group.last_backup(true).unwrap_or(None);
if let Some(info) = info {
let (manifest, _) = info.backup_dir.load_manifest()?;
- let verify = manifest.unprotected["verify_state"].clone();
- match serde_json::from_value::<SnapshotVerifyState>(verify) {
- Ok(verify) => match verify.state {
+ match manifest.verify_state() {
+ Ok(Some(verify)) => match verify.state {
VerifyState::Ok => Some(info),
VerifyState::Failed => None,
},
- Err(_) => {
+ Ok(None) => {
// no verify state found, treat as valid
Some(info)
+ },
+ Err(err) => {
+ warn!("error parsing the snapshot manifest: {err:#}");
+ Some(info)
}
}
} else {
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 6ef7e8eb3ebb..c1abe69a4fde 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -5,7 +5,7 @@ use std::time::Instant;
use anyhow::{bail, format_err, Error};
use nix::dir::Dir;
-use tracing::{error, info};
+use tracing::{error, info, warn};
use proxmox_sys::fs::lock_dir_noblock_shared;
use proxmox_worker_task::WorkerTaskContext;
@@ -553,10 +553,13 @@ pub fn verify_filter(
return true;
}
- let raw_verify_state = manifest.unprotected["verify_state"].clone();
- match serde_json::from_value::<SnapshotVerifyState>(raw_verify_state) {
- Err(_) => true, // no last verification, always include
- Ok(last_verify) => {
+ match manifest.verify_state() {
+ Err(err) => {
+ warn!("error reading manifest: {err:#}");
+ true
+ }
+ Ok(None) => true, // no last verification, always include
+ Ok(Some(last_verify)) => {
match outdated_after {
None => false, // never re-verify if ignored and no max age
Some(max_age) => {
--
2.39.5
More information about the pbs-devel
mailing list