datastore: move update_manifest into BackupDir impl

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2022-05-11 18:55:57 +02:00
parent 87cdc327b9
commit 9ccf933be5
8 changed files with 33 additions and 55 deletions

View File

@ -6,7 +6,7 @@ use std::sync::Arc;
use anyhow::{bail, format_err, Error};
use proxmox_sys::fs::lock_dir_noblock;
use proxmox_sys::fs::{lock_dir_noblock, replace_file, CreateOptions};
use pbs_api_types::{
Authid, BackupNamespace, BackupType, GroupFilter, BACKUP_DATE_REGEX, BACKUP_FILE_REGEX,
@ -493,7 +493,22 @@ impl BackupDir {
&self,
update_fn: impl FnOnce(&mut BackupManifest),
) -> Result<(), Error> {
self.store.update_manifest(self, update_fn)
let _guard = self.lock_manifest()?;
let (mut manifest, _) = self.load_manifest()?;
update_fn(&mut manifest);
let manifest = serde_json::to_value(manifest)?;
let manifest = serde_json::to_string_pretty(&manifest)?;
let blob = DataBlob::encode(manifest.as_bytes(), None, true)?;
let raw_data = blob.raw_data();
let mut path = self.full_path();
path.push(MANIFEST_BLOB_NAME);
// atomic replace invalidates flock - no other writes past this point!
replace_file(&path, raw_data, CreateOptions::new(), false)?;
Ok(())
}
/// Cleans up the backup directory by removing any file not mentioned in the manifest.

View File

@ -28,7 +28,7 @@ use crate::chunk_store::ChunkStore;
use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
use crate::index::IndexFile;
use crate::manifest::{archive_type, ArchiveType, BackupManifest, MANIFEST_BLOB_NAME};
use crate::manifest::{archive_type, ArchiveType};
use crate::task_tracking::update_active_operations;
use crate::DataBlob;
@ -1098,37 +1098,6 @@ impl DataStore {
})
}
/// Load the manifest without a lock. Must not be written back.
pub fn load_manifest(&self, backup_dir: &BackupDir) -> Result<(BackupManifest, u64), Error> {
backup_dir.load_manifest()
}
/// Update the manifest of the specified snapshot. Never write a manifest directly,
/// only use this method - anything else may break locking guarantees.
pub fn update_manifest(
&self,
backup_dir: &BackupDir,
update_fn: impl FnOnce(&mut BackupManifest),
) -> Result<(), Error> {
let _guard = backup_dir.lock_manifest()?;
let (mut manifest, _) = self.load_manifest(backup_dir)?;
update_fn(&mut manifest);
let manifest = serde_json::to_value(manifest)?;
let manifest = serde_json::to_string_pretty(&manifest)?;
let blob = DataBlob::encode(manifest.as_bytes(), None, true)?;
let raw_data = blob.raw_data();
let mut path = backup_dir.full_path();
path.push(MANIFEST_BLOB_NAME);
// atomic replace invalidates flock - no other writes past this point!
replace_file(&path, raw_data, CreateOptions::new(), false)?;
Ok(())
}
/// Updates the protection status of the specified snapshot.
pub fn update_protection(&self, backup_dir: &BackupDir, protection: bool) -> Result<(), Error> {
let full_path = backup_dir.full_path();

View File

@ -46,7 +46,7 @@ impl SnapshotReader {
let datastore_name = datastore.name().to_string();
let manifest = match datastore.load_manifest(&snapshot) {
let manifest = match snapshot.load_manifest() {
Ok((manifest, _)) => manifest,
Err(err) => {
bail!(

View File

@ -135,10 +135,9 @@ fn check_privs_and_load_store(
}
fn read_backup_index(
store: &DataStore,
backup_dir: &BackupDir,
) -> Result<(BackupManifest, Vec<BackupContent>), Error> {
let (manifest, index_size) = store.load_manifest(backup_dir)?;
let (manifest, index_size) = backup_dir.load_manifest()?;
let mut result = Vec::new();
for item in manifest.files() {
@ -162,10 +161,9 @@ fn read_backup_index(
}
fn get_all_snapshot_files(
store: &DataStore,
info: &BackupInfo,
) -> Result<(BackupManifest, Vec<BackupContent>), Error> {
let (manifest, mut files) = read_backup_index(store, &info.backup_dir)?;
let (manifest, mut files) = read_backup_index(&info.backup_dir)?;
let file_set = files.iter().fold(HashSet::new(), |mut acc, item| {
acc.insert(item.filename.clone());
@ -373,7 +371,7 @@ pub fn list_snapshot_files(
let info = BackupInfo::new(snapshot)?;
let (_manifest, files) = get_all_snapshot_files(&datastore, &info)?;
let (_manifest, files) = get_all_snapshot_files(&info)?;
Ok(files)
}
@ -503,7 +501,7 @@ pub fn list_snapshots(
};
let protected = info.backup_dir.is_protected();
match get_all_snapshot_files(&datastore, &info) {
match get_all_snapshot_files(&info) {
Ok((manifest, files)) => {
// extract the first line from notes
let comment: Option<String> = manifest.unprotected["notes"]
@ -1369,7 +1367,7 @@ pub fn download_file_decoded(
let file_name = required_string_param(&param, "file-name")?.to_owned();
let (manifest, files) = read_backup_index(&datastore, &backup_dir)?;
let (manifest, files) = read_backup_index(&backup_dir)?;
for file in files {
if file.filename == file_name && file.crypt_mode == Some(CryptMode::Encrypt) {
bail!("cannot decode '{}' - is encrypted", file_name);
@ -1572,7 +1570,7 @@ pub fn catalog(
let file_name = CATALOG_NAME;
let (manifest, files) = read_backup_index(&datastore, &backup_dir)?;
let (manifest, files) = read_backup_index(&backup_dir)?;
for file in files {
if file.filename == file_name && file.crypt_mode == Some(CryptMode::Encrypt) {
bail!("cannot decode '{}' - is encrypted", file_name);
@ -1662,7 +1660,7 @@ pub fn pxar_file_download(
let mut split = components.splitn(2, |c| *c == b'/');
let pxar_name = std::str::from_utf8(split.next().unwrap())?;
let file_path = split.next().unwrap_or(b"/");
let (manifest, files) = read_backup_index(&datastore, &backup_dir)?;
let (manifest, files) = read_backup_index(&backup_dir)?;
for file in files {
if file.filename == pxar_name && file.crypt_mode == Some(CryptMode::Encrypt) {
bail!("cannot decode '{}' - is encrypted", pxar_name);

View File

@ -607,8 +607,8 @@ impl BackupEnvironment {
// check for valid manifest and store stats
let stats = serde_json::to_value(state.backup_stat)?;
self.datastore
.update_manifest(&self.backup_dir, |manifest| {
self.backup_dir
.update_manifest(|manifest| {
manifest.unprotected["chunk_upload_stats"] = stats;
})
.map_err(|err| format_err!("unable to update manifest blob - {}", err))?;

View File

@ -155,7 +155,7 @@ fn upgrade_to_backup_protocol(
let last_backup = {
let info = backup_group.last_backup(true).unwrap_or(None);
if let Some(info) = info {
let (manifest, _) = datastore.load_manifest(&info.backup_dir)?;
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 {

View File

@ -351,10 +351,7 @@ pub fn update_sync_job(
data.remote_store = remote_store;
}
if let Some(remote_ns) = update.remote_ns {
check_max_depth(
&remote_ns,
update.max_depth.unwrap_or(data.max_depth),
)?;
check_max_depth(&remote_ns, update.max_depth.unwrap_or(data.max_depth))?;
data.remote_ns = Some(remote_ns);
}
if let Some(owner) = update.owner {

View File

@ -357,7 +357,7 @@ pub fn verify_backup_dir_with_lock(
filter: Option<&dyn Fn(&BackupManifest) -> bool>,
_snap_lock: Dir,
) -> Result<bool, Error> {
let manifest = match verify_worker.datastore.load_manifest(backup_dir) {
let manifest = match backup_dir.load_manifest() {
Ok((manifest, _)) => manifest,
Err(err) => {
task_log!(
@ -425,9 +425,8 @@ pub fn verify_backup_dir_with_lock(
upid,
};
let verify_state = serde_json::to_value(verify_state)?;
verify_worker
.datastore
.update_manifest(backup_dir, |manifest| {
backup_dir
.update_manifest(|manifest| {
manifest.unprotected["verify_state"] = verify_state;
})
.map_err(|err| format_err!("unable to update manifest blob - {}", err))?;