datastore: move update_manifest into BackupDir impl

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index c9aa72b8bb789ce6fc59d77aa09a2ca5c01c1ee2..5440cdb35905defb062fd4bc2d480d78518305e0 100644 (file)
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -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.

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 9a63a9e0cf17ac99f9a2eb60e148c6e653661c88..e2057a00839881f12846db928cf0da200c9e0e5d 100644 (file)
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -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();

diff --git a/pbs-datastore/src/snapshot_reader.rs b/pbs-datastore/src/snapshot_reader.rs
index 44b7694013ce9e3889daa59f938ce025ff0b4f34..54a2b9633471099ef8a50fb6d18e6e2af18ac4a6 100644 (file)
--- a/pbs-datastore/src/snapshot_reader.rs
+++ b/pbs-datastore/src/snapshot_reader.rs
@@ -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!(

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 5dc7c596da6cd9e2f14e5517bb71ec62eace773c..f9d50a5bf636b2a7d97fdf3725091f6280971fe1 100644 (file)
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -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);

diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 8adfce1be19a8d127011e4e0fa2c250bbc59d44a..81879d4af1f61d8c10476c09efde2caf44779dcc 100644 (file)
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -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))?;

diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index 70e28e799f423657b12f6d586d913df3a21fcc7f..db856ecdcf63a8749e2345dd41772a6f82a6333d 100644 (file)
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -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 {

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index b02ae691c55b7b1de372e09d3b324e9808c998a3..966ce2a4c4e14df1423506fd36dc74263590c346 100644 (file)
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -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 {

diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 41ffdc66cf2364313eb17939ce8539c1d30a1875..a5ec9a4c46b8ff9257ecee23fe875f1518c6960b 100644 (file)
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -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))?;