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 1B7791FF125 for ; Mon, 03 Aug 2026 11:08:36 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5118621592; Mon, 03 Aug 2026 11:08:10 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v4 12/14] pbs-config: add helper to check new datastore config sections Date: Mon, 3 Aug 2026 11:07:45 +0200 Message-ID: <20260803090747.265683-13-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260803090747.265683-1-c.ebner@proxmox.com> References: <20260803090747.265683-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: 1785748075289 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.131 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_LOW -0.7 Sender listed at https://www.dnswl.org/, low 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: 7LRYGYFFRCFHMWOVCSRLBGQJYEJW75OH X-Message-ID-Hash: 7LRYGYFFRCFHMWOVCSRLBGQJYEJW75OH 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