From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id C511D1FF0B2 for ; Fri, 09 Oct 2026 12:43:25 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 64BD021350; Fri, 09 Oct 2026 12:43:25 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 09 Oct 2026 12:43:20 +0200 Message-Id: From: "Shan Shaji" To: "Christian Ebner" , Subject: Re: [PATCH proxmox-backup v5 14/14] datastore: protect datastore creation by maintenance-mode X-Mailer: aerc 0.20.0 References: <20261006144644.744818-1-c.ebner@proxmox.com> <20261006144644.744818-15-c.ebner@proxmox.com> In-Reply-To: <20261006144644.744818-15-c.ebner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791542601007 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.433 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: L6MEAY5RLDIHUZKOCIRMLVF22PU2QFCN X-Message-ID-Hash: L6MEAY5RLDIHUZKOCIRMLVF22PU2QFCN X-MailFrom: s.shaji@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: Hi Chris, I have got an error when I tried to create a datastore with --reuse-datast= ore 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 maintena= nce-mode lock: Unable to acquire lock "/run/proxmox-backup/locks/s3-store/m= aintenance-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. 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, RpcEnvironmentT= ype, 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, D= atastoreBackendType, > - DatastoreNotify, KeepOptions, MaintenanceMode, PRIV_DATASTORE_ALLOCA= TE, 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, PROXMO= X_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_m= ode_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, &co= nfig, &lock)?; > + // always start out in maintenance-mode create, so access can be blo= cked during creation > + // without needing to hold the config lock, only the maintenance-mod= e lock. > + let maintenance_mode_lock =3D 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 =3D MaintenanceType::Create; > + set_maintenance_type(lock, &maintenance_mode_lock, &mut datastore, c= urrent_type)?; > > let unmount_guard =3D 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, ov= erwrite_in_use)?; > - > - config.set_data(&datastore.name, "datastore", &datastore)?; > - > - pbs_config::datastore::save_config(&config)?; > + DataStore::construct_chunk_store(&datastore, reuse_datastore, overwr= ite_in_use)?; > > jobstate::create_state_file("garbage_collection", &datastore.name)?; > > unmount_guard.disable(); > > - drop(lock); > - > if let Some(prune_job_config) =3D prune_job_config { > do_create_prune_job(prune_job_config)?; > } > > if reuse_datastore && backend_type =3D=3D DatastoreBackendType::S3 { > + // update maintenance mode to s3-refresh, so can drop config loc= k again and no other > + // operation will start until done with the refresh. > + let lock =3D pbs_config::datastore::lock_config()?; > + current_type =3D MaintenanceType::S3Refresh; > + set_maintenance_type(lock, &maintenance_mode_lock, &mut datastor= e, current_type)?; > + > crate::api2::admin::datastore::do_s3_refresh(&datastore.name, wo= rker)?; 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]