From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 4BA971FF0AA for ; Tue, 06 Oct 2026 16:47:42 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 1F12221678; Tue, 06 Oct 2026 16:47:12 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v5 12/14] pbs-config: add helper to check new datastore config sections Date: Tue, 6 Oct 2026 16:46:42 +0200 Message-ID: <20261006144644.744818-13-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261006144644.744818-1-c.ebner@proxmox.com> References: <20261006144644.744818-1-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791298027334 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.587 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 67GOXCO4BAGTDZIWPPMSH3LHFJBFXO6P X-Message-ID-Hash: 67GOXCO4BAGTDZIWPPMSH3LHFJBFXO6P X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Factor out the datastore config check so this can be done independent from the datastore creation, which is going to happen without holding the datastore config lock but rather within the maintenance type create. While moving the code, unify it with other call sites to have all checks performed in a central location. The nesting check is now no longer performed within the datastore creation task but rather early on and for datastore creations combined with storage from disks as well. Signed-off-by: Christian Ebner --- pbs-config/src/datastore.rs | 45 +++++++++++++++++++++++++++++++- src/api2/config/datastore.rs | 40 ++++------------------------ src/api2/node/disks/directory.rs | 5 +--- src/api2/node/disks/zfs.rs | 5 +--- 4 files changed, 51 insertions(+), 44 deletions(-) diff --git a/pbs-config/src/datastore.rs b/pbs-config/src/datastore.rs index 2a200b35e..de397302f 100644 --- a/pbs-config/src/datastore.rs +++ b/pbs-config/src/datastore.rs @@ -1,10 +1,11 @@ use std::collections::HashMap; +use std::path::Path; use std::sync::LazyLock; use anyhow::Error; use proxmox_product_config::replace_privileged_config; -use proxmox_schema::{AllOfSchema, ApiType}; +use proxmox_schema::{AllOfSchema, ApiType, param_bail}; use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin}; use pbs_api_types::{DATASTORE_SCHEMA, DataStoreConfig, DatastoreBackendConfig, DatastoreTuning}; @@ -132,3 +133,45 @@ pub fn datastore_backend_type(store: &str) -> Result Result<(), Error> { + if config.sections.contains_key(&store.name) { + param_bail!("name", "datastore '{}' already exists.", store.name); + } + + let path = Path::new(&store.path); + if store.backing_device.is_none() { + if path.is_relative() { + param_bail!("path", "expected an absolute path, '{}' is not", store.path); + } + if path.parent().is_none() { + param_bail!("path", "cannot create datastore in root path",); + } + if store.gc_on_unmount.unwrap_or(false) { + param_bail!( + "gc-on-unmount", + "GC on unmount is only supported on removable datastores", + ); + } + } else { + if path.is_absolute() { + param_bail!( + "path", + "expected a relative on-device path, '{}' is not", + store.path + ); + } + } + + let existing_stores = config.convert_to_typed_array("datastore")?; + if let Err(err) = store.ensure_not_nested(&existing_stores) { + param_bail!("path", err); + } + + Ok(()) +} diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index 74118de92..7505e8c78 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; use ::serde::{Deserialize, Serialize}; -use anyhow::{Context, Error, bail, format_err}; +use anyhow::{Context, Error, format_err}; use serde_json::Value; use tracing::warn; @@ -93,26 +93,17 @@ impl Drop for UnmountGuard { } pub(crate) fn do_create_datastore( - _lock: BackupLockGuard, + lock: BackupLockGuard, mut config: SectionConfigData, mut datastore: DataStoreConfig, reuse_datastore: bool, overwrite_in_use: bool, ) -> Result<(), Error> { - let path: PathBuf = datastore.absolute_path().into(); - - if path.parent().is_none() && datastore.backing_device.is_none() { - bail!("cannot create datastore in root path"); - } - - let existing_stores = config.convert_to_typed_array("datastore")?; - if let Err(err) = datastore.ensure_not_nested(&existing_stores) { - param_bail!("path", err); - } + pbs_config::datastore::validate_new_datastore_config(&datastore, &config, &lock)?; let unmount_guard = if datastore.backing_device.is_some() { do_mount_device(datastore.clone())?; - UnmountGuard::new(Some(path.clone())) + UnmountGuard::new(Some(datastore.absolute_path().into())) } else { UnmountGuard::new(None) }; @@ -182,28 +173,7 @@ pub fn create_datastore( let lock = pbs_config::datastore::lock_config()?; let (section_config, _digest) = pbs_config::datastore::config()?; - - if section_config.sections.contains_key(&config.name) { - param_bail!("name", "datastore '{}' already exists.", config.name); - } - - if config.backing_device.is_none() && !config.path.starts_with("/") { - param_bail!( - "path", - "expected an absolute path, '{}' is not", - config.path - ); - } - if config.backing_device.is_some() && config.path.starts_with("/") { - param_bail!( - "path", - "expected a relative on-device path, '{}' is not", - config.path - ); - } - if config.backing_device.is_none() && config.gc_on_unmount.unwrap_or(false) { - param_bail!("gc-on-unmount", "only supported on removable datastores",); - } + pbs_config::datastore::validate_new_datastore_config(&config, §ion_config, &lock)?; let mut prune_job_config = None; if config.keep.keeps_something() || !has_prune_job(&config.name)? { diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs index 573e11744..9979355e5 100644 --- a/src/api2/node/disks/directory.rs +++ b/src/api2/node/disks/directory.rs @@ -248,10 +248,7 @@ pub fn create_datastore_disk( serde_json::from_value(json!({ "name": name, "path": mount_point }))? }; let (config, _digest) = pbs_config::datastore::config()?; - - if config.sections.contains_key(&datastore.name) { - bail!("datastore '{}' already exists.", datastore.name); - } + pbs_config::datastore::validate_new_datastore_config(&datastore, &config, &lock)?; crate::api2::config::datastore::do_create_datastore( lock, config, datastore, false, false, diff --git a/src/api2/node/disks/zfs.rs b/src/api2/node/disks/zfs.rs index 7a62e3f70..0e8ce75b5 100644 --- a/src/api2/node/disks/zfs.rs +++ b/src/api2/node/disks/zfs.rs @@ -308,10 +308,7 @@ pub fn create_zpool( serde_json::from_value(json!({ "name": name, "path": mount_point }))?; let (config, _digest) = pbs_config::datastore::config()?; - - if config.sections.contains_key(&datastore.name) { - bail!("datastore '{}' already exists.", datastore.name); - } + pbs_config::datastore::validate_new_datastore_config(&datastore, &config, &lock)?; crate::api2::config::datastore::do_create_datastore( lock, config, datastore, false, false, -- 2.47.3