[pbs-devel] [PATCH proxmox-backup v3 0/7] wait for active operations to finish before s3 refresh

[pbs-devel] [PATCH proxmox-backup v3 0/7] wait for active operations to finish before s3 refresh

[Christian Ebner]

Datastore contents located on the s3 backend can be refreshed on the local
datastore cache by running an s3-refresh, which will set the maintenance mode
s3-refresh to block any read/write operation and fetch the contents to a
temporary directory before moving them in place if the fetching was successful.

Setting the maintenance mode has however no effect on already active operations,
which can cause inconsistencies once the s3 refresh moves the temporary folders
in place.

Fix this by actively waiting for ongoing read/write operations to finish before
starting with the s3-refresh and locking the datastore config while the refresh
is ongoing, so the maintenance mode cannot be altered.

Changes since version 2 (thanks @Fabian for review):
- combine previously split helpers into common one
- also guard s3-refresh during datastore recreaton

Changes since version 1 (thanks @Fabian for review):
- reuse and check for active operations based on unmount logic
- reuse and check for correct maintenance mode based on unmount logic

proxmox-backup:

Christian Ebner (7):
  api: admin: drop useless option for do_unmount_device() parameter
  api: admin: rework waiting on active operations and maintenance
    locking
  api: admin: make s3 refresh handler sync
  datastore: s3 refresh: set/unset maintenance mode in api handler
  api: datastore: wait for active operations to clear before s3 refresh
  api: config: avoid unneeded backend instance on s3 store recreation
  api: config: guard s3-refresh on datastore recreation

 pbs-datastore/src/datastore.rs |  31 +-------
 src/api2/admin/datastore.rs    | 127 +++++++++++++++++++++------------
 src/api2/config/datastore.rs   |  43 +++++------
 3 files changed, 98 insertions(+), 103 deletions(-)


Summary over all repositories:
  3 files changed, 98 insertions(+), 103 deletions(-)

-- 
Generated by git-murpp 0.8.1


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 1/7] api: admin: drop useless option for do_unmount_device() parameter

[Christian Ebner]

