public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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]




  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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal