[pbs-devel] [PATCH proxmox-backup RFC 05/10] backup_info: add generics and separate functions into impl blocks

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

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 179 ++++++++++++++++---------------
 1 file changed, 93 insertions(+), 86 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 414ec878..d353f9d6 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -12,6 +12,7 @@ use pbs_api_types::{
 };
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
+use crate::chunk_store::{CanRead, CanWrite};
 use crate::manifest::{
     BackupManifest, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MANIFEST_LOCK_NAME,
 };
@@ -49,14 +50,14 @@ impl BackupGroupDeleteStats {
 
 /// BackupGroup is a directory containing a list of BackupDir
 #[derive(Clone)]
-pub struct BackupGroup {
-    store: Arc<DataStore>,
+pub struct BackupGroup<T> {
+    store: Arc<DataStore<T>>,
 
     ns: BackupNamespace,
     group: pbs_api_types::BackupGroup,
 }
 
-impl fmt::Debug for BackupGroup {
+impl<T: CanRead> fmt::Debug for BackupGroup<T> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_struct("BackupGroup")
             .field("store", &self.store.name())
@@ -66,9 +67,9 @@ impl fmt::Debug for BackupGroup {
     }
 }
 
-impl BackupGroup {
+impl<T: Clone> BackupGroup<T> {
     pub(crate) fn new(
-        store: Arc<DataStore>,
+        store: Arc<DataStore<T>>,
         ns: BackupNamespace,
         group: pbs_api_types::BackupGroup,
     ) -> Self {
@@ -106,14 +107,30 @@ impl BackupGroup {
         path.push(&self.group.id);
         path
     }
+    pub fn matches(&self, filter: &GroupFilter) -> bool {
+        self.group.matches(filter)
+    }
+
+    pub fn backup_dir(&self, time: i64) -> Result<BackupDir<T>, Error> {
+        BackupDir::with_group(self.clone(), time)
+    }
+
+    pub fn backup_dir_with_rfc3339<D: Into<String>>(
+        &self,
+        time_string: D,
+    ) -> Result<BackupDir<T>, Error> {
+        BackupDir::with_rfc3339(self.clone(), time_string.into())
+    }
+}
 
+impl<T: CanRead> BackupGroup<T> {
     /// Simple check whether a group exists. This does not check whether there are any snapshots,
     /// but rather it simply checks whether the directory exists.
     pub fn exists(&self) -> bool {
         self.full_group_path().exists()
     }
 
-    pub fn list_backups(&self) -> Result<Vec<BackupInfo>, Error> {
+    pub fn list_backups(&self) -> Result<Vec<BackupInfo<T>>, Error> {
         let mut list = vec![];
 
         let path = self.full_group_path();
@@ -145,7 +162,7 @@ impl BackupGroup {
     }
 
     /// Finds the latest backup inside a backup group
-    pub fn last_backup(&self, only_finished: bool) -> Result<Option<BackupInfo>, Error> {
+    pub fn last_backup(&self, only_finished: bool) -> Result<Option<BackupInfo<T>>, Error> {
         let backups = self.list_backups()?;
         Ok(backups
             .into_iter()
@@ -206,23 +223,23 @@ impl BackupGroup {
         Ok(last)
     }
 
-    pub fn matches(&self, filter: &GroupFilter) -> bool {
-        self.group.matches(filter)
-    }
-
-    pub fn backup_dir(&self, time: i64) -> Result<BackupDir, Error> {
-        BackupDir::with_group(self.clone(), time)
+    pub fn iter_snapshots(&self) -> Result<crate::ListSnapshots<T>, Error> {
+        crate::ListSnapshots::new(self.clone())
     }
 
-    pub fn backup_dir_with_rfc3339<T: Into<String>>(
-        &self,
-        time_string: T,
-    ) -> Result<BackupDir, Error> {
-        BackupDir::with_rfc3339(self.clone(), time_string.into())
+    /// Returns the backup owner.
+    ///
+    /// The backup owner is the entity who first created the backup group.
+    pub fn get_owner(&self) -> Result<Authid, Error> {
+        self.store.get_owner(&self.ns, self.as_ref())
     }
+}
 
-    pub fn iter_snapshots(&self) -> Result<crate::ListSnapshots, Error> {
-        crate::ListSnapshots::new(self.clone())
+impl<T: CanWrite> BackupGroup<T> {
+    /// Set the backup owner.
+    pub fn set_owner(&self, auth_id: &Authid, force: bool) -> Result<(), Error> {
+        self.store
+            .set_owner(&self.ns, self.as_ref(), auth_id, force)
     }
 
     /// Destroy the group inclusive all its backup snapshots (BackupDir's)
@@ -254,49 +271,36 @@ impl BackupGroup {
 
         Ok(delete_stats)
     }
-
-    /// Returns the backup owner.
-    ///
-    /// The backup owner is the entity who first created the backup group.
-    pub fn get_owner(&self) -> Result<Authid, Error> {
-        self.store.get_owner(&self.ns, self.as_ref())
-    }
-
-    /// Set the backup owner.
-    pub fn set_owner(&self, auth_id: &Authid, force: bool) -> Result<(), Error> {
-        self.store
-            .set_owner(&self.ns, self.as_ref(), auth_id, force)
-    }
 }
 
-impl AsRef<pbs_api_types::BackupNamespace> for BackupGroup {
+impl<T> AsRef<pbs_api_types::BackupNamespace> for BackupGroup<T> {
     #[inline]
     fn as_ref(&self) -> &pbs_api_types::BackupNamespace {
         &self.ns
     }
 }
 
-impl AsRef<pbs_api_types::BackupGroup> for BackupGroup {
+impl<T> AsRef<pbs_api_types::BackupGroup> for BackupGroup<T> {
     #[inline]
     fn as_ref(&self) -> &pbs_api_types::BackupGroup {
         &self.group
     }
 }
 
-impl From<&BackupGroup> for pbs_api_types::BackupGroup {
-    fn from(group: &BackupGroup) -> pbs_api_types::BackupGroup {
+impl<T> From<&BackupGroup<T>> for pbs_api_types::BackupGroup {
+    fn from(group: &BackupGroup<T>) -> pbs_api_types::BackupGroup {
         group.group.clone()
     }
 }
 
-impl From<BackupGroup> for pbs_api_types::BackupGroup {
-    fn from(group: BackupGroup) -> pbs_api_types::BackupGroup {
+impl<T> From<BackupGroup<T>> for pbs_api_types::BackupGroup {
+    fn from(group: BackupGroup<T>) -> pbs_api_types::BackupGroup {
         group.group
     }
 }
 
-impl From<BackupDir> for BackupGroup {
-    fn from(dir: BackupDir) -> BackupGroup {
+impl<T> From<BackupDir<T>> for BackupGroup<T> {
+    fn from(dir: BackupDir<T>) -> BackupGroup<T> {
         BackupGroup {
             store: dir.store,
             ns: dir.ns,
@@ -305,8 +309,8 @@ impl From<BackupDir> for BackupGroup {
     }
 }
 
-impl From<&BackupDir> for BackupGroup {
-    fn from(dir: &BackupDir) -> BackupGroup {
+impl<T: CanRead> From<&BackupDir<T>> for BackupGroup<T> {
+    fn from(dir: &BackupDir<T>) -> BackupGroup<T> {
         BackupGroup {
             store: Arc::clone(&dir.store),
             ns: dir.ns.clone(),
@@ -319,15 +323,15 @@ impl From<&BackupDir> for BackupGroup {
 ///
 /// We also call this a backup snaphost.
 #[derive(Clone)]
-pub struct BackupDir {
-    store: Arc<DataStore>,
+pub struct BackupDir<T> {
+    store: Arc<DataStore<T>>,
     ns: BackupNamespace,
     dir: pbs_api_types::BackupDir,
     // backup_time as rfc3339
     backup_time_string: String,
 }
 
-impl fmt::Debug for BackupDir {
+impl<T> fmt::Debug for BackupDir<T> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_struct("BackupDir")
             .field("store", &self.store.name())
@@ -338,19 +342,21 @@ impl fmt::Debug for BackupDir {
     }
 }
 
-impl BackupDir {
+impl<T> BackupDir<T> {
     /// Temporarily used for tests.
     #[doc(hidden)]
     pub fn new_test(dir: pbs_api_types::BackupDir) -> Self {
         Self {
-            store: unsafe { DataStore::new_test() },
+            store: DataStore::new_test(),
             backup_time_string: Self::backup_time_to_string(dir.time).unwrap(),
             ns: BackupNamespace::root(),
             dir,
         }
     }
+}
 
-    pub(crate) fn with_group(group: BackupGroup, backup_time: i64) -> Result<Self, Error> {
+impl<T> BackupDir<T> {
+    pub(crate) fn with_group(group: BackupGroup<T>, backup_time: i64) -> Result<Self, Error> {
         let backup_time_string = Self::backup_time_to_string(backup_time)?;
         Ok(Self {
             store: group.store,
@@ -361,7 +367,7 @@ impl BackupDir {
     }
 
     pub(crate) fn with_rfc3339(
-        group: BackupGroup,
+        group: BackupGroup<T>,
         backup_time_string: String,
     ) -> Result<Self, Error> {
         let backup_time = proxmox_time::parse_rfc3339(&backup_time_string)?;
@@ -436,18 +442,6 @@ impl BackupDir {
         proxmox_time::epoch_to_rfc3339_utc(backup_time)
     }
 
-    /// load a `DataBlob` from this snapshot's backup dir.
-    pub fn load_blob(&self, filename: &str) -> Result<DataBlob, Error> {
-        let mut path = self.full_path();
-        path.push(filename);
-
-        proxmox_lang::try_block!({
-            let mut file = std::fs::File::open(&path)?;
-            DataBlob::load_from_reader(&mut file)
-        })
-        .map_err(|err| format_err!("unable to load blob '{:?}' - {}", path, err))
-    }
-
     /// Returns the filename to lock a manifest
     ///
     /// Also creates the basedir. The lockfile is located in
@@ -502,19 +496,25 @@ impl BackupDir {
     }
 
     /// Get the datastore.
-    pub fn datastore(&self) -> &Arc<DataStore> {
+    pub fn datastore(&self) -> &Arc<DataStore<T>> {
         &self.store
     }
+}
+impl<T: CanRead> BackupDir<T> {
+    /// load a `DataBlob` from this snapshot's backup dir.
+    pub fn load_blob(&self, filename: &str) -> Result<DataBlob, Error> {
+        let mut path = self.full_path();
+        path.push(filename);
 
-    /// Returns the backup owner.
-    ///
-    /// The backup owner is the entity who first created the backup group.
-    pub fn get_owner(&self) -> Result<Authid, Error> {
-        self.store.get_owner(&self.ns, self.as_ref())
+        proxmox_lang::try_block!({
+            let mut file = std::fs::File::open(&path)?;
+            DataBlob::load_from_reader(&mut file)
+        })
+        .map_err(|err| format_err!("unable to load blob '{:?}' - {}", path, err))
     }
 
     /// Lock the snapshot and open a reader.
-    pub fn locked_reader(&self) -> Result<crate::SnapshotReader, Error> {
+    pub fn locked_reader(&self) -> Result<crate::SnapshotReader<T>, Error> {
         crate::SnapshotReader::new_do(self.clone())
     }
 
@@ -525,7 +525,15 @@ impl BackupDir {
         let manifest = BackupManifest::try_from(blob)?;
         Ok((manifest, raw_size))
     }
+    /// Returns the backup owner.
+    ///
+    /// The backup owner is the entity who first created the backup group.
+    pub fn get_owner(&self) -> Result<Authid, Error> {
+        self.store.get_owner(&self.ns, self.as_ref())
+    }
+}
 
+impl<T: CanWrite> BackupDir<T> {
     /// 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(
@@ -584,62 +592,61 @@ impl BackupDir {
         Ok(())
     }
 }
-
-impl AsRef<pbs_api_types::BackupNamespace> for BackupDir {
+impl<T> AsRef<pbs_api_types::BackupNamespace> for BackupDir<T> {
     fn as_ref(&self) -> &pbs_api_types::BackupNamespace {
         &self.ns
     }
 }
 
-impl AsRef<pbs_api_types::BackupDir> for BackupDir {
+impl<T> AsRef<pbs_api_types::BackupDir> for BackupDir<T> {
     fn as_ref(&self) -> &pbs_api_types::BackupDir {
         &self.dir
     }
 }
 
-impl AsRef<pbs_api_types::BackupGroup> for BackupDir {
+impl<T> AsRef<pbs_api_types::BackupGroup> for BackupDir<T> {
     fn as_ref(&self) -> &pbs_api_types::BackupGroup {
         &self.dir.group
     }
 }
 
-impl From<&BackupDir> for pbs_api_types::BackupGroup {
-    fn from(dir: &BackupDir) -> pbs_api_types::BackupGroup {
+impl<T> From<&BackupDir<T>> for pbs_api_types::BackupGroup {
+    fn from(dir: &BackupDir<T>) -> pbs_api_types::BackupGroup {
         dir.dir.group.clone()
     }
 }
 
-impl From<BackupDir> for pbs_api_types::BackupGroup {
-    fn from(dir: BackupDir) -> pbs_api_types::BackupGroup {
+impl<T> From<BackupDir<T>> for pbs_api_types::BackupGroup {
+    fn from(dir: BackupDir<T>) -> pbs_api_types::BackupGroup {
         dir.dir.group
     }
 }
 
-impl From<&BackupDir> for pbs_api_types::BackupDir {
-    fn from(dir: &BackupDir) -> pbs_api_types::BackupDir {
+impl<T> From<&BackupDir<T>> for pbs_api_types::BackupDir {
+    fn from(dir: &BackupDir<T>) -> pbs_api_types::BackupDir {
         dir.dir.clone()
     }
 }
 
-impl From<BackupDir> for pbs_api_types::BackupDir {
-    fn from(dir: BackupDir) -> pbs_api_types::BackupDir {
+impl<T> From<BackupDir<T>> for pbs_api_types::BackupDir {
+    fn from(dir: BackupDir<T>) -> pbs_api_types::BackupDir {
         dir.dir
     }
 }
 
 /// Detailed Backup Information, lists files inside a BackupDir
 #[derive(Clone, Debug)]
-pub struct BackupInfo {
+pub struct BackupInfo<T> {
     /// the backup directory
-    pub backup_dir: BackupDir,
+    pub backup_dir: BackupDir<T>,
     /// List of data files
     pub files: Vec<String>,
     /// Protection Status
     pub protected: bool,
 }
 
-impl BackupInfo {
-    pub fn new(backup_dir: BackupDir) -> Result<BackupInfo, Error> {
+impl<T: CanRead> BackupInfo<T> {
+    pub fn new(backup_dir: BackupDir<T>) -> Result<Self, Error> {
         let path = backup_dir.full_path();
 
         let files = list_backup_files(libc::AT_FDCWD, &path)?;
@@ -652,7 +659,7 @@ impl BackupInfo {
         })
     }
 
-    pub fn sort_list(list: &mut [BackupInfo], ascendending: bool) {
+    pub fn sort_list(list: &mut [BackupInfo<T>], ascendending: bool) {
         if ascendending {
             // oldest first
             list.sort_unstable_by(|a, b| a.backup_dir.dir.time.cmp(&b.backup_dir.dir.time));
-- 
2.39.2



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