[pbs-devel] applied: [PATCH proxmox-backup v2] backup/datastore: move manifest locking to /run

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

applied

On Wed, Dec 02, 2020 at 03:55:19PM +0100, Dominik Csapak wrote:
> this fixes the issue that on some filesystems, you cannot recursively
> remove a directory when you hold a lock on a file inside (e.g. nfs/cifs)
> 
> it is not really backwards compatible (so during an upgrade, there
> could be two daemons have the lock), but since the locking was
> broken before (see previous patch) it should not really matter
> (also it seems very unlikely that someone will trigger this)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * document the path method
> * use String instead of PathBuf and use write! to change it
>  src/backup/datastore.rs | 31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
> index 0f74ac3c..19e7ccba 100644
> --- a/src/backup/datastore.rs
> +++ b/src/backup/datastore.rs
> @@ -257,6 +257,12 @@ impl DataStore {
>                  )
>              })?;
>  
> +        // the manifest does not exists anymore, we do not need to keep the lock
> +        if let Ok(path) = self.manifest_lock_path(backup_dir) {
> +            // ignore errors
> +            let _ = std::fs::remove_file(path);
> +        }
> +
>          Ok(())
>      }
>  
> @@ -698,13 +704,32 @@ impl DataStore {
>          ))
>      }
>  
> +    /// Returns the filename to lock a manifest
> +    ///
> +    /// Also creates the basedir. The lockfile is located in
> +    /// '/run/proxmox-backup/locks/{datastore}/{type}/{id}/{timestamp}.index.json.lck'
> +    fn manifest_lock_path(
> +        &self,
> +        backup_dir: &BackupDir,
> +    ) -> Result<String, Error> {
> +        let mut path = format!(
> +            "/run/proxmox-backup/locks/{}/{}/{}",
> +            self.name(),
> +            backup_dir.group().backup_type(),
> +            backup_dir.group().backup_id(),
> +        );
> +        std::fs::create_dir_all(&path)?;
> +        use std::fmt::Write;
> +        write!(path, "/{}{}", backup_dir.backup_time_string(), &MANIFEST_LOCK_NAME)?;
> +
> +        Ok(path)
> +    }
> +
>      fn lock_manifest(
>          &self,
>          backup_dir: &BackupDir,
>      ) -> Result<File, Error> {
> -        let mut path = self.base_path();
> -        path.push(backup_dir.relative_path());
> -        path.push(&MANIFEST_LOCK_NAME);
> +        let path = self.manifest_lock_path(backup_dir)?;
>  
>          // update_manifest should never take a long time, so if someone else has
>          // the lock we can simply block a bit and should get it soon
> -- 
> 2.20.1