Re: [pbs-devel] [PATCH proxmox-backup v2 3/6] api: admin: factor out busy waiting on active operations

[Christian Ebner <c.ebner@proxmox.com>]

On 11/13/25 9:16 AM, Fabian Grünbichler wrote:
> On November 12, 2025 5:36 pm, Christian Ebner wrote:
>> Move the logic to wait on no more active operations on a given
>> datastore into a dedicated helper function, to be reused
>> by s3-refresh.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> changes since version 1:
>> - not present in previous version
>>
>>   src/api2/admin/datastore.rs | 54 ++++++++++++++++++++++++++-----------
>>   1 file changed, 38 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
>> index 6e66b5cf0..7daccf9fd 100644
>> --- a/src/api2/admin/datastore.rs
>> +++ b/src/api2/admin/datastore.rs
>> @@ -2586,29 +2586,21 @@ fn do_unmount_device(
>>       }
>>       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 let Some(worker) = worker {
>> -            if worker.abort_requested()
>> -                || expect_maintenance_type(&datastore.name, MaintenanceType::Unmount).is_err()
>> -            {
>> -                aborted = true;
>> -                break;
>> -            }
>> +    let aborted = wait_on_active_operations(
>> +        &datastore.name,
>> +        worker,
>> +        MaintenanceType::Unmount,
>> +        |reads, writes| {
>>               let status = format!(
>> -                "cannot unmount yet, still {} read and {} write operations active",
>> -                active_operations.read, active_operations.write
>> +                "cannot unmount yet, still {reads} read and {writes} write operations active",
>>               );
>>               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()) {
>>           let _ = expect_maintenance_type(&datastore.name, MaintenanceType::Unmount)
>> @@ -2725,6 +2717,36 @@ 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.
>> +/// If a worker task context is provided, the given callback will be executed for each busy wait
>> +/// iteration.
>> +///
>> +/// Returns with Ok(true) if the worker was aborted or the expected maintenance mode was not set,
>> +/// Ok(false) if no more operations are active.
>> +fn wait_on_active_operations(
>> +    store: &str,
>> +    worker: Option<&dyn WorkerTaskContext>,
> 
> this can drop the Option, all callers actually run in a worker context..

Right, now I'm wondering why this was an option to begin with in the 
do_unmount_device(), but will drop that for the v3.

> 
>> +    maintenance_expected: MaintenanceType,
>> +    mut status_msg_callback: impl FnMut(i64, i64),
>> +) -> Result<bool, Error> {
>> +    let mut active_operations = task_tracking::get_active_operations(&store)?;
>> +
>> +    while active_operations.read + active_operations.write > 0 {
>> +        if let Some(worker) = worker {
>> +            if worker.abort_requested()
>> +                || expect_maintenance_type(&store, maintenance_expected).is_err()
>> +            {
>> +                return Ok(true);
>> +            }
>> +            status_msg_callback(active_operations.read, active_operations.write);
>> +        }
>> +        std::thread::sleep(std::time::Duration::from_secs(1));
>> +        active_operations = task_tracking::get_active_operations(&store)?;
>> +    }
>> +
>> +    Ok(false)
>> +}
>> +
>>   #[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 mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 



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