[pbs-devel] [PATCH proxmox-backup RFC 02/10] chunkstore: separate functions into impl block

[Hannes Laimer <h.laimer@proxmox.com>]

... based on whether they are reading/writing.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-datastore/src/chunk_store.rs | 211 ++++++++++++++++++-------------
 1 file changed, 120 insertions(+), 91 deletions(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index 2ffd8488..c9f316ad 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -78,30 +78,29 @@ fn digest_to_prefix(digest: &[u8]) -> PathBuf {
     path.into()
 }
 
-impl ChunkStore {
-    #[doc(hidden)]
-    pub unsafe fn panic_store() -> Self {
-        Self {
-            name: String::new(),
-            base: PathBuf::new(),
-            chunk_dir: PathBuf::new(),
-            mutex: Mutex::new(()),
-            locker: None,
-            sync_level: Default::default(),
-        }
-    }
+impl ChunkStore<Lookup> {
+    pub fn open_lookup<P: Into<PathBuf>>(name: &str, base: P) -> Result<Self, Error> {
+        let base: PathBuf = base.into();
 
-    fn chunk_dir<P: AsRef<Path>>(path: P) -> PathBuf {
-        let mut chunk_dir: PathBuf = PathBuf::from(path.as_ref());
-        chunk_dir.push(".chunks");
+        if !base.is_absolute() {
+            bail!("expected absolute path - got {:?}", base);
+        }
 
-        chunk_dir
-    }
+        let chunk_dir = Self::chunk_dir(&base);
 
-    pub fn base(&self) -> &Path {
-        &self.base
+        Ok(Self {
+            name: name.to_owned(),
+            base,
+            chunk_dir,
+            locker: None,
+            mutex: Mutex::new(()),
+            sync_level: DatastoreFSyncLevel::None,
+            _marker: std::marker::PhantomData,
+        })
     }
+}
 
+impl ChunkStore<Write> {
     pub fn create<P>(
         name: &str,
         path: P,
@@ -164,13 +163,9 @@ impl ChunkStore {
 
         Self::open(name, base, sync_level)
     }
+}
 
-    fn lockfile_path<P: Into<PathBuf>>(base: P) -> PathBuf {
-        let mut lockfile_path: PathBuf = base.into();
-        lockfile_path.push(".lock");
-        lockfile_path
-    }
-
+impl<T: CanRead> ChunkStore<T> {
     /// Opens the chunk store with a new process locker.
     ///
     /// Note that this must be used with care, as it's dangerous to create two instances on the
@@ -204,62 +199,10 @@ impl ChunkStore {
             locker: Some(locker),
             mutex: Mutex::new(()),
             sync_level,
+            _marker: std::marker::PhantomData,
         })
     }
 
-    pub fn touch_chunk(&self, digest: &[u8; 32]) -> Result<(), Error> {
-        // unwrap: only `None` in unit tests
-        assert!(self.locker.is_some());
-
-        self.cond_touch_chunk(digest, true)?;
-        Ok(())
-    }
-
-    pub fn cond_touch_chunk(&self, digest: &[u8; 32], assert_exists: bool) -> Result<bool, Error> {
-        // unwrap: only `None` in unit tests
-        assert!(self.locker.is_some());
-
-        let (chunk_path, _digest_str) = self.chunk_path(digest);
-        self.cond_touch_path(&chunk_path, assert_exists)
-    }
-
-    pub fn cond_touch_path(&self, path: &Path, assert_exists: bool) -> Result<bool, Error> {
-        // unwrap: only `None` in unit tests
-        assert!(self.locker.is_some());
-
-        const UTIME_NOW: i64 = (1 << 30) - 1;
-        const UTIME_OMIT: i64 = (1 << 30) - 2;
-
-        let times: [libc::timespec; 2] = [
-            // access time -> update to now
-            libc::timespec {
-                tv_sec: 0,
-                tv_nsec: UTIME_NOW,
-            },
-            // modification time -> keep as is
-            libc::timespec {
-                tv_sec: 0,
-                tv_nsec: UTIME_OMIT,
-            },
-        ];
-
-        use nix::NixPath;
-
-        let res = path.with_nix_path(|cstr| unsafe {
-            let tmp = libc::utimensat(-1, cstr.as_ptr(), &times[0], libc::AT_SYMLINK_NOFOLLOW);
-            nix::errno::Errno::result(tmp)
-        })?;
-
-        if let Err(err) = res {
-            if !assert_exists && err == nix::errno::Errno::ENOENT {
-                return Ok(false);
-            }
-            bail!("update atime failed for chunk/file {path:?} - {err}");
-        }
-
-        Ok(true)
-    }
-
     pub fn get_chunk_iterator(
         &self,
     ) -> Result<
@@ -354,11 +297,74 @@ impl ChunkStore {
         })
         .fuse())
     }
+    pub fn try_shared_lock(&self) -> Result<ProcessLockSharedGuard, Error> {
+        // unwrap: only `None` in unit tests
+        ProcessLocker::try_shared_lock(self.locker.clone().unwrap())
+    }
 
+    pub fn try_exclusive_lock(&self) -> Result<ProcessLockExclusiveGuard, Error> {
+        // unwrap: only `None` in unit tests
+        ProcessLocker::try_exclusive_lock(self.locker.clone().unwrap())
+    }
     pub fn oldest_writer(&self) -> Option<i64> {
         // unwrap: only `None` in unit tests
         ProcessLocker::oldest_shared_lock(self.locker.clone().unwrap())
     }
