[pbs-devel] [PATCH proxmox-backup v5 1/2] fix #5439: allow to reuse existing datastore

[Wolfgang Bumiller]

Just noticed another thing...

On Thu, Aug 29, 2024 at 12:45:10PM GMT, Gabriel Goller wrote:
> @@ -70,21 +70,43 @@ pub(crate) fn do_create_datastore(
>      _lock: BackupLockGuard,
>      mut config: SectionConfigData,
>      datastore: DataStoreConfig,
> +    reuse_datastore: bool,
>  ) -> Result<(), Error> {
>      let path: PathBuf = datastore.path.clone().into();
>  
> +    if path.parent().is_none() {
> +        bail!("cannot create datastore in root path");
> +    }
> +
>      let tuning: DatastoreTuning = serde_json::from_value(
>          DatastoreTuning::API_SCHEMA
>              .parse_property_string(datastore.tuning.as_deref().unwrap_or(""))?,
>      )?;
> -    let backup_user = pbs_config::backup_user()?;
> -    let _store = ChunkStore::create(
> -        &datastore.name,
> -        path,
> -        backup_user.uid,
> -        backup_user.gid,
> -        tuning.sync_level.unwrap_or_default(),
> -    )?;
> +
> +    if reuse_datastore {
> +        ChunkStore::verify_chunkstore(&path)?;
> +    } else {
> +        let datastore_empty = std::fs::read_dir(path.clone()).map_or(true, |mut d| {

No need to clone() path here.

> +            d.all(|dir| {
> +                dir.map_or(false, |file| {
> +                    file.file_name()
> +                        .to_str()
> +                        .map_or(false, |name| name.starts_with('.'))

IMO if we manage to open the directory, but the iteration fails, we
should actually fail here as well.
Also we should not just ignore *anything* starting with a `.`, but
specifically only exactly `.` and `..`.

Something like

        let datastore_empty = 'empty: {
            if let Ok(dir) = std::fs::read_dir(&path) {
                for file in dir {
                    let name = file?.file_name();
                    let name = name.as_encoded_bytes();
                    if name != b"." && name != b".." {
                        break 'empty false;
                    }
                }
            }
            true
        };

> +                })
> +            })
> +        });
> +        if !datastore_empty {
> +            bail!("path not empty!");
> +        }
> +        let backup_user = pbs_config::backup_user()?;
> +        let _store = ChunkStore::create(
> +            &datastore.name,
> +            path,
> +            backup_user.uid,
> +            backup_user.gid,
> +            tuning.sync_level.unwrap_or_default(),
> +        )?;
> +    }
>  
>      config.set_data(&datastore.name, "datastore", &datastore)?;
>