This helper method is always called with the worker task context
present, so do not make it an optional parameter.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in previous version

 src/api2/admin/datastore.rs | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 6e66b5cf0..935933bc5 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2579,7 +2579,7 @@ fn unset_maintenance(
 
 fn do_unmount_device(
     datastore: DataStoreConfig,
-    worker: Option<&dyn WorkerTaskContext>,
+    worker: &dyn WorkerTaskContext,
 ) -> Result<(), Error> {
     if datastore.backing_device.is_none() {
         bail!("can't unmount non-removable datastore");
@@ -2590,27 +2590,25 @@ fn do_unmount_device(
     let mut old_status = String::new();
     let mut aborted = false;
     while active_operations.read + active_operations.write > 0 {
-        if let Some(worker) = worker {
-            if worker.abort_requested()
-                || expect_maintenance_type(&datastore.name, MaintenanceType::Unmount).is_err()
-            {
-                aborted = true;
-                break;
-            }
-            let status = format!(
-                "cannot unmount yet, still {} read and {} write operations active",
-                active_operations.read, active_operations.write
-            );
-            if status != old_status {
-                info!("{status}");
-                old_status = status;
-            }
+        if worker.abort_requested()
+            || expect_maintenance_type(&datastore.name, MaintenanceType::Unmount).is_err()
+        {
+            aborted = true;
+            break;
+        }
+        let status = format!(
+            "cannot unmount yet, still {} read and {} write operations active",
+            active_operations.read, active_operations.write
+        );
+        if status != old_status {
+            info!("{status}");
+            old_status = status;
         }
         std::thread::sleep(std::time::Duration::from_secs(1));
         active_operations = task_tracking::get_active_operations(&datastore.name)?;
     }
 
-    if aborted || worker.is_some_and(|w| w.abort_requested()) {
+    if aborted || worker.abort_requested() {
         let _ = expect_maintenance_type(&datastore.name, MaintenanceType::Unmount)
             .inspect_err(|e| warn!("maintenance mode was not as expected: {e}"))
             .and_then(|(lock, config)| {
@@ -2686,7 +2684,7 @@ pub async fn unmount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<V
         Some(store),
         auth_id.to_string(),
         to_stdout,
-        move |worker| do_unmount_device(datastore, Some(&worker)),
+        move |worker| do_unmount_device(datastore, &worker),
     )?;
 
     Ok(json!(upid))
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 2/7] api: admin: rework waiting on active operations and maintenance locking

[Christian Ebner]

Move the logic to wait on no more active operations on a given
datastore and locking the datastore config into a dedicated helper
function.

While this is currently only used by the device unmount for removable
datastores, the same logic will be reused by the s3-refresh. The
helper gets a callback to be executed in locked context so the
maintenance mode cannot be altered and the status message on busy
waiting is adapted to be generic to fit both cases.

Instead of e.g.:

`cannot unmount yet, still 1 read and 1 write operations active`

the status line now states

`waiting for active operations to finsish before continuing: read 1, write 1`

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- combine previously split helpers into one

 src/api2/admin/datastore.rs | 90 +++++++++++++++++++++----------------
 1 file changed, 52 insertions(+), 38 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 935933bc5..0e81533ce 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2584,45 +2584,11 @@ fn do_unmount_device(
     if datastore.backing_device.is_none() {
         bail!("can't unmount non-removable datastore");
     }
-    let mount_point = datastore.absolute_path();
 
-    let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
-    let mut old_status = String::new();
-    let mut aborted = false;
-    while active_operations.read + active_operations.write > 0 {
-        if worker.abort_requested()
-            || expect_maintenance_type(&datastore.name, MaintenanceType::Unmount).is_err()
-        {
-            aborted = true;
-            break;
-        }
-        let status = format!(
-            "cannot unmount yet, still {} read and {} write operations active",
-            active_operations.read, active_operations.write
-        );
-        if status != old_status {
-            info!("{status}");
-            old_status = status;
-        }
-        std::thread::sleep(std::time::Duration::from_secs(1));
-        active_operations = task_tracking::get_active_operations(&datastore.name)?;
-    }
-
-    if aborted || worker.abort_requested() {
-        let _ = expect_maintenance_type(&datastore.name, MaintenanceType::Unmount)
-            .inspect_err(|e| warn!("maintenance mode was not as expected: {e}"))
-            .and_then(|(lock, config)| {
-                unset_maintenance(lock, config)
-                    .inspect_err(|e| warn!("could not reset maintenance mode: {e}"))
-            });
-        bail!("aborted, due to user request");
-    } else {
-        let (lock, config) = expect_maintenance_type(&datastore.name, MaintenanceType::Unmount)?;
-        crate::tools::disks::unmount_by_mountpoint(Path::new(&mount_point))?;
-        unset_maintenance(lock, config)
-            .map_err(|e| format_err!("could not reset maintenance mode: {e}"))?;
-    }
-    Ok(())
+    let mount_point = datastore.absolute_path();
+    run_maintenance_locked(&datastore.name, MaintenanceType::Unmount, worker, || {
+        crate::tools::disks::unmount_by_mountpoint(Path::new(&mount_point))
+    })
 }
 
 #[api(
@@ -2723,6 +2689,54 @@ pub async fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Resul
     Ok(json!(upid))
 }
 
+/// Wait for no more active operations on the given datastore and run the provided callback in
+/// a datastore locked context, protecting against maintenance mode changes.
+fn run_maintenance_locked(
+    store: &str,
+    maintenance_expected: MaintenanceType,
+    worker: &dyn WorkerTaskContext,
+    callback: impl Fn() -> Result<(), Error>,
+) -> Result<(), Error> {
+    let mut active_operations = task_tracking::get_active_operations(store)?;
+    let mut old_status = String::new();
+    let mut aborted = false;
+
+    while active_operations.read + active_operations.write > 0 {
+        if worker.abort_requested() || expect_maintenance_type(store, maintenance_expected).is_err()
+        {
+            aborted = true;
+            break;
+        }
+        let status = format!(
+            "waiting for active operations to finsish before continuing: read {}, write {}",
+            active_operations.read, active_operations.write,
+        );
+        if status != old_status {
+            info!("{status}");
+            old_status = status;
+        }
+        std::thread::sleep(std::time::Duration::from_secs(1));
+        active_operations = task_tracking::get_active_operations(store)?;
+    }
+
+    if aborted || worker.abort_requested() {
+        let _ = expect_maintenance_type(store, maintenance_expected)
+            .inspect_err(|e| warn!("maintenance mode was not as expected: {e}"))
+            .and_then(|(lock, config)| {
+                unset_maintenance(lock, config)
+                    .inspect_err(|e| warn!("could not reset maintenance mode: {e}"))
+            });
+        bail!("aborted, due to user request");
+    }
+
+    let (lock, config) = expect_maintenance_type(store, maintenance_expected)?;
+    callback()?;
+    unset_maintenance(lock, config)
+        .map_err(|e| format_err!("could not reset maintenance mode: {e}"))?;
+
+    Ok(())
+}
+
 #[sortable]
 const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
     (
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 3/7] api: admin: make s3 refresh handler sync

[Christian Ebner]

In preparation for properly awaiting active operations and locking
of the datastore context so it cannot be altered while doing the
refresh. This will mostly be sync code, therefore run this on its
own thread.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in previous version, splitof for better readability and
  reduced patch diff

 src/api2/admin/datastore.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 0e81533ce..25cf153d0 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2673,17 +2673,17 @@ pub async fn unmount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<V
     },
 )]
 /// Refresh datastore contents from S3 to local cache store.
-pub async fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+pub fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
     let datastore = DataStore::lookup_datastore(&store, Some(Operation::Lookup))?;
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    let upid = WorkerTask::spawn(
+    let upid = WorkerTask::new_thread(
         "s3-refresh",
         Some(store),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| async move { datastore.s3_refresh().await },
+        move |_worker| proxmox_async::runtime::block_on(datastore.s3_refresh()),
     )?;
 
     Ok(json!(upid))
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 4/7] datastore: s3 refresh: set/unset maintenance mode in api handler

[Christian Ebner]

Instead of setting the maintenance mode in the datastores s3 refresh
helper method, do this in the api handler directly, reusing the pre-
defined helpers.

This is in preparation for fixing the s3 refresh to only be started
after still ongoing active operations complete.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- lock only after parsing auth id, ecc.
- async to sync method handle transition has been split off from this

 pbs-datastore/src/datastore.rs | 31 ++-----------------------------
 src/api2/admin/datastore.rs    | 22 ++++++++++++++++++++--
 2 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index b9d4ed2d2..031fa958b 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -2305,23 +2305,12 @@ impl DataStore {
         *OLD_LOCKING
     }
 
-    /// Set the datastore's maintenance mode to `S3Refresh`, fetch from S3 object store, clear and
-    /// replace the local cache store contents. Once finished disable the maintenance mode again.
-    /// Returns with error for other datastore backends without setting the maintenance mode.
+    /// Fetch contents from S3 object store, clear and replace the local cache store contents.
+    /// Returns with error for non-S3 datastore backends.
     pub async fn s3_refresh(self: &Arc<Self>) -> Result<(), Error> {
         match self.backend()? {
             DatastoreBackend::Filesystem => bail!("store '{}' not backed by S3", self.name()),
             DatastoreBackend::S3(s3_client) => {
-                let self_clone = Arc::clone(self);
-                tokio::task::spawn_blocking(move || {
-                    self_clone.maintenance_mode(Some(MaintenanceMode {
-                        ty: MaintenanceType::S3Refresh,
-                        message: None,
-                    }))
-                })
-                .await?
-                .context("failed to set maintenance mode")?;
-
                 let tmp_base = proxmox_sys::fs::make_tmp_dir(self.base_path(), None)
                     .context("failed to create temporary content folder in {store_base}")?;
 
@@ -2335,27 +2324,11 @@ impl DataStore {
                     let _ = std::fs::remove_dir_all(&tmp_base);
                     return Err(err);
                 }
-
-                let self_clone = Arc::clone(self);
-                tokio::task::spawn_blocking(move || self_clone.maintenance_mode(None))
-                    .await?
-                    .context("failed to clear maintenance mode")?;
             }
         }
         Ok(())
     }
 
-    // Set or clear the datastores maintenance mode by locking and updating the datastore config
-    fn maintenance_mode(&self, maintenance_mode: Option<MaintenanceMode>) -> Result<(), Error> {
-        let _lock = pbs_config::datastore::lock_config()?;
-        let (mut section_config, _digest) = pbs_config::datastore::config()?;
-        let mut datastore: DataStoreConfig = section_config.lookup("datastore", self.name())?;
-        datastore.set_maintenance_mode(maintenance_mode)?;
-        section_config.set_data(self.name(), "datastore", &datastore)?;
-        pbs_config::datastore::save_config(&section_config)?;
-        Ok(())
-    }
-
     // Fetch the contents (metadata, no chunks) of the datastore from the S3 object store to the
     // provided temporaray directory
     async fn fetch_tmp_contents(&self, tmp_base: &Path, s3_client: &S3Client) -> Result<(), Error> {
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 25cf153d0..7bd7c9f72 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2678,12 +2678,30 @@ pub fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
+    let _lock = pbs_config::datastore::lock_config()?;
+    let (mut section_config, _digest) = pbs_config::datastore::config()?;
+    let mut datastore_config: DataStoreConfig = section_config.lookup("datastore", &store)?;
+
+    datastore_config.set_maintenance_mode(Some(MaintenanceMode {
+        ty: MaintenanceType::S3Refresh,
+        message: None,
+    }))?;
+
+    section_config.set_data(&store, "datastore", &datastore_config)?;
+    pbs_config::datastore::save_config(&section_config)?;
+    drop(_lock);
+
     let upid = WorkerTask::new_thread(
         "s3-refresh",
-        Some(store),
+        Some(store.clone()),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| proxmox_async::runtime::block_on(datastore.s3_refresh()),
+        move |_worker| {
+            proxmox_async::runtime::block_on(datastore.s3_refresh())?;
+
+            let (_lock, config) = expect_maintenance_type(&store, MaintenanceType::S3Refresh)?;
+            unset_maintenance(_lock, config).context("failed to clear maintenance mode")
+        },
     )?;
 
     Ok(json!(upid))
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 5/7] api: datastore: wait for active operations to clear before s3 refresh

[Christian Ebner]

Currently, the s3 refresh does not take into consideration already
ongoing active operations, only blocking new ones.

This will however lead to inconsistencies if there are ongoing read
or write operations. Therefore, actively wait for ongoing operatioins
to complete before running the actual refresh and keep the datastore
config locked so the maintenance mode cannot be altered.If an abort
was requested while waiting, clear the maintenance mode as well.

By adding this as helper, it can be reuse for datastore recreation as
well.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- use dedicated helper which can be reused also for datastore recreation

 src/api2/admin/datastore.rs | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 7bd7c9f72..8fc5efe41 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2674,7 +2674,6 @@ pub async fn unmount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<V
 )]
 /// Refresh datastore contents from S3 to local cache store.
 pub fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
-    let datastore = DataStore::lookup_datastore(&store, Some(Operation::Lookup))?;
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
@@ -2696,17 +2695,21 @@ pub fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
         Some(store.clone()),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| {
-            proxmox_async::runtime::block_on(datastore.s3_refresh())?;
-
-            let (_lock, config) = expect_maintenance_type(&store, MaintenanceType::S3Refresh)?;
-            unset_maintenance(_lock, config).context("failed to clear maintenance mode")
-        },
+        move |worker| do_s3_refresh(&store, &worker),
     )?;
 
     Ok(json!(upid))
 }
 