+}
+
+impl<T: CanWrite> ChunkStore<T> {
+    pub fn touch_chunk(&self, digest: &[u8; 32]) -> Result<(), Error> {
+        // unwrap: only `None` in unit tests
+        assert!(self.locker.is_some());
+
+        self.cond_touch_chunk(digest, true)?;
+        Ok(())
+    }
+
+    pub fn cond_touch_chunk(&self, digest: &[u8; 32], assert_exists: bool) -> Result<bool, Error> {
+        // unwrap: only `None` in unit tests
+        assert!(self.locker.is_some());
+
+        let (chunk_path, _digest_str) = self.chunk_path(digest);
+        self.cond_touch_path(&chunk_path, assert_exists)
+    }
+
+    pub fn cond_touch_path(&self, path: &Path, assert_exists: bool) -> Result<bool, Error> {
+        // unwrap: only `None` in unit tests
+        assert!(self.locker.is_some());
+
+        const UTIME_NOW: i64 = (1 << 30) - 1;
+        const UTIME_OMIT: i64 = (1 << 30) - 2;
+
+        let times: [libc::timespec; 2] = [
+            // access time -> update to now
+            libc::timespec {
+                tv_sec: 0,
+                tv_nsec: UTIME_NOW,
+            },
+            // modification time -> keep as is
+            libc::timespec {
+                tv_sec: 0,
+                tv_nsec: UTIME_OMIT,
+            },
+        ];
+
+        use nix::NixPath;
+
+        let res = path.with_nix_path(|cstr| unsafe {
+            let tmp = libc::utimensat(-1, cstr.as_ptr(), &times[0], libc::AT_SYMLINK_NOFOLLOW);
+            nix::errno::Errno::result(tmp)
+        })?;
+
+        if let Err(err) = res {
+            if !assert_exists && err == nix::errno::Errno::ENOENT {
+                return Ok(false);
+            }
+            bail!("update atime failed for chunk/file {path:?} - {err}");
+        }
+
+        Ok(true)
+    }
 
     pub fn sweep_unused_chunks(
         &self,
@@ -534,6 +540,38 @@ impl ChunkStore {
 
         Ok((false, encoded_size))
     }
+}
+
+impl<T> ChunkStore<T> {
+    #[doc(hidden)]
+    pub fn dummy_store() -> Self {
+        Self {
+            name: String::new(),
+            base: PathBuf::new(),
+            chunk_dir: PathBuf::new(),
+            mutex: Mutex::new(()),
+            locker: None,
+            sync_level: Default::default(),
+            _marker: std::marker::PhantomData,
+        }
+    }
+
+    fn chunk_dir<P: AsRef<Path>>(path: P) -> PathBuf {
+        let mut chunk_dir: PathBuf = PathBuf::from(path.as_ref());
+        chunk_dir.push(".chunks");
+
+        chunk_dir
+    }
+
+    pub fn base(&self) -> &Path {
+        &self.base
+    }
+
+    fn lockfile_path<P: Into<PathBuf>>(base: P) -> PathBuf {
+        let mut lockfile_path: PathBuf = base.into();
+        lockfile_path.push(".lock");
+        lockfile_path
+    }
 
     pub fn chunk_path(&self, digest: &[u8; 32]) -> (PathBuf, String) {
         // unwrap: only `None` in unit tests
@@ -566,16 +604,6 @@ impl ChunkStore {
 
         self.base.clone()
     }
-
-    pub fn try_shared_lock(&self) -> Result<ProcessLockSharedGuard, Error> {
-        // unwrap: only `None` in unit tests
-        ProcessLocker::try_shared_lock(self.locker.clone().unwrap())
-    }
-
-    pub fn try_exclusive_lock(&self) -> Result<ProcessLockExclusiveGuard, Error> {
-        // unwrap: only `None` in unit tests
-        ProcessLocker::try_exclusive_lock(self.locker.clone().unwrap())
-    }
 }
 
 #[test]
@@ -585,13 +613,14 @@ fn test_chunk_store1() {
 
     if let Err(_e) = std::fs::remove_dir_all(".testdir") { /* ignore */ }
 
-    let chunk_store = ChunkStore::open("test", &path, DatastoreFSyncLevel::None);
+    let chunk_store: Result<ChunkStore<Read>, _> =
+        ChunkStore::open("test", &path, DatastoreFSyncLevel::None);
     assert!(chunk_store.is_err());
 
     let user = nix::unistd::User::from_uid(nix::unistd::Uid::current())
         .unwrap()
         .unwrap();
-    let chunk_store =
+    let chunk_store: ChunkStore<Write> =
         ChunkStore::create("test", &path, user.uid, user.gid, DatastoreFSyncLevel::None).unwrap();
 
     let (chunk, digest) = crate::data_blob::DataChunkBuilder::new(&[0u8, 1u8])
@@ -604,7 +633,7 @@ fn test_chunk_store1() {
     let (exists, _) = chunk_store.insert_chunk(&chunk, &digest).unwrap();
     assert!(exists);
 
-    let chunk_store =
+    let chunk_store: Result<ChunkStore<Write>, _> =
         ChunkStore::create("test", &path, user.uid, user.gid, DatastoreFSyncLevel::None);
     assert!(chunk_store.is_err());
 
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel