From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7A45A97C80 for ; Wed, 6 Mar 2024 10:40:17 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 52353147C8 for ; Wed, 6 Mar 2024 10:39:47 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 6 Mar 2024 10:39:46 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 11135434E2 for ; Wed, 6 Mar 2024 10:39:46 +0100 (CET) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Wed, 6 Mar 2024 10:39:45 +0100 Message-Id: <20240306093945.1463987-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.019 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH proxmox-backup v2] api: tape: don't allow overwriting of ids in changer/drive config X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Mar 2024 09:40:17 -0000 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 --- 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 = 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(<o_drives, &config.path)?; @@ -41,9 +45,6 @@ pub fn create_drive(config: LtoTapeDrive) -> Result<(), Error> { let existing: Vec = 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