datastore: factor type out of ListGroups into ListGroupsType

In the API we want to iterate over all backup groups
belonging to a particular type at least once, and iterating
through *everything* and simply "skipping" over every single
entry from another type makes no sense given that the groups
are organized into subdirectories based on their type.

Let's have an `.iter_backup_type()` method which returns an
iterator over all the groups of a specific type named
ListGroupsType and factorize the type level iterator out of
ListGroups for reuse.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 1fa15362fcf2b2d7c8016a9d0ac6d45cecd70fc8..c871eae5192e4b9a6124c5fa9df4a63048c35cba 100644 (file)
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -26,7 +26,7 @@ use crate::backup_info::{BackupDir, BackupGroup};
 use crate::chunk_store::ChunkStore;
 use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
 use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
-use crate::hierarchy::{ListGroups, ListNamespaces, ListNamespacesRecursive};
+use crate::hierarchy::{ListGroups, ListGroupsType, ListNamespaces, ListNamespacesRecursive};
 use crate::index::IndexFile;
 use crate::manifest::{archive_type, ArchiveType};
 use crate::task_tracking::update_active_operations;
@@ -371,6 +371,13 @@ impl DataStore {
         path
     }
 
+    /// Returns the absolute path for a backup_type
+    pub fn type_path(&self, ns: &BackupNamespace, backup_type: BackupType) -> PathBuf {
+        let mut full_path = self.namespace_path(ns);
+        full_path.push(backup_type.to_string());
+        full_path
+    }
+
     /// Returns the absolute path for a backup_group
     pub fn group_path(
         &self,
@@ -762,6 +769,31 @@ impl DataStore {
         }))
     }
 
+    /// Get a streaming iter over top-level backup groups of a datatstore of a particular type.
+    ///
+    /// The iterated item is still a Result that can contain errors from rather unexptected FS or
+    /// parsing errors.
+    pub fn iter_backup_type(
+        self: &Arc<DataStore>,
+        ns: BackupNamespace,
+        ty: BackupType,
+    ) -> Result<ListGroupsType, Error> {
+        ListGroupsType::new(Arc::clone(self), ns, ty)
+    }
+
+    /// Get a streaming iter over top-level backup groups of a datatstore of a particular type,
+    /// filtered by `Ok` results
+    ///
+    /// The iterated item's result is already unwrapped, if it contained an error it will be
+    /// logged. Can be useful in iterator chain commands
+    pub fn iter_backup_type_ok(
+        self: &Arc<DataStore>,
+        ns: BackupNamespace,
+        ty: BackupType,
+    ) -> Result<impl Iterator<Item = BackupGroup> + 'static, Error> {
+        Ok(self.iter_backup_type(ns, ty)?.ok())
+    }
+
     /// Get a streaming iter over top-level backup groups of a datatstore
     ///
     /// The iterated item is still a Result that can contain errors from rather unexptected FS or
@@ -781,16 +813,7 @@ impl DataStore {
         self: &Arc<DataStore>,
         ns: BackupNamespace,
     ) -> Result<impl Iterator<Item = BackupGroup> + 'static, Error> {
-        let this = Arc::clone(self);
-        Ok(self
-            .iter_backup_groups(ns)?
-            .filter_map(move |group| match group {
-                Ok(group) => Some(group),
-                Err(err) => {
-                    log::error!("list groups error on datastore {} - {}", this.name(), err);
-                    None
-                }
-            }))
+        Ok(self.iter_backup_groups(ns)?.ok())
     }
 
     /// Get a in-memory vector for all top-level backup groups of a datatstore

diff --git a/pbs-datastore/src/hierarchy.rs b/pbs-datastore/src/hierarchy.rs
index 8609364b69cc94469f5a3bb9334355a5303c4715..260d5406902df161954012b96855f863b7038b0c 100644 (file)
--- a/pbs-datastore/src/hierarchy.rs
+++ b/pbs-datastore/src/hierarchy.rs
@@ -1,3 +1,4 @@
+use std::os::unix::io::RawFd;
 use std::path::PathBuf;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -67,25 +68,97 @@ impl Iterator for ListSnapshots {
     }
 }
 
