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 4576D1FF0B0 for ; Fri, 09 Oct 2026 13:31:10 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 9480E2133F; Fri, 09 Oct 2026 13:31:09 +0200 (CEST) Message-ID: <79a1a751-acdb-4d8b-965b-b6b6ee3d2e41@proxmox.com> Date: Fri, 9 Oct 2026 13:31:03 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup v5 14/14] datastore: protect datastore creation by maintenance-mode To: Shan Shaji , pbs-devel@lists.proxmox.com References: <20261006144644.744818-1-c.ebner@proxmox.com> <20261006144644.744818-15-c.ebner@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791545463911 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.568 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: 2HVORHD6J3WTP7RDOBV2DKINM4UFMLYK X-Message-ID-Hash: 2HVORHD6J3WTP7RDOBV2DKINM4UFMLYK 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: On 10/9/26 12:43 PM, Shan Shaji wrote: > Hi Chris, > > I have got an error when I tried to create a datastore with --reuse-datastore > option set as true. > > ``` > TASK ERROR: unable to acquire exclusive datastore's maintenance-mode lock: Unable to acquire lock "/run/proxmox-backup/locks/s3-store/maintenance-mode.lck" - Resource temporarily unavailable (os error 11) > Error: task failed (status unable to acquire exclusive datastore's maintenance-mode lock: Unable to acquire lock "/run/proxmox-backup/locks/s3-store/maintenance-mode.lck" - Resource temporarily unavailable (os error 11)) > ``` > > AFAIU, the lock from the creation phase is never dropped before the > s3-refresh call. I think it is creating a deadlock situation, as we > are trying to acquire already held resource again. But, I could have > missed something here. I have marked the code paths. Please check the > inline comments. > > If I create the datastore without the flag, it works fine. Thanks for testing and catching this! The maintenance mode lock must however be held for the whole datastore creation phase, including the s3 refresh, it cannot be dropped per-maturely. Should be easily fixable though by getting the datastore instance and calling proxmox_async::runtime::block_on(datastore.s3_refresh()) instead of doing do_s3_refresh(), as the latter is just the wrapper to set the maintenance mode. Will double check and send a new version if no other issues surface. > On Tue Oct 6, 2026 at 4:46 PM CEST, Christian Ebner wrote: >> Instead of holding the lock for the whole datastore creation, use the >> maintenance-mode `create`, protected by the maintenance-mode lock. >> It is inserted and written to the config when setting the maintenance >> mode before even starting any datastore related operation. Cleanup >> from config if creation failed must be performed manually. >> >> This has the advantage that a potentially long running creation does >> not block any concurrent access to the datastore config, thereby >> blocking also any unrelated datastore. >> >> The config has to be re-parsed a second time for this, but given that >> this unblocks the config and this is no performance critical path >> that tradeoff is better than its alternative. By refactoring the >> helpers to set and unset the maintenance mode, they can be reused for >> all call sites. >> >> Rely on re-acquisition of the lock to also succeed if other operations >> are being performed, due to the 10s timeout. >> >> Ordering of locking cannot always be preserved, but deadlocks are >> excluded by timeouts. >> >> Signed-off-by: Christian Ebner >> --- > > [snip] > >> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs >> index 5d0aa966a..57d256df4 100644 >> --- a/src/api2/config/datastore.rs >> +++ b/src/api2/config/datastore.rs >> @@ -7,17 +7,15 @@ use tracing::warn; >> >> use proxmox_router::{Permission, Router, RpcEnvironment, RpcEnvironmentType, http_bail}; >> use proxmox_schema::{ApiType, api, param_bail}; >> -use proxmox_section_config::SectionConfigData; >> use proxmox_uuid::Uuid; >> use proxmox_worker_task::WorkerTaskContext; >> >> use pbs_api_types::{ >> Authid, DATASTORE_SCHEMA, DataStoreConfig, DataStoreConfigUpdater, DatastoreBackendType, >> - DatastoreNotify, KeepOptions, MaintenanceMode, PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, >> - PRIV_DATASTORE_MODIFY, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, PruneJobConfig, >> - PruneJobOptions, UPID_SCHEMA, >> + DatastoreNotify, KeepOptions, MaintenanceMode, MaintenanceType, PRIV_DATASTORE_ALLOCATE, >> + PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, >> + PruneJobConfig, PruneJobOptions, UPID_SCHEMA, >> }; >> -use pbs_config::BackupLockGuard; >> >> use crate::api2::admin::datastore::do_mount_device; >> use crate::api2::admin::prune::list_prune_jobs; >> @@ -27,11 +25,12 @@ use crate::api2::config::prune::{delete_prune_job, do_create_prune_job, has_prun >> use crate::api2::config::sync::delete_sync_job; >> use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_backup_jobs}; >> use crate::api2::config::verify::delete_verification_job; >> -use pbs_config::CachedUserInfo; >> +use pbs_config::{BackupLockGuard, CachedUserInfo}; >> >> use pbs_datastore::{DataStore, get_datastore_mount_status, maintenance_mode_lock}; >> use proxmox_rest_server::WorkerTask; >> >> +use crate::api2::helpers::{set_maintenance_type, unset_maintenance_mode}; >> use crate::server::jobstate; >> use crate::tools::disks::unmount_by_mountpoint; >> >> @@ -95,7 +94,6 @@ impl Drop for UnmountGuard { >> >> pub(crate) fn do_create_datastore( >> lock: BackupLockGuard, >> - mut config: SectionConfigData, >> worker: &dyn WorkerTaskContext, >> backend_type: DatastoreBackendType, >> mut datastore: DataStoreConfig, >> @@ -103,7 +101,11 @@ pub(crate) fn do_create_datastore( >> reuse_datastore: bool, >> overwrite_in_use: bool, >> ) -> Result<(), Error> { >> - pbs_config::datastore::validate_new_datastore_config(&datastore, &config, &lock)?; >> + // always start out in maintenance-mode create, so access can be blocked during creation >> + // without needing to hold the config lock, only the maintenance-mode lock. >> + let maintenance_mode_lock = pbs_datastore::maintenance_mode_lock(&datastore.name, &lock)?; > > The lock that we acquired here is not dropped before the s3-refresh in > the later part. > >> + let mut current_type = MaintenanceType::Create; >> + set_maintenance_type(lock, &maintenance_mode_lock, &mut datastore, current_type)?; >> >> let unmount_guard = if datastore.backing_device.is_some() { >> do_mount_device(datastore.clone())?; >> @@ -112,26 +114,28 @@ pub(crate) fn do_create_datastore( >> UnmountGuard::new(None) >> }; >> >> - DataStore::construct_chunk_store(&mut datastore, reuse_datastore, overwrite_in_use)?; >> - >> - config.set_data(&datastore.name, "datastore", &datastore)?; >> - >> - pbs_config::datastore::save_config(&config)?; >> + DataStore::construct_chunk_store(&datastore, reuse_datastore, overwrite_in_use)?; >> >> jobstate::create_state_file("garbage_collection", &datastore.name)?; >> >> unmount_guard.disable(); >> >> - drop(lock); >> - >> if let Some(prune_job_config) = prune_job_config { >> do_create_prune_job(prune_job_config)?; >> } >> >> if reuse_datastore && backend_type == DatastoreBackendType::S3 { >> + // update maintenance mode to s3-refresh, so can drop config lock again and no other >> + // operation will start until done with the refresh. >> + let lock = pbs_config::datastore::lock_config()?; >> + current_type = MaintenanceType::S3Refresh; >> + set_maintenance_type(lock, &maintenance_mode_lock, &mut datastore, current_type)?; >> + >> crate::api2::admin::datastore::do_s3_refresh(&datastore.name, worker)?; > > Inside the run_maintenance_locked method we are trying to acquire the > lock again. Since it is already held during the create phase and never > dropped. It fails to acquire it again. > >> } >> >> + unset_maintenance_mode(Some(maintenance_mode_lock), &datastore.name, current_type)?; > >> Ok(()) >> } >> > > [snip]