From 4d5e14c07e9d314a117920970850e0b8d5efd7a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
Date: Wed, 27 Nov 2024 15:11:27 +0100
Subject: [PATCH] datastore: extract nesting check into helper
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 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 d3876838..ddd8d3c6 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 7c087d9f..d8bae207 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();