[pbs-devel] [PATCH v2 proxmox-backup 0/2] api: disks: directory: fail if mount unit already exists

[pbs-devel] [PATCH v2 proxmox-backup 0/2] api: disks: directory: fail if mount unit already exists

[Fiona Ebner]

Changes in v2:
* style changes

Protect against overriding a pre-existing mount unit during creation
of a directory (datastore). The unit might belong to an existing
datastore.

Fiona Ebner (2):
  api: disks: directory: factor out helper for mount unit path
  api: disks: directory: fail if mount unit already exists

 src/api2/node/disks/directory.rs | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

-- 
2.39.5



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

[pbs-devel] [PATCH v2 proxmox-backup 1/2] api: disks: directory: factor out helper for mount unit path

[Fiona Ebner]

In preparation to check for a pre-existing mount unit.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
---

Changes in v2:
* inline variable name in format!()

 src/api2/node/disks/directory.rs | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 6a76dd5a..57add02b 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -324,16 +324,23 @@ pub const ROUTER: Router = Router::new()
     .post(&API_METHOD_CREATE_DATASTORE_DISK)
     .match_all("name", &ITEM_ROUTER);
 
+fn datastore_mount_unit_path_info(mount_point: &str) -> (String, String) {
+    let mut mount_unit_name = proxmox_systemd::escape_unit(mount_point, true);
+    mount_unit_name.push_str(".mount");
+
+    (
+        format!("/etc/systemd/system/{mount_unit_name}"),
+        mount_unit_name,
+    )
+}
+
 fn create_datastore_mount_unit(
     datastore_name: &str,
     mount_point: &str,
     fs_type: FileSystemType,
     what: &str,
 ) -> Result<String, Error> {
-    let mut mount_unit_name = proxmox_systemd::escape_unit(mount_point, true);
-    mount_unit_name.push_str(".mount");
-
-    let mount_unit_path = format!("/etc/systemd/system/{}", mount_unit_name);
+    let (mount_unit_path, mount_unit_name) = datastore_mount_unit_path_info(mount_point);
 
     let unit = SystemdUnitSection {
         Description: format!(
-- 
2.39.5



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

[pbs-devel] [PATCH v2 proxmox-backup 2/2] api: disks: directory: fail if mount unit already exists

[Fiona Ebner]

Without this check, if a mount unit is present, but the file system is
not mounted, it will just get overwritten. The unit might belong to an
existing datastore.

There already is a check against a duplicate datastore, but only after
the mount unit is already overwritten and having the add-datastore
flag present is not a precondition to trigger the issue.

The check is done even if the newly created directory datastore is
removable. While in that case, the mount unit is not overwritten, the
conflict for the mount point is still present, so it is nice to fail
early.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* don't rely on debug trait for printing error message

 src/api2/node/disks/directory.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 57add02b..62f46343 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -204,6 +204,11 @@ pub fn create_datastore_disk(
         }
     }
 
+    let (mount_unit_path, _) = datastore_mount_unit_path_info(&mount_point);
+    if std::path::PathBuf::from(&mount_unit_path).exists() {
+        bail!("systemd mount unit '{mount_unit_path}' already exists");
+    }
+
     let upid_str = WorkerTask::new_thread(
         "dircreate",
         Some(name.clone()),
-- 
2.39.5



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

Re: [pbs-devel] [PATCH v2 proxmox-backup 0/2] api: disks: directory: fail if mount unit already exists

[Thomas Lamprecht]

Am 27.11.24 um 17:06 schrieb Fiona Ebner:
> Changes in v2:
> * style changes
> 
> Protect against overriding a pre-existing mount unit during creation
> of a directory (datastore). The unit might belong to an existing
> datastore.
> 
> Fiona Ebner (2):
>   api: disks: directory: factor out helper for mount unit path
>   api: disks: directory: fail if mount unit already exists
> 
>  src/api2/node/disks/directory.rs | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 

applied series with a detour by first applying and pushing out v1, which
I should have questioned, and then merging in the actual v2 to show what
happened for real.


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