[pbs-devel] [PATCH proxmox] sync: pull: fix regression in resync of corrupt snapshots

[Hannes Laimer]

Gave this a quick spin and I could reproduce the described problem, the
patch fixes it. This will ignore `transfer_last` though, I think it
makes sense like this, just wanted to note it.

Consider this

Reviewed-by: Hannes Laimer <h.laimer at proxmox.com>
Tested-by: Hannes Laimer <h.laimer at proxmox.com>

On 17.09.25 12:30, Christian Ebner wrote:
> In commit eea1176f ("pull: refactor source snapshot list filtering
> logic") the filtering of snapshots to be synced when pulling from the
> source datastore was split in preparation for correct pre-filtering
> of the snapshot when the verified-only or encrypted-only flags have
> been set.
> 
> This however introduced a regression, as previously also snapshots
> older than the newest snapshot on the target were included in the
> list, if the `resync_corrupt` parameter is set and the corresponding
> target snapshot failed verification or the loading of the manifest to
> check this failed, now however being filtered out unconditionally.
> 
> Fix this by keeping these snapshots in the list in case the
> `resync_corrupt` flag is set.
> 
> Also rename the boolean flag from the incorrect `verified` to
> `needs_resync`, in order to reflect the behaviour.
> 
> Fixes: eea1176f ("pull: refactor source snapshot list filtering logic")
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
> Noted while investigating an unrelated issue.
> 
>   src/server/pull.rs | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/server/pull.rs b/src/server/pull.rs
> index e1c8a7acd..08e6e6b64 100644
> --- a/src/server/pull.rs
> +++ b/src/server/pull.rs
> @@ -676,7 +676,10 @@ async fn pull_group(
>       let list: Vec<(BackupDir, bool)> = list
>           .into_iter()
>           .enumerate()
> -        .filter_map(|(pos, (dir, verified))| {
> +        .filter_map(|(pos, (dir, needs_resync))| {
> +            if params.resync_corrupt && needs_resync {
> +                return Some((dir, needs_resync));
> +            }
>               // Note: the snapshot represented by `last_sync_time` might be missing its backup log
>               // or post-backup verification state if those were not yet available during the last
>               // sync run, always resync it
> @@ -688,7 +691,7 @@ async fn pull_group(
>                   transfer_last_skip_info.update(dir.time);
>                   return None;
>               }
> -            Some((dir, verified))
> +            Some((dir, needs_resync))
>           })
>           .collect();
>