From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 0FA831FF17E for ; Thu, 13 Nov 2025 15:21:37 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8E9DF1E20A; Thu, 13 Nov 2025 15:22:29 +0100 (CET) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Thu, 13 Nov 2025 15:22:09 +0100 Message-ID: <20251113142214.719217-3-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251113142214.719217-1-c.ebner@proxmox.com> References: <20251113142214.719217-1-c.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1763043719508 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.047 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup v3 2/7] api: admin: rework waiting on active operations and maintenance locking X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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 --- 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