[pbs-devel] [PATCH proxmox-backup 1/2] datastore: extract nesting check into helper

* [pbs-devel] [PATCH proxmox-backup 1/2] datastore: extract nesting check into helper
@ 2024-11-27 14:11 Fabian Grünbichler
  2024-11-27 14:11 ` [pbs-devel] [RFC proxmox-backup 2/2] GC: add check for nested datastore Fabian Grünbichler
  2024-11-27 14:27 ` [pbs-devel] applied: [PATCH proxmox-backup 1/2] datastore: extract nesting check into helper Thomas Lamprecht
  0 siblings, 2 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2024-11-27 14:11 UTC (permalink / raw)
  To: pbs-devel

and improve the variable namign while we are at it. this allows the check to be
re-used in other code paths, like when starting a garbage collection.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    no semantic changes intended, for *after* the release

 pbs-api-types/src/datastore.rs | 39 ++++++++++++++++++++++++++++++++++
 src/api2/config/datastore.rs   | 29 +++----------------------
 2 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index d3876838b..ddd8d3c6b 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -441,6 +441,45 @@ impl DataStoreConfig {
 
         Ok(())
     }
+
+    pub fn ensure_not_nested(&self, stores: &[DataStoreConfig]) -> Result<(), Error> {
+        let our_absolute_path = PathBuf::from(self.absolute_path());
+        let removable = self.backing_device.is_some();
+        for other_store in stores {
+            if self == other_store {
+                continue;
+            };
+
+            // Relative paths must not be nested on the backing device of removable datastores
+            if removable && other_store.backing_device == self.backing_device {
+                let our_relative_path = Path::new(&self.path);
+                let other_relative_path = Path::new(&other_store.path);
+                if our_relative_path.starts_with(other_relative_path)
+                    || other_relative_path.starts_with(our_relative_path)
+                {
+                    bail!(
+                        "paths on backing device must not be nested - {path:?} already used by '{store}'!",
+                        path = other_relative_path,
+                        store = other_store.name,
+                    );
+                }
+            }
+
+            // No two datastores should have a nested absolute path
+            let other_absolute_path = PathBuf::from(other_store.absolute_path());
+            if other_absolute_path.starts_with(&our_absolute_path)
+                || our_absolute_path.starts_with(&other_absolute_path)
+            {
+                bail!(
+                    "nested datastores not allowed: '{}' already in {:?}",
+                    other_store.name,
+                    other_absolute_path,
+                );
+            }
+        }
+
+        Ok(())
+    }
 }
 
 #[api(
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 7c087d9fc..d8bae2078 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -82,32 +82,9 @@ pub(crate) fn do_create_datastore(
         bail!("cannot create datastore in root path");
     }
 
-    let new_store_path = PathBuf::from(&datastore.absolute_path());
-    let removable = datastore.backing_device.is_some();
-    for store in config.convert_to_typed_array::<DataStoreConfig>("datastore")? {
-        // Relative paths must not be nested on the backing device of removable datastores
-        if removable && store.backing_device == datastore.backing_device {
-            let new_path = Path::new(&datastore.path);
-            let path = Path::new(&store.path);
-            if new_path.starts_with(path) || path.starts_with(new_path) {
-                param_bail!(
-                    "path",
-                    "paths on backing device must not be nested - {path:?} already used by '{store}'!",
-                    store = store.name
-                );
-            }
-        }
-
-        // No two datastores should have a nested absolute path
-        let store_path = PathBuf::from(store.absolute_path());
-        if store_path.starts_with(&new_store_path) || new_store_path.starts_with(&store_path) {
-            param_bail!(
-                "path",
-                "nested datastores not allowed: '{}' already in {:?}",
-                store.name,
-                store_path,
-            );
-        }
+    let existing_stores = config.convert_to_typed_array("datastore")?;
+    if let Err(err) = datastore.ensure_not_nested(&existing_stores) {
+        param_bail!("path", err);
     }
 
     let need_unmount = datastore.backing_device.is_some();
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 4+ messages in thread