Re: [pbs-devel] [PATCH proxmox-backup] api: enhance directory existence check

[Lukas Wagner <l.wagner@proxmox.com>]

On 11/29/23 15:08, Markus Frank wrote:
> Instead of just checking the existence of the path, the code now also checks
> whether the directory is empty, continues if it is, and aborts if it is not.
> 
> Previously, if a directory were deleted and a directory with the same name
> would be created, the old check prevented the creation even though the
> directory could be used as a mount point.


But that does not detect if another (empty) filesystem is already 
mounted at that directory, right?

> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>   src/api2/node/disks/directory.rs | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
> index 5e1cb124..cd7b44a3 100644
> --- a/src/api2/node/disks/directory.rs
> +++ b/src/api2/node/disks/directory.rs
> @@ -155,13 +155,17 @@ pub fn create_datastore_disk(
>   
>       let mount_point = format!("{}{}", BASE_MOUNT_DIR, &name);
>   
> -    // check if the default path does exist already and bail if it does
> +    // check if the default path does exist already
> +    // and bail if it does and is not empty
>       let default_path = std::path::PathBuf::from(&mount_point);
>   
>       match std::fs::metadata(&default_path) {
>           Err(_) => {} // path does not exist
>           Ok(_) => {
> -            bail!("path {:?} already exists", default_path);
> +            let is_empty = default_path.read_dir()?.next().is_none();
> +            if !is_empty {
> +                bail!("path {:?} already exists and is not empty", default_path);
You can inline 'default_path' here: {default_path:?}

> +            }
>           }
>       }
>   

Also, I guess you could change the 'match' to a
'if std::fs::metadata(...).is_ok()', since we ignore the enclosed values 
anyway.

-- 
- Lukas