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

[Christian Ebner]

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...