From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 6455C1FF13B for ; Wed, 06 May 2026 18:57:51 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 438681FA8; Wed, 6 May 2026 18:57:51 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v2 03/10] api/datastore: refactor datastore creation helper logic Date: Wed, 6 May 2026 18:56:44 +0200 Message-ID: <20260506165651.1322947-4-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260506165651.1322947-1-c.ebner@proxmox.com> References: <20260506165651.1322947-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: 1778086519058 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.070 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 Message-ID-Hash: 5V2LXW5SJVK7I7HOR7HKPYIJ6FQVR73B X-Message-ID-Hash: 5V2LXW5SJVK7I7HOR7HKPYIJ6FQVR73B 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: Move most of the logic from the datastore creation worker task helper to the datastore implementation itself. This has the advantage of better encapsulation, reducing the interdependencies. Signed-off-by: Christian Ebner --- pbs-datastore/src/datastore.rs | 131 ++++++++++++++++++++++++++++++++- src/api2/config/datastore.rs | 123 ++----------------------------- 2 files changed, 136 insertions(+), 118 deletions(-) diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 34efcd398..84314cd29 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -19,7 +19,7 @@ use tracing::{info, warn}; use proxmox_human_byte::HumanByte; use proxmox_s3_client::{ RequestCounterThresholds, S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3PathPrefix, - S3RateLimiterOptions, S3RequestCounterConfig, SharedRequestCounters, + S3RateLimiterOptions, S3RequestCounterConfig, SharedRequestCounters, S3_HTTP_REQUEST_TIMEOUT, }; use proxmox_schema::ApiType; @@ -388,6 +388,17 @@ impl DatastoreThreadSettings { } } +#[derive(Default, serde::Deserialize, serde::Serialize)] +#[serde(rename_all = "kebab-case")] +/// S3 in-use marker content +/// +/// Allows to de-/serialize the contents of the in-use object contents stored on S3 backends to +/// protect against access by multiple PBS instances. +struct InUseContent { + #[serde(skip_serializing_if = "Option::is_none")] + hostname: Option, +} + /// Returns the parsed datastore config (`datastore.cfg`) and its /// generation. /// @@ -464,6 +475,124 @@ impl DataStore { }) } + /// Create or reuse the chunk store from given datastore configuration + pub fn construct_chunk_store( + store_config: &mut DataStoreConfig, + tuning: &DatastoreTuning, + reuse_existing: bool, + overwrite_in_use: bool, + ) -> Result<(), Error> { + let (backend_type, backend_s3_client) = + match DataStore::s3_client_and_backend_from_datastore_config(store_config)? { + (backend_type, Some(s3_client)) => { + if !overwrite_in_use { + let object_key = S3ObjectKey::try_from(S3_DATASTORE_IN_USE_MARKER) + .context("failed to generate s3 object key")?; + if let Some(response) = proxmox_async::runtime::block_on( + s3_client.get_object(object_key.clone()), + ) + .context("failed to get in-use marker from bucket")? + { + let content = + proxmox_async::runtime::block_on(response.content.collect()) + .unwrap_or_default(); + let content = + String::from_utf8(content.to_bytes().to_vec()).unwrap_or_default(); + let in_use: InUseContent = + serde_json::from_str(&content).unwrap_or_default(); + if let Some(hostname) = in_use.hostname { + bail!( + "Bucket already contains datastore in use by host {hostname}" + ); + } else { + bail!("Bucket already contains datastore in use"); + } + } + } + + (backend_type, Some(Arc::new(s3_client))) + } + (backend_type, None) => (backend_type, None), + }; + + let path: PathBuf = store_config.absolute_path().into(); + + let chunk_store = if reuse_existing && backend_type == DatastoreBackendType::Filesystem { + ChunkStore::verify_chunkstore(&path).and_then(|_| { + // Must be the only instance accessing and locking the chunk store, + // dropping will close all other locks from this process on the lockfile as well. + ChunkStore::open( + &store_config.name, + &path, + tuning.sync_level.unwrap_or_default(), + ) + })? + } else { + if !reuse_existing && backend_type == DatastoreBackendType::Filesystem { + if let Ok(dir) = std::fs::read_dir(&path) { + for file in dir { + let name = file?.file_name(); + let name = name.to_str(); + if !name.is_some_and(|name| name.starts_with('.') || name == "lost+found") { + bail!("datastore path not empty"); + } + } + } + } + if reuse_existing && backend_type == DatastoreBackendType::S3 { + let chunks_path = path.join(".chunks"); + if let Err(err) = std::fs::remove_dir_all(&chunks_path) { + if err.kind() != std::io::ErrorKind::NotFound { + return Err(err).with_context(|| { + format!("failed to remove pre-existing chunks in {chunks_path:?}") + }); + } + } + // starting out in maintenance mode s3-refresh, + // so no other operation will start until done with that. + store_config.set_maintenance_mode(Some(MaintenanceMode { + ty: MaintenanceType::S3Refresh, + message: None, + }))?; + } + let backup_user = pbs_config::backup_user()?; + ChunkStore::create( + &store_config.name, + path.clone(), + backup_user.uid, + backup_user.gid, + tuning.sync_level.unwrap_or_default(), + )? + }; + + if let Some(ref s3_client) = backend_s3_client { + let object_key = S3ObjectKey::try_from(S3_DATASTORE_IN_USE_MARKER) + .context("failed to generate s3 object key")?; + let content = serde_json::to_string(&InUseContent { + hostname: Some(proxmox_sys::nodename().to_string()), + }) + .context("failed to encode hostname")?; + proxmox_async::runtime::block_on(s3_client.put_object( + object_key, + hyper::body::Bytes::from(content).into(), + Some(S3_HTTP_REQUEST_TIMEOUT), + true, + )) + .context("failed to upload in-use marker for datastore")?; + } + + if tuning.gc_atime_safety_check.unwrap_or(true) { + chunk_store + .check_fs_atime_updates(true, backend_s3_client) + .context("access time safety check failed")?; + info!("Access time update check successful."); + } else { + info!("Access time update check skipped."); + } + + Ok(()) + } + /// Get the backend for this datastore based on it's configuration pub fn backend(&self) -> Result { let backend = match self.backend_type() { diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index 7c3531a4b..70e349b2f 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -1,11 +1,9 @@ use std::path::{Path, PathBuf}; -use std::sync::Arc; use ::serde::{Deserialize, Serialize}; use anyhow::{bail, format_err, Context, Error}; -use http_body_util::BodyExt; use serde_json::Value; -use tracing::{info, warn}; +use tracing::warn; use proxmox_router::{http_bail, Permission, Router, RpcEnvironment, RpcEnvironmentType}; use proxmox_schema::{api, param_bail, ApiType}; @@ -14,12 +12,11 @@ use proxmox_uuid::Uuid; use pbs_api_types::{ Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreBackendType, DatastoreNotify, - DatastoreTuning, KeepOptions, MaintenanceMode, MaintenanceType, PruneJobConfig, - PruneJobOptions, DATASTORE_SCHEMA, PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, - PRIV_DATASTORE_MODIFY, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA, + DatastoreTuning, KeepOptions, MaintenanceMode, PruneJobConfig, PruneJobOptions, + DATASTORE_SCHEMA, PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, + PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA, }; use pbs_config::BackupLockGuard; -use pbs_datastore::chunk_store::ChunkStore; use crate::api2::admin::datastore::do_mount_device; use crate::api2::admin::prune::list_prune_jobs; @@ -31,20 +28,12 @@ use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_bac use crate::api2::config::verify::delete_verification_job; use pbs_config::CachedUserInfo; -use pbs_datastore::{get_datastore_mount_status, DataStore, S3_DATASTORE_IN_USE_MARKER}; +use pbs_datastore::{get_datastore_mount_status, DataStore}; use proxmox_rest_server::WorkerTask; -use proxmox_s3_client::{S3ObjectKey, S3_HTTP_REQUEST_TIMEOUT}; use crate::server::jobstate; use crate::tools::disks::unmount_by_mountpoint; -#[derive(Default, serde::Deserialize, serde::Serialize)] -#[serde(rename_all = "kebab-case")] -struct InUseContent { - #[serde(skip_serializing_if = "Option::is_none")] - hostname: Option, -} - #[api( input: { properties: {}, @@ -133,107 +122,7 @@ pub(crate) fn do_create_datastore( UnmountGuard::new(None) }; - let (backend_type, backend_s3_client) = - match DataStore::s3_client_and_backend_from_datastore_config(&datastore)? { - (backend_type, Some(s3_client)) => { - if !overwrite_in_use { - let object_key = S3ObjectKey::try_from(S3_DATASTORE_IN_USE_MARKER) - .context("failed to generate s3 object key")?; - if let Some(response) = - proxmox_async::runtime::block_on(s3_client.get_object(object_key.clone())) - .context("failed to get in-use marker from bucket")? - { - let content = proxmox_async::runtime::block_on(response.content.collect()) - .unwrap_or_default(); - let content = - String::from_utf8(content.to_bytes().to_vec()).unwrap_or_default(); - let in_use: InUseContent = - serde_json::from_str(&content).unwrap_or_default(); - if let Some(hostname) = in_use.hostname { - bail!("Bucket already contains datastore in use by host {hostname}"); - } else { - bail!("Bucket already contains datastore in use"); - } - } - } - - (backend_type, Some(Arc::new(s3_client))) - } - (backend_type, None) => (backend_type, None), - }; - - let chunk_store = if reuse_datastore && backend_type == DatastoreBackendType::Filesystem { - ChunkStore::verify_chunkstore(&path).and_then(|_| { - // Must be the only instance accessing and locking the chunk store, - // dropping will close all other locks from this process on the lockfile as well. - ChunkStore::open( - &datastore.name, - &path, - tuning.sync_level.unwrap_or_default(), - ) - })? - } else { - if !reuse_datastore && backend_type == DatastoreBackendType::Filesystem { - if let Ok(dir) = std::fs::read_dir(&path) { - for file in dir { - let name = file?.file_name(); - let name = name.to_str(); - if !name.is_some_and(|name| name.starts_with('.') || name == "lost+found") { - bail!("datastore path not empty"); - } - } - } - } - if reuse_datastore && backend_type == DatastoreBackendType::S3 { - let chunks_path = path.join(".chunks"); - if let Err(err) = std::fs::remove_dir_all(&chunks_path) { - if err.kind() != std::io::ErrorKind::NotFound { - return Err(err).with_context(|| { - format!("failed to remove pre-existing chunks in {chunks_path:?}") - }); - } - } - // starting out in maintenance mode s3-refresh, - // so no other operation will start until done with that. - datastore.set_maintenance_mode(Some(MaintenanceMode { - ty: MaintenanceType::S3Refresh, - message: None, - }))?; - } - let backup_user = pbs_config::backup_user()?; - ChunkStore::create( - &datastore.name, - path.clone(), - backup_user.uid, - backup_user.gid, - tuning.sync_level.unwrap_or_default(), - )? - }; - - if let Some(ref s3_client) = backend_s3_client { - let object_key = S3ObjectKey::try_from(S3_DATASTORE_IN_USE_MARKER) - .context("failed to generate s3 object key")?; - let content = serde_json::to_string(&InUseContent { - hostname: Some(proxmox_sys::nodename().to_string()), - }) - .context("failed to encode hostname")?; - proxmox_async::runtime::block_on(s3_client.put_object( - object_key, - hyper::body::Bytes::from(content).into(), - Some(S3_HTTP_REQUEST_TIMEOUT), - true, - )) - .context("failed to upload in-use marker for datastore")?; - } - - if tuning.gc_atime_safety_check.unwrap_or(true) { - chunk_store - .check_fs_atime_updates(true, backend_s3_client) - .context("access time safety check failed")?; - info!("Access time update check successful."); - } else { - info!("Access time update check skipped."); - } + DataStore::construct_chunk_store(&mut datastore, &tuning, reuse_datastore, overwrite_in_use)?; config.set_data(&datastore.name, "datastore", &datastore)?; -- 2.47.3