+/// Performs an s3 refresh for given datastore. Expects the store to already be in maintenance mode
+/// s3-refresh.
+pub(crate) fn do_s3_refresh(store: &str, worker: &dyn WorkerTaskContext) -> Result<(), Error> {
+    let datastore = DataStore::lookup_datastore(&store, Some(Operation::Lookup))?;
+    run_maintenance_locked(&store, MaintenanceType::S3Refresh, worker, || {
+        proxmox_async::runtime::block_on(datastore.s3_refresh())
+    })
+}
+
 /// Wait for no more active operations on the given datastore and run the provided callback in
 /// a datastore locked context, protecting against maintenance mode changes.
 fn run_maintenance_locked(
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 6/7] api: config: avoid unneeded backend instance on s3 store recreation

[Christian Ebner]

The backend type is already known from the config and can be used to
check if the s3-refresh should be performed, there is no need to
actually instantiate the backend at this point.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in previous version

 src/api2/config/datastore.rs | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 677adf3d5..0bbf2002f 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -32,9 +32,7 @@ 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, DatastoreBackend, S3_DATASTORE_IN_USE_MARKER,
-};
+use pbs_datastore::{get_datastore_mount_status, DataStore, S3_DATASTORE_IN_USE_MARKER};
 use proxmox_rest_server::WorkerTask;
 use proxmox_s3_client::{S3ObjectKey, S3_HTTP_REQUEST_TIMEOUT};
 
