From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 4746C1FF16C for ; Tue, 3 Sep 2024 14:34:13 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9EF5F97AC; Tue, 3 Sep 2024 14:34:41 +0200 (CEST) From: Hannes Laimer To: pbs-devel@lists.proxmox.com Date: Tue, 3 Sep 2024 14:33:53 +0200 Message-Id: <20240903123401.91513-3-h.laimer@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240903123401.91513-1-h.laimer@proxmox.com> References: <20240903123401.91513-1-h.laimer@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.018 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH proxmox-backup RFC 02/10] chunkstore: separate functions into impl block X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" ... based on whether they are reading/writing. Signed-off-by: Hannes Laimer --- 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 { + pub fn open_lookup>(name: &str, base: P) -> Result { + let base: PathBuf = base.into(); - fn chunk_dir>(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 { pub fn create

( name: &str, path: P, @@ -164,13 +163,9 @@ impl ChunkStore { Self::open(name, base, sync_level) } +} - fn lockfile_path>(base: P) -> PathBuf { - let mut lockfile_path: PathBuf = base.into(); - lockfile_path.push(".lock"); - lockfile_path - } - +impl ChunkStore { /// 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 { - // 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 { - // 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(), ×[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 { + // unwrap: only `None` in unit tests + ProcessLocker::try_shared_lock(self.locker.clone().unwrap()) + } + pub fn try_exclusive_lock(&self) -> Result { + // unwrap: only `None` in unit tests + ProcessLocker::try_exclusive_lock(self.locker.clone().unwrap()) + } pub fn oldest_writer(&self) -> Option { // unwrap: only `None` in unit tests ProcessLocker::oldest_shared_lock(self.locker.clone().unwrap()) } +} + +impl ChunkStore { + 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 { + // 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 { + // 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(), ×[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 ChunkStore { + #[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>(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>(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 { - // unwrap: only `None` in unit tests - ProcessLocker::try_shared_lock(self.locker.clone().unwrap()) - } - - pub fn try_exclusive_lock(&self) -> Result { - // 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::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 = 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::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