Re: [PATCH proxmox-backup 10/10] fix #7254: datastore: refuse new backps when capacity is almost full

[Christian Ebner <c.ebner@proxmox.com>]

On 4/30/26 5:05 PM, Robert Obkircher wrote:
> Add a datastore config option that can be used to reserve free space.
> 
> HumanByte is somewhat inaccurate (e.g. 1025 MiB rounds to 1024 MiB)
> but it should be good enough for this case.
> 
> Setting this option is not backwards compatible as older versions of
> the parser will not recognize it. For example, running something like
> `proxmox-backup-manager datastore list` will fail with older binaries.

As with any incremental config adaption, that should never happen under 
normal operation though? So IMHO not worth to be mentioned in the commit 
message.

> 
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
>   pbs-datastore/src/chunk_store.rs | 34 +++++++++++++++++++++++++-------
>   pbs-datastore/src/datastore.rs   |  9 +++++++++
>   src/api2/config/datastore.rs     |  2 ++
>   www/Utils.js                     |  6 ++++++
>   www/datastore/OptionView.js      | 10 ++++++++++
>   5 files changed, 54 insertions(+), 7 deletions(-)
> 
> diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
> index 6a9dfbcef..515f007d3 100644
> --- a/pbs-datastore/src/chunk_store.rs
> +++ b/pbs-datastore/src/chunk_store.rs
> @@ -10,6 +10,7 @@ use tracing::{info, warn};
>   
>   use pbs_api_types::{DatastoreFSyncLevel, GarbageCollectionStatus};
>   use pbs_config::BackupLockGuard;
> +use proxmox_human_byte::HumanByte;
>   use proxmox_io::ReadExt;
>   use proxmox_s3_client::S3Client;
>   use proxmox_sys::fs::{create_dir, create_path, file_type_from_file_stat, CreateOptions};
> @@ -105,6 +106,7 @@ impl ChunkStore {
>           uid: nix::unistd::Uid,
>           gid: nix::unistd::Gid,
>           sync_level: DatastoreFSyncLevel,
> +        reserved_space: Option<HumanByte>,
>       ) -> Result<Self, Error>
>       where
>           P: Into<PathBuf>,
> @@ -159,7 +161,7 @@ impl ChunkStore {
>               }
>           }
>   
> -        Self::open(name, base, sync_level)
> +        Self::open(name, base, sync_level, reserved_space)
>       }
>   
>       fn lockfile_path<P: Into<PathBuf>>(base: P) -> PathBuf {
> @@ -193,6 +195,7 @@ impl ChunkStore {
>           name: &str,
>           base: P,
>           sync_level: DatastoreFSyncLevel,
> +        reserved_space: Option<HumanByte>,
>       ) -> Result<Self, Error> {
>           let base: PathBuf = base.into();
>   
> @@ -209,10 +212,14 @@ impl ChunkStore {
>               locker: Some(locker),
>               mutex: Mutex::new(()),
>               sync_level,
> -            fs_limit: FileSystemLimit::new(None),
> +            fs_limit: FileSystemLimit::new(reserved_space.map(|s| s.as_u64())),
>           })
>       }
>   
> +    pub(crate) fn set_reserved_space(&self, bytes: Option<HumanByte>) {
> +        self.fs_limit.set_reserved_space(bytes.map(|s| s.as_u64()));
> +    }
> +
>       fn touch_chunk_no_lock(&self, digest: &[u8; 32]) -> Result<(), Error> {
>           // unwrap: only `None` in unit tests
>           assert!(self.locker.is_some());
> @@ -998,14 +1005,21 @@ fn test_chunk_store1() {
>       let temp_dir = TempDir::new().unwrap();
>       let path = temp_dir.path();
>   
> -    let chunk_store = ChunkStore::open("test", path, DatastoreFSyncLevel::None);
> +    let chunk_store = ChunkStore::open("test", path, DatastoreFSyncLevel::None, None);
>       assert!(chunk_store.is_err());
>   
>       let user = nix::unistd::User::from_uid(nix::unistd::Uid::current())
>           .unwrap()
>           .unwrap();
> -    let chunk_store =
> -        ChunkStore::create("test", path, user.uid, user.gid, DatastoreFSyncLevel::None).unwrap();
> +    let chunk_store = ChunkStore::create(
> +        "test",
> +        path,
> +        user.uid,
> +        user.gid,
> +        DatastoreFSyncLevel::None,
> +        Some(1 << 20),
> +    )
> +    .unwrap();
>   
>       let (chunk, digest) = crate::data_blob::DataChunkBuilder::new(&[0u8, 1u8])
>           .build()
> @@ -1021,8 +1035,14 @@ fn test_chunk_store1() {
>       let (exists, _) = chunk_store.insert_chunk(&chunk, &digest).unwrap();
>       assert!(exists);
>   
> -    let chunk_store =
> -        ChunkStore::create("test", path, user.uid, user.gid, DatastoreFSyncLevel::None);
> +    let chunk_store = ChunkStore::create(
> +        "test",
> +        path,
> +        user.uid,
> +        user.gid,
> +        DatastoreFSyncLevel::None,
> +        None,
> +    );
>       assert!(chunk_store.is_err());
>   
>       temp_dir.close().unwrap();
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index def88f30a..46163f2bd 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -597,6 +597,13 @@ impl DataStore {
>                       operation: Some(lookup.operation),
>                   }));
>               }
> +            let tuning: DatastoreTuning = serde_json::from_value(
> +                DatastoreTuning::API_SCHEMA
> +                    .parse_property_string(config.tuning.as_deref().unwrap_or(""))?,
> +            )?;
> +            datastore
> +                .chunk_store
> +                .set_reserved_space(tuning.reserved_space);
>               Arc::clone(&datastore.chunk_store)
>           } else {
>               let tuning: DatastoreTuning = serde_json::from_value(
> @@ -607,6 +614,7 @@ impl DataStore {
>                   lookup.name,
>                   config.absolute_path(),
>                   tuning.sync_level.unwrap_or_default(),
> +                tuning.reserved_space,
>               )?)
>           };
>   
> @@ -699,6 +707,7 @@ impl DataStore {
>               &name,
>               config.absolute_path(),
>               tuning.sync_level.unwrap_or_default(),
> +            tuning.reserved_space,
>           )?;
>           let inner = Arc::new(Self::with_store_and_config(
>               Arc::new(chunk_store),
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 16e85a636..50019e8bd 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -170,6 +170,7 @@ pub(crate) fn do_create_datastore(
>                   &datastore.name,
>                   &path,
>                   tuning.sync_level.unwrap_or_default(),
> +                tuning.reserved_space,
>               )
>           })?
>       } else {
> @@ -207,6 +208,7 @@ pub(crate) fn do_create_datastore(
>               backup_user.uid,
>               backup_user.gid,
>               tuning.sync_level.unwrap_or_default(),
> +            tuning.reserved_space,
>           )?
>       };
>   
> diff --git a/www/Utils.js b/www/Utils.js
> index a9239b005..ebb681bfa 100644
> --- a/www/Utils.js
> +++ b/www/Utils.js
> @@ -909,6 +909,12 @@ Ext.define('PBS.Utils', {
>           sync = PBS.Utils.tuningOptions['sync-level'][sync ?? '__default__'];
>           options.push(`${gettext('Sync Level')}: ${sync}`);
>   
> +        let reserved_space = tuning['reserved-space'];
> +        delete tuning['reserved-space'];
> +        options.push(
> +            `${gettext('Reserved Space')}: ${reserved_space ?? gettext('None')}`,
> +        );
> +
>           let gc_atime_safety_check = tuning['gc-atime-safety-check'];
>           delete tuning['gc-atime-safety-check'];
>           options.push(
> diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
> index bac9eab0c..dbf12b99b 100644
> --- a/www/datastore/OptionView.js
> +++ b/www/datastore/OptionView.js
> @@ -309,6 +309,16 @@ Ext.define('PBS.Datastore.Options', {
>                               deleteEmpty: true,
>                               value: '__default__',
>                           },
> +                        {
> +                            xtype: 'pmxSizeField',
> +                            name: 'reserved-space',
> +                            fieldLabel: gettext('Reserved Space'),
> +                            labelWidth: 200,
> +                            unit: 'MiB',
> +                            submitAutoScaledSizeUnit: true,
> +                            allowZero: true,
> +                            emptyText: gettext('None'),
> +                        },
>                           {
>                               xtype: 'proxmoxcheckbox',
>                               name: 'gc-atime-safety-check',