From: "Shan Shaji" <s.shaji@proxmox.com>
To: "Christian Ebner" <c.ebner@proxmox.com>, <pbs-devel@lists.proxmox.com>
Subject: Re: [PATCH proxmox-backup v5 14/14] datastore: protect datastore creation by maintenance-mode
Date: Fri, 09 Oct 2026 12:43:20 +0200 [thread overview]
Message-ID: <DM08UKW215IW.83BR9IZ3JAHI@proxmox.com> (raw)
In-Reply-To: <20261006144644.744818-15-c.ebner@proxmox.com>
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.
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 <c.ebner@proxmox.com>
> ---
[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]
next prev parent reply other threads:[~2026-10-09 10:43 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-06 14:46 [PATCH proxmox{,-backup} v5 00/14] keep datastore config unlocked during long running operations Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox v5 01/14] pbs-api-types: add datastore create maintenance-mode type Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 02/14] api: config: early perform user access checks for datastore creation Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 03/14] api: config: unlocked s3 bucket access check " Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 04/14] api: config: rearrange independent code block " Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 05/14] api/datastore: refactor datastore creation helper logic Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 06/14] datastore: restrict chunk store scope to pbs-datastore crate Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 07/14] datastore: move lock files base path constant to central location Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 08/14] datastore: move file lock helper to centralized place Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 09/14] datastore: add additional check for device number on lock helper Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 10/14] datastore: create lockdir with correct mode for backup user access Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 11/14] api/datastore: use maintenance-mode lock to protect against changes Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 12/14] pbs-config: add helper to check new datastore config sections Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 13/14] api: datastore: move prune job and s3 backend logic to create helper Christian Ebner
2026-10-06 14:46 ` [PATCH proxmox-backup v5 14/14] datastore: protect datastore creation by maintenance-mode Christian Ebner
2026-10-09 10:43 ` Shan Shaji [this message]
2026-10-09 11:31 ` Christian Ebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM08UKW215IW.83BR9IZ3JAHI@proxmox.com \
--to=s.shaji@proxmox.com \
--cc=c.ebner@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox