[pbs-devel] [PATCH proxmox-backup v1 2/3] fix: datastore: move manifest locking to new locking method

[Stefan Sterz]

adds double stat'ing and removes directory hierarchy to bring manifest
locking in-line with other locks used by the datastore trait.

Signed-off-by: Stefan Sterz <s.sterz at proxmox.com>
---
 pbs-datastore/src/datastore.rs | 46 +++++++++++++---------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index cb2a8a4e..619e2710 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -30,9 +30,7 @@ use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
 use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
 use crate::index::IndexFile;
 use crate::manifest::{
-    MANIFEST_BLOB_NAME, MANIFEST_LOCK_NAME, CLIENT_LOG_BLOB_NAME,
-    ArchiveType, BackupManifest,
-    archive_type,
+    archive_type, ArchiveType, BackupManifest, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME,
 };
 
 lazy_static! {
@@ -811,38 +809,30 @@ impl DataStore {
     ///
     /// Also creates the basedir. The lockfile is located in
     /// '/run/proxmox-backup/locks/{datastore}/{type}/{id}/{timestamp}.index.json.lck'
-    fn manifest_lock_path(
-        &self,
-        backup_dir: &BackupDir,
-    ) -> Result<String, Error> {
-        let mut path = format!(
-            "{}/{}/{}/{}",
-            DATASTORE_LOCKS_DIR,
-            self.name(),
+    fn manifest_lock_path(&self, backup_dir: &BackupDir) -> Result<PathBuf, Error> {
+        let path = Path::new(DATASTORE_LOCKS_DIR).join(self.name());
+
+        let rpath = format!(
+            "{}/{}/{}-manifest",
             backup_dir.group().backup_type(),
             backup_dir.group().backup_id(),
+            backup_dir.backup_time_string(),
         );
+
         std::fs::create_dir_all(&path)?;
-        use std::fmt::Write;
-        write!(path, "/{}{}", backup_dir.backup_time_string(), &MANIFEST_LOCK_NAME)?;
+        let rpath_hash = openssl::sha::sha256(rpath.as_bytes());
 
-        Ok(path)
+        Ok(path.join(hex::encode(rpath_hash)))
     }
 
-    fn lock_manifest(
-        &self,
-        backup_dir: &BackupDir,
-    ) -> Result<BackupLockGuard, Error> {
-        let path = self.manifest_lock_path(backup_dir)?;
-
-        // update_manifest should never take a long time, so if someone else has
-        // the lock we can simply block a bit and should get it soon
-        open_backup_lockfile(&path, Some(Duration::from_secs(5)), true)
-            .map_err(|err| {
-                format_err!(
-                    "unable to acquire manifest lock {:?} - {}", &path, err
-                )
-            })
+    fn lock_manifest(&self, backup_dir: &BackupDir) -> Result<BackupLockGuard, Error> {
+        self.lock_helper(&self.manifest_lock_path(backup_dir)?, |p| {
+            // update_manifest should never take a long time, so if
+            // someone else has the lock we can simply block a bit
+            // and should get it soon
+            open_backup_lockfile(&p, Some(Duration::from_secs(5)), true)
+                .map_err(|err| format_err!("unable to acquire manifest lock {:?} - {}", &p, err))
+        })
     }
 
     /// Load the manifest without a lock. Must not be written back.
-- 
2.30.2