Re: [PATCH proxmox-backup 3/4] datastore: move try_ensure_sync_level() to DataStoreImpl

[Robert Obkircher <r.obkircher@proxmox.com>]

On 08.05.26 14:28, Christian Ebner wrote:
> Keep the public interface in place, but move the method to
> DataStoreImpl so it can be reused from the inner context as well.
>
> This is in preparation for fixing the sync level updates not being
> propagated to the chunk store.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  pbs-datastore/src/datastore.rs | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index bc218cb94..8f69dd7ac 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -234,6 +234,19 @@ impl DataStoreImpl {
>              thresholds_exceeded_callback: None,
>          })
>      }
> +
> +    fn try_ensure_sync_level(&self) -> Result<(), Error> {
> +        if self.sync_level != DatastoreFSyncLevel::Filesystem {
If the level in the chunk store changes from File to Filesystem this
still wouldn't sync the chunks that have been written afterwards.

It may be better to just move the entire method into the chunk store.
> +            return Ok(());
> +        }
> +        let file = std::fs::File::open(self.chunk_store.base_path())?;
> +        let fd = file.as_raw_fd();
> +        log::info!("syncing filesystem");
> +        if unsafe { libc::syncfs(fd) } < 0 {
> +            bail!("error during syncfs: {}", std::io::Error::last_os_error());
> +        }
> +        Ok(())
> +    }
>  }
>  
>  pub type ThresholdsExceededCallback = fn(&str, &str, u64, u64) -> Result<(), Error>;
> @@ -3015,16 +3028,7 @@ impl DataStore {
>      /// Syncs the filesystem of the datastore if 'sync_level' is set to
>      /// [`DatastoreFSyncLevel::Filesystem`]. Uses syncfs(2).
>      pub fn try_ensure_sync_level(&self) -> Result<(), Error> {
> -        if self.inner.sync_level != DatastoreFSyncLevel::Filesystem {
> -            return Ok(());
> -        }
> -        let file = std::fs::File::open(self.base_path())?;
> -        let fd = file.as_raw_fd();
> -        log::info!("syncing filesystem");
> -        if unsafe { libc::syncfs(fd) } < 0 {
> -            bail!("error during syncfs: {}", std::io::Error::last_os_error());
> -        }
> -        Ok(())
> +        self.inner.try_ensure_sync_level()
>      }
>  
>      /// Destroy a datastore. This requires that there are no active operations on the datastore.