@@ -338,9 +336,8 @@ pub fn create_datastore(
 
     let store_name = config.name.to_string();
 
-    if let (_backend, Some(s3_client)) =
-        DataStore::s3_client_and_backend_from_datastore_config(&config)?
-    {
+    let (backend, s3_client) = DataStore::s3_client_and_backend_from_datastore_config(&config)?;
+    if let Some(s3_client) = s3_client {
         proxmox_async::runtime::block_on(s3_client.head_bucket())
             .context("failed to access bucket")
             .map_err(|err| format_err!("{err:#}"))?;
@@ -370,15 +367,9 @@ pub fn create_datastore(
                     Some(Operation::Lookup),
                 )
                 .context("failed to lookup datastore")?;
-                match datastore
-                    .backend()
-                    .context("failed to get datastore backend")?
-                {
-                    DatastoreBackend::Filesystem => (),
-                    DatastoreBackend::S3(_s3_client) => {
-                        proxmox_async::runtime::block_on(datastore.s3_refresh())
-                            .context("S3 refresh failed")?;
-                    }
+                if backend == DatastoreBackendType::S3 {
+                    proxmox_async::runtime::block_on(datastore.s3_refresh())
+                        .context("S3 refresh failed")?;
                 }
             }
             Ok(())
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v3 7/7] api: config: guard s3-refresh on datastore recreation

[Christian Ebner]

Makes sure to wait for active operations to finish and then
lock the maintenance mode during the s3 refresh, which requires
exclusive access to the datastore contents.

Since this already holds the datastore lock, only update the
maintenance mode before using the previously introduced helper,
which now assures better consistency.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in previous version

 src/api2/config/datastore.rs | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 0bbf2002f..ec7ca0e33 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -15,9 +15,9 @@ use proxmox_uuid::Uuid;
 
 use pbs_api_types::{
     Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreBackendType, DatastoreNotify,
-    DatastoreTuning, KeepOptions, MaintenanceMode, Operation, 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, MaintenanceType, 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;
@@ -107,7 +107,7 @@ impl Drop for UnmountGuard {
 pub(crate) fn do_create_datastore(
     _lock: BackupLockGuard,
     mut config: SectionConfigData,
-    datastore: DataStoreConfig,
+    mut datastore: DataStoreConfig,
     reuse_datastore: bool,
     overwrite_in_use: bool,
 ) -> Result<(), Error> {
@@ -194,6 +194,12 @@ pub(crate) fn do_create_datastore(
                     });
                 }
             }
+            // 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(
@@ -348,7 +354,7 @@ pub fn create_datastore(
         Some(store_name.clone()),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| {
+        move |worker| {
             do_create_datastore(
                 lock,
                 section_config,
@@ -361,16 +367,8 @@ pub fn create_datastore(
                 do_create_prune_job(prune_job_config)?;
             }
 
-            if reuse_datastore {
-                let datastore = pbs_datastore::DataStore::lookup_datastore(
-                    &store_name,
-                    Some(Operation::Lookup),
-                )
-                .context("failed to lookup datastore")?;
-                if backend == DatastoreBackendType::S3 {
-                    proxmox_async::runtime::block_on(datastore.s3_refresh())
-                        .context("S3 refresh failed")?;
-                }
+            if reuse_datastore && backend == DatastoreBackendType::S3 {
+                crate::api2::admin::datastore::do_s3_refresh(&store_name, &worker)?;
             }
             Ok(())
         },
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied-series: [PATCH proxmox-backup v3 0/7] wait for active operations to finish before s3 refresh

[Fabian Grünbichler]

On Thu, 13 Nov 2025 15:22:07 +0100, Christian Ebner wrote:
> Datastore contents located on the s3 backend can be refreshed on the local
> datastore cache by running an s3-refresh, which will set the maintenance mode
> s3-refresh to block any read/write operation and fetch the contents to a
> temporary directory before moving them in place if the fetching was successful.
> 
> Setting the maintenance mode has however no effect on already active operations,
> which can cause inconsistencies once the s3 refresh moves the temporary folders
> in place.
> 
> [...]

Applied, thanks!

[1/7] api: admin: drop useless option for do_unmount_device() parameter
      commit: 30476ce11b702958eb7a7b668d1d135adfe4b6fb
[2/7] api: admin: rework waiting on active operations and maintenance locking
      commit: 8bf3d560768177450a299928dae30af986a48992
[3/7] api: admin: make s3 refresh handler sync
      commit: 524cf1e7bf0918a6bb916c4eeffdd6cc686f7701
[4/7] datastore: s3 refresh: set/unset maintenance mode in api handler
      commit: b98b22b2a6526fcf3217bc11a8b9fcbd0a883840
[5/7] api: datastore: wait for active operations to clear before s3 refresh
      commit: 29f4f48d92751f11410cd525323c2e0a436b40cf
[6/7] api: config: avoid unneeded backend instance on s3 store recreation
      commit: f5b2c6b86e7791104df62e384351490831e68b3c
[7/7] api: config: guard s3-refresh on datastore recreation
      commit: 7f36096e3b66f100ff59310663275f37c64cab8a

Best regards,
-- 
Fabian Grünbichler <f.gruenbichler@proxmox.com>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel