[pbs-devel] [RFC v2 proxmox-backup 06/21] api: tape: check trash marker when trying to write snapshot

[Fabian Grünbichler]

> Christian Ebner <c.ebner at proxmox.com> hat am 12.05.2025 11:55 CEST geschrieben:
> 
>  
> On 5/12/25 11:38, Fabian Grünbichler wrote:
> > On May 12, 2025 11:19 am, Christian Ebner wrote:
> >> On 5/9/25 14:27, Fabian Grünbichler wrote:
> >>> On May 8, 2025 3:05 pm, Christian Ebner wrote:
> >>>> Since snapshots might be marked as trash, the snapshot directory
> >>>> can still be present until cleaned up by garbage collection.
> >>>>
> >>>> Therefore, check for the presence of the trash marker after acquiring
> >>>> the locked snapshot reader and skip over marked ones.
> >>>>
> >>>> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> >>>> ---
> >>>>    src/api2/tape/backup.rs | 8 +++++++-
> >>>>    1 file changed, 7 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
> >>>> index 923cb7834..17c8bc605 100644
> >>>> --- a/src/api2/tape/backup.rs
> >>>> +++ b/src/api2/tape/backup.rs
> >>>> @@ -574,7 +574,13 @@ fn backup_snapshot(
> >>>>        info!("backup snapshot {snapshot_path:?}");
> >>>>    
> >>>>        let snapshot_reader = match snapshot.locked_reader() {
> >>>> -        Ok(reader) => reader,
> >>>> +        Ok(reader) => {
> >>>> +            if snapshot.is_trashed() {
> >>>> +                info!("snapshot {snapshot_path:?} trashed, skipping");
> >>>
> >>> not sure why we log this, but don't log this in other places?
> >>
> >> The intention was to keep the pre-existing logging as for vanished
> >> snapshots (since pruned in the mean time), but make the 2 cases
> >> distinguishable.
> >>
> >> So I think that either the logging should be dropped for both cases, or
> >> this should be logged as is. Opinions?
> > 
> > iff we make trashing the default at some point, this would become very
> > noisy.. the vanished logging is different, since it "just" triggers on
> > the time window between listing snapshots and attempting to read from
> > them..
> 
> On second thought, the logging here would act just the same as for the 
> vanished case, as the snapshot list generate at the start of the job is 
> pre-filtered already, only considering not trashed snapshots...

I guess then we can keep it..