[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 at 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