[pbs-devel] [PATCH proxmox-backup v2] api: tape: don't allow overwriting of ids in changer/drive config

[pbs-devel] [PATCH proxmox-backup v2] api: tape: don't allow overwriting of ids in changer/drive config

[Dominik Csapak]

by checking the whole section config for an existing id, not only the
ones of the given type.

This prevents creation of a drive config with the same name as an
existing changer and vice versa, as it is confusing that existing things
get deleted, and we can get in the situation that we reference a changer
that does not exist anymore, i.e. consider this:

* create a changer with name `foo`
* create a drive with name `foo` and select changer `foo` for it

this would delete the changer config, but still reference it, leading
to errors when trying to use it.

We could implement support for separate id namespaces in section configs
for different types, but this is much more easier to do and be enough
for now.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
If we decide to implement different id namespaces in the section config
later, then this commit can be reverted.

 src/api2/config/changer.rs | 8 ++++----
 src/api2/config/drive.rs   | 7 ++++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/api2/config/changer.rs b/src/api2/config/changer.rs
index db0ea14a..91286697 100644
--- a/src/api2/config/changer.rs
+++ b/src/api2/config/changer.rs
@@ -33,6 +33,10 @@ pub fn create_changer(config: ScsiTapeChanger) -> Result<(), Error> {
 
     let (mut section_config, _digest) = pbs_config::drive::config()?;
 
+    if section_config.sections.get(&config.name).is_some() {
+        param_bail!("name", "Entry '{}' already exists", config.name);
+    }
+
     let linux_changers = linux_tape_changer_list();
 
     check_drive_path(&linux_changers, &config.path)?;
@@ -40,10 +44,6 @@ pub fn create_changer(config: ScsiTapeChanger) -> Result<(), Error> {
     let existing: Vec<ScsiTapeChanger> = section_config.convert_to_typed_array("changer")?;
 
     for changer in existing {
-        if changer.name == config.name {
-            param_bail!("name", "Entry '{}' already exists", config.name);
-        }
-
         if changer.path == config.path {
             param_bail!(
                 "path",
diff --git a/src/api2/config/drive.rs b/src/api2/config/drive.rs
index 02589aaf..cf620aaf 100644
--- a/src/api2/config/drive.rs
+++ b/src/api2/config/drive.rs
@@ -34,6 +34,10 @@ pub fn create_drive(config: LtoTapeDrive) -> Result<(), Error> {
 
     let (mut section_config, _digest) = pbs_config::drive::config()?;
 
+    if section_config.sections.get(&config.name).is_some() {
+        param_bail!("name", "Entry '{}' already exists", config.name);
+    }
+
     let lto_drives = lto_tape_device_list();
 
     check_drive_path(&lto_drives, &config.path)?;
@@ -41,9 +45,6 @@ pub fn create_drive(config: LtoTapeDrive) -> Result<(), Error> {
     let existing: Vec<LtoTapeDrive> = section_config.convert_to_typed_array("lto")?;
 
     for drive in existing {
-        if drive.name == config.name {
-            param_bail!("name", "Entry '{}' already exists", config.name);
-        }
         if drive.path == config.path {
             param_bail!(
                 "path",
-- 
2.39.2

[pbs-devel] applied: [PATCH proxmox-backup v2] api: tape: don't allow overwriting of ids in changer/drive config

[Thomas Lamprecht]

Am 06/03/2024 um 10:39 schrieb Dominik Csapak:
> by checking the whole section config for an existing id, not only the
> ones of the given type.
> 
> This prevents creation of a drive config with the same name as an
> existing changer and vice versa, as it is confusing that existing things
> get deleted, and we can get in the situation that we reference a changer
> that does not exist anymore, i.e. consider this:
> 
> * create a changer with name `foo`
> * create a drive with name `foo` and select changer `foo` for it
> 
> this would delete the changer config, but still reference it, leading
> to errors when trying to use it.
> 
> We could implement support for separate id namespaces in section configs
> for different types, but this is much more easier to do and be enough
> for now.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> If we decide to implement different id namespaces in the section config
> later, then this commit can be reverted.
> 
>  src/api2/config/changer.rs | 8 ++++----
>  src/api2/config/drive.rs   | 7 ++++---
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
>

applied, thanks!


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