+/// An iterator for a single backup group type.
+pub struct ListGroupsType {
+    store: Arc<DataStore>,
+    ns: BackupNamespace,
+    ty: BackupType,
+    dir: proxmox_sys::fs::ReadDir,
+}
+
+impl ListGroupsType {
+    pub fn new(store: Arc<DataStore>, ns: BackupNamespace, ty: BackupType) -> Result<Self, Error> {
+        Self::new_at(libc::AT_FDCWD, store, ns, ty)
+    }
+
+    fn new_at(
+        fd: RawFd,
+        store: Arc<DataStore>,
+        ns: BackupNamespace,
+        ty: BackupType,
+    ) -> Result<Self, Error> {
+        Ok(Self {
+            dir: proxmox_sys::fs::read_subdir(fd, &store.type_path(&ns, ty))?,
+            store,
+            ns,
+            ty,
+        })
+    }
+
+    pub(crate) fn ok(self) -> ListGroupsOk<Self> {
+        ListGroupsOk::new(self)
+    }
+}
+
+impl Iterator for ListGroupsType {
+    type Item = Result<BackupGroup, Error>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        loop {
+            let item = self.dir.next()?;
+
+            let entry = match item {
+                Ok(ref entry) => {
+                    match entry.file_type() {
+                        Some(nix::dir::Type::Directory) => entry, // OK
+                        None => match get_file_type(entry.parent_fd(), entry.file_name()) {
+                            Ok(nix::dir::Type::Directory) => entry,
+                            Ok(_) => continue,
+                            Err(err) => {
+                                log::info!("error listing groups for {}: {err}", self.store.name());
+                                continue;
+                            }
+                        },
+                        _ => continue,
+                    }
+                }
+                Err(err) => return Some(Err(err)),
+            };
+
+            if let Ok(name) = entry.file_name().to_str() {
+                if BACKUP_ID_REGEX.is_match(name) {
+                    return Some(Ok(BackupGroup::new(
+                        Arc::clone(&self.store),
+                        self.ns.clone(),
+                        (self.ty, name.to_owned()).into(),
+                    )));
+                }
+            }
+        }
+    }
+}
+
 /// A iterator for a (single) level of Backup Groups
 pub struct ListGroups {
     store: Arc<DataStore>,
     ns: BackupNamespace,
     type_fd: proxmox_sys::fs::ReadDir,
-    id_state: Option<(BackupType, proxmox_sys::fs::ReadDir)>,
+    id_state: Option<ListGroupsType>,
 }
 
 impl ListGroups {
     pub fn new(store: Arc<DataStore>, ns: BackupNamespace) -> Result<Self, Error> {
-        let mut base_path = store.base_path();
-        base_path.push(ns.path());
-        Ok(ListGroups {
-            type_fd: proxmox_sys::fs::read_subdir(libc::AT_FDCWD, &base_path)?,
+        Ok(Self {
+            type_fd: proxmox_sys::fs::read_subdir(libc::AT_FDCWD, &store.namespace_path(&ns))?,
             store,
             ns,
             id_state: None,
         })
     }
+
+    pub(crate) fn ok(self) -> ListGroupsOk<Self> {
+        ListGroupsOk::new(self)
+    }
 }
 
 impl Iterator for ListGroups {
@@ -93,43 +166,14 @@ impl Iterator for ListGroups {
 
     fn next(&mut self) -> Option<Self::Item> {
         loop {
-            if let Some((group_type, ref mut id_fd)) = self.id_state {
-                let item = match id_fd.next() {
-                    Some(item) => item,
+            if let Some(ref mut id_iter) = self.id_state {
+                match id_iter.next() {
+                    Some(item) => return Some(item),
                     None => {
                         self.id_state = None;
-                        continue; // exhausted all IDs for the current group type, try others
-                    }
-                };
-                let entry = match item {
-                    Ok(ref entry) => {
-                        match entry.file_type() {
-                            Some(nix::dir::Type::Directory) => entry, // OK
-                            None => match get_file_type(entry.parent_fd(), entry.file_name()) {
-                                Ok(nix::dir::Type::Directory) => entry,
-                                Ok(_) => continue,
-                                Err(err) => {
-                                    log::info!(
-                                        "error listing groups for {}: {err}",
-                                        self.store.name()
-                                    );
-                                    continue;
-                                }
-                            },
-                            _ => continue,
-                        }
+                        // exhausted all IDs for the current group type, try others
                     }
-                    Err(err) => return Some(Err(err)),
                 };
-                if let Ok(name) = entry.file_name().to_str() {
-                    if BACKUP_ID_REGEX.is_match(name) {
-                        return Some(Ok(BackupGroup::new(
-                            Arc::clone(&self.store),
-                            self.ns.clone(),
-                            (group_type, name.to_owned()).into(),
-                        )));
-                    }
-                }
             } else {
                 let item = self.type_fd.next()?;
                 let entry = match item {
@@ -153,16 +197,20 @@ impl Iterator for ListGroups {
                     }
                     Err(err) => return Some(Err(err)),
                 };
+
                 if let Ok(name) = entry.file_name().to_str() {
                     if let Ok(group_type) = BackupType::from_str(name) {
                         // found a backup group type, descend into it to scan all IDs in it
                         // by switching to the id-state branch
-                        let base_fd = entry.parent_fd();
-                        let id_dirfd = match proxmox_sys::fs::read_subdir(base_fd, name) {
-                            Ok(dirfd) => dirfd,
+                        match ListGroupsType::new_at(
+                            entry.parent_fd(),
+                            Arc::clone(&self.store),
+                            self.ns.clone(),
+                            group_type,
+                        ) {
+                            Ok(ty) => self.id_state = Some(ty),
                             Err(err) => return Some(Err(err.into())),
-                        };
-                        self.id_state = Some((group_type, id_dirfd));
+                        }
                     }
                 }
             }
@@ -170,6 +218,61 @@ impl Iterator for ListGroups {
     }
 }
 
+pub(crate) trait GroupIter {
+    fn store_name(&self) -> &str;
+}
+
+impl GroupIter for ListGroups {
+    fn store_name(&self) -> &str {
+        self.store.name()
+    }
+}
+
+impl GroupIter for ListGroupsType {
+    fn store_name(&self) -> &str {
+        self.store.name()
+    }
+}
+
+pub(crate) struct ListGroupsOk<I>(Option<I>)
+where
+    I: GroupIter + Iterator<Item = Result<BackupGroup, Error>>;
+
+impl<I> ListGroupsOk<I>
+where
+    I: GroupIter + Iterator<Item = Result<BackupGroup, Error>>,
+{
+    fn new(inner: I) -> Self {
+        Self(Some(inner))
+    }
+}
+
+impl<I> Iterator for ListGroupsOk<I>
+where
+    I: GroupIter + Iterator<Item = Result<BackupGroup, Error>>,
+{
+    type Item = BackupGroup;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if let Some(iter) = &mut self.0 {
+            match iter.next() {
+                Some(Ok(item)) => return Some(item),
+                Some(Err(err)) => {
+                    log::error!(
+                        "list groups error on datastore {} - {}",
+                        iter.store_name(),
+                        err
+                    );
+                }
+                None => (),
+            }
+
+            self.0 = None;
+        }
+        None
+    }
+}
+
 /// A iterator for a (single) level of Namespaces
 pub struct ListNamespaces {
     ns: BackupNamespace,

diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
index 3eeaaa4e69e80a5698a3ea9b37cd9b7d796d4aad..b09fd114ae25d6ab66668ac64991ffd4f6796f21 100644 (file)
--- a/pbs-datastore/src/lib.rs
+++ b/pbs-datastore/src/lib.rs
@@ -209,7 +209,9 @@ mod datastore;
 pub use datastore::{check_backup_owner, DataStore};
 
 mod hierarchy;
-pub use hierarchy::{ListGroups, ListNamespaces, ListNamespacesRecursive, ListSnapshots};
+pub use hierarchy::{
+    ListGroups, ListGroupsType, ListNamespaces, ListNamespacesRecursive, ListSnapshots,
+};
 
 mod snapshot_reader;
 pub use snapshot_reader::SnapshotReader;