[pbs-devel] [PATCH proxmox-backup v2 02/12] 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 | 305 +++++++++++++++++--------------
 1 file changed, 169 insertions(+), 136 deletions(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index 9a77bef2..e998c798 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -88,30 +88,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,
@@ -174,13 +173,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> {
     /// Check if the chunkstore path is absolute and that we can
     /// access it. Returns the absolute '.chunks' path on success.
     fn chunk_dir_accessible(base: &Path) -> Result<PathBuf, Error> {
@@ -209,7 +204,7 @@ impl ChunkStore {
     ) -> Result<Self, Error> {
         let base: PathBuf = base.into();
 
-        let chunk_dir = ChunkStore::chunk_dir_accessible(&base)?;
+        let chunk_dir = Self::chunk_dir_accessible(&base)?;
 
         let lockfile_path = Self::lockfile_path(&base);
 
@@ -222,59 +217,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());
-
-        let times: [libc::timespec; 2] = [
-            // access time -> update to now
-            libc::timespec {
-                tv_sec: 0,
-                tv_nsec: libc::UTIME_NOW,
-            },
-            // modification time -> keep as is
-            libc::timespec {
-                tv_sec: 0,
-                tv_nsec: libc::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<
@@ -370,10 +316,116 @@ impl ChunkStore {
         .fuse())
     }
 
+    /// Checks permissions and owner of passed path.
+    fn check_permissions<P: AsRef<Path>>(path: P, file_mode: u32) -> Result<(), Error> {
+        match nix::sys::stat::stat(path.as_ref()) {
+            Ok(stat) => {
+                if stat.st_uid != u32::from(pbs_config::backup_user()?.uid)
+                    || stat.st_gid != u32::from(pbs_config::backup_group()?.gid)
+                    || stat.st_mode & 0o777 != file_mode
+                {
+                    bail!(
+                              "unable to open existing chunk store path {:?} - permissions or owner not correct",
+                              path.as_ref(),
+                          );
+                }
+            }
+            Err(err) => {
+                bail!(
+                    "unable to open existing chunk store path {:?} - {err}",
+                    path.as_ref(),
+                );
+            }
+        }
+        Ok(())
+    }
+
+    /// Verify vital files in datastore. Checks the owner and permissions of: the chunkstore, it's
+    /// subdirectories and the lock file.
+    pub fn verify_chunkstore<P: AsRef<Path>>(path: P) -> Result<(), Error> {
+        // Check datastore root path perm/owner
+        Self::check_permissions(path.as_ref(), 0o755)?;
+
+        let chunk_dir = Self::chunk_dir(path.as_ref());
+        // Check datastore .chunks path perm/owner
+        Self::check_permissions(&chunk_dir, 0o750)?;
+
+        // Check all .chunks subdirectories
+        for i in 0..64 * 1024 {
+            let mut l1path = chunk_dir.clone();
+            l1path.push(format!("{:04x}", i));
+            Self::check_permissions(&l1path, 0o750)?;
+        }
+
+        // Check .lock file
+        let lockfile_path = Self::lockfile_path(path.as_ref());
+        Self::check_permissions(lockfile_path, 0o644)?;
+        Ok(())
+    }
+
+    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 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,
@@ -611,6 +663,43 @@ impl ChunkStore {
         Ok((false, encoded_size))
     }
 
+    pub fn try_exclusive_lock(&self) -> Result<ProcessLockExclusiveGuard, Error> {
+        // unwrap: only `None` in unit tests
+        ProcessLocker::try_exclusive_lock(self.locker.clone().unwrap())
+    }
+}
+
+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
         assert!(self.locker.is_some());
@@ -642,63 +731,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())
-    }
-
-    /// Checks permissions and owner of passed path.
-    fn check_permissions<T: AsRef<Path>>(path: T, file_mode: u32) -> Result<(), Error> {
-        match nix::sys::stat::stat(path.as_ref()) {
-            Ok(stat) => {
-                if stat.st_uid != u32::from(pbs_config::backup_user()?.uid)
-                    || stat.st_gid != u32::from(pbs_config::backup_group()?.gid)
-                    || stat.st_mode & 0o777 != file_mode
-                {
-                    bail!(
-                            "unable to open existing chunk store path {:?} - permissions or owner not correct",
-                            path.as_ref(),
-                        );
-                }
-            }
-            Err(err) => {
-                bail!(
-                    "unable to open existing chunk store path {:?} - {err}",
-                    path.as_ref(),
-                );
-            }
-        }
-        Ok(())
-    }
-
-    /// Verify vital files in datastore. Checks the owner and permissions of: the chunkstore, it's
-    /// subdirectories and the lock file.
-    pub fn verify_chunkstore<T: AsRef<Path>>(path: T) -> Result<(), Error> {
-        // Check datastore root path perm/owner
-        ChunkStore::check_permissions(path.as_ref(), 0o755)?;
-
-        let chunk_dir = Self::chunk_dir(path.as_ref());
-        // Check datastore .chunks path perm/owner
-        ChunkStore::check_permissions(&chunk_dir, 0o750)?;
-
-        // Check all .chunks subdirectories
-        for i in 0..64 * 1024 {
-            let mut l1path = chunk_dir.clone();
-            l1path.push(format!("{:04x}", i));
-            ChunkStore::check_permissions(&l1path, 0o750)?;
-        }
-
-        // Check .lock file
-        let lockfile_path = Self::lockfile_path(path.as_ref());
-        ChunkStore::check_permissions(lockfile_path, 0o644)?;
-        Ok(())
-    }
 }
 
 #[test]
@@ -708,13 +740,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])
@@ -727,7 +760,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.5



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