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

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

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

Good point, so will drop it then!

Although this opens up another thing to consider a bit closer: the 
current patches already favor the use of the trash over immediately 
deleting contents. I feel it might be best to only use it by default for 
manual operations via cli or ui, but not for actions taken by jobs... 
and for the first one provide the opt-out flags.


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