Re: [pbs-devel] [RFC v2 proxmox-backup 12/21] datastore: clear trashed snapshot dir if re-creation requested

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On May 8, 2025 3:05 pm, Christian Ebner wrote:
> If a previously trashed snapshot has been requested for re-creation
> (e.g. by a sync job in push direction), drop the contents of the
> currently trashed snapshot.
> The snapshot directory itself is already locked at that point, either
> by the old locking mechanism acting on the directory itself or by the
> new locking mechanism. Therefore, concurrent operations can be
> excluded.
> 
> For the call site this acts as if the snapshot directory has been
> newly created.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  pbs-datastore/src/datastore.rs | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index ffc6a7039..4f7766c36 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -951,8 +951,9 @@ impl DataStore {
>      ) -> Result<(PathBuf, bool, BackupLockGuard), Error> {
>          let backup_dir = self.backup_dir(ns.clone(), backup_dir.clone())?;
>          let relative_path = backup_dir.relative_path();
> +        let full_path = backup_dir.full_path();
>  
> -        match std::fs::create_dir(backup_dir.full_path()) {
> +        match std::fs::create_dir(&full_path) {
>              Ok(_) => {
>                  let guard = backup_dir.lock().with_context(|| {
>                      format!("while creating new locked snapshot '{backup_dir:?}'")
> @@ -963,6 +964,32 @@ impl DataStore {
>                  let guard = backup_dir
>                      .lock()
>                      .with_context(|| format!("while creating locked snapshot '{backup_dir:?}'"))?;
> +
> +                if backup_dir.is_trashed() {
> +                    info!("clear trashed backup snapshot {full_path:?}");
> +                    let dir_entries = std::fs::read_dir(&full_path).context(
> +                        "failed to read directory contents during cleanup of trashed snapshot",
> +                    )?;
> +                    for entry in dir_entries {
> +                        let entry = entry.context(
> +                            "failed to read directory entry during clenup of trashed snapshot",
> +                        )?;
> +                        // Only expect regular file entries
> +                        std::fs::remove_file(entry.path()).context(
> +                            "failed to remove directory entry during clenup of trashed snapshot",
> +                        )?;
> +                    }
> +                    let group = BackupGroup::from(backup_dir);
> +                    let group_trash_file = group.full_group_path().join(TRASH_MARKER_FILENAME);
> +                    if let Err(err) = std::fs::remove_file(&group_trash_file) {
> +                        if err.kind() != std::io::ErrorKind::NotFound {
> +                            bail!("failed to remove group trash file of trashed snapshot");
> +                        }
> +                    }

this shouldn't be possible to hit, right? as creating a backup dir
entails first creating the backup group (guarded by the group lock), and
that would already clear the group's trash marker..

> +                    self.untrash_namespace(ns)?;
> +                    return Ok((relative_path, true, guard));
> +                }
> +
>                  Ok((relative_path, false, guard))
>              }
>              Err(e) => Err(e.into()),
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


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