Re: [PATCH proxmox-backup] fix #7964: tape: make barcode-label task abortable

[Dominik Csapak <d.csapak@proxmox.com>]

LGTM

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>


On 11/6/26 10:42 AM, Jakob-Tobias Winter wrote:
> The barcode-label-media worker never checked whether an abort was
> requested, so pressing stop in the GUI had no effect and the task kept
> loading and labeling every unlabeled tape in the changer.
> 
> Check for an abort request before each tape is loaded, and again before
> the label is written, since loading the media can take a while. A load
> or label write that is already running cannot be interrupted, so those
> operations still run to completion.
> 
> Signed-off-by: Jakob-Tobias Winter <tobias@stuttgart.de>
> ---
> Tested on PBS 4.2.8 (patch applied on top of 4.2.8-1) with a real tape
> library: stopping the barcode-label task now aborts after the current
> load, without labeling the loaded tape or loading the next one.
> 
>   src/api2/tape/drive.rs | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
> index 79b3b269c..3d8cca128 100644
> --- a/src/api2/tape/drive.rs
> +++ b/src/api2/tape/drive.rs
> @@ -30,6 +30,7 @@ use pbs_tape::{
>       sg_tape::tape_alert_flags_critical,
>   };
>   use proxmox_rest_server::WorkerTask;
> +use proxmox_worker_task::WorkerTaskContext;
>   
>   use crate::{
>       api2::tape::restore::{fast_catalog_restore, restore_media},
> @@ -1040,13 +1041,14 @@ pub fn barcode_label_media(
>           drive.clone(),
>           "barcode-label-media",
>           Some(drive.clone()),
> -        move |_worker, config| barcode_label_media_worker(drive, &config, pool),
> +        move |worker, config| barcode_label_media_worker(&worker, drive, &config, pool),
>       )?;
>   
>       Ok(upid_str.into())
>   }
>   
>   fn barcode_label_media_worker(
> +    worker: &WorkerTask,
>       drive: String,
>       drive_config: &SectionConfigData,
>       pool: Option<String>,
> @@ -1076,6 +1078,10 @@ fn barcode_label_media_worker(
>               continue;
>           }
>   
> +        // loading and labeling a tape cannot be interrupted, so check
> +        // before starting with the next one
> +        worker.check_abort()?;
> +
>           inventory.reload()?;
>           match inventory.find_media_by_label_text(&label_text) {
>               Ok(Some(_)) => {
> @@ -1112,6 +1118,9 @@ fn barcode_label_media_worker(
>               }
>           }
>   
> +        // loading the media can take a while, so check again before writing the label
> +        worker.check_abort()?;
> +
>           let ctime = proxmox_time::epoch_i64();
>           let label = MediaLabel {
>               label_text: label_text.to_string(),