From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id CEDD11FF141 for ; Tue, 05 May 2026 15:49:09 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8D2EE3F92; Tue, 5 May 2026 15:49:08 +0200 (CEST) Message-ID: Date: Tue, 5 May 2026 15:49:00 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Christian Ebner Subject: Re: [PATCH proxmox-backup 10/10] fix #7254: datastore: refuse new backps when capacity is almost full To: Robert Obkircher , pbs-devel@lists.proxmox.com References: <20260430150607.330413-1-r.obkircher@proxmox.com> <20260430150607.330413-14-r.obkircher@proxmox.com> Content-Language: en-US, de-DE In-Reply-To: <20260430150607.330413-14-r.obkircher@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1777988834904 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.070 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: JSZPF2JH4ZS6TVIPEUS3MWV3FPL3CLRE X-Message-ID-Hash: JSZPF2JH4ZS6TVIPEUS3MWV3FPL3CLRE X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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. > > Signed-off-by: Robert Obkircher > --- > 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, > ) -> Result > where > P: Into, > @@ -159,7 +161,7 @@ impl ChunkStore { > } > } > > - Self::open(name, base, sync_level) > + Self::open(name, base, sync_level, reserved_space) > } > > fn lockfile_path>(base: P) -> PathBuf { > @@ -193,6 +195,7 @@ impl ChunkStore { > name: &str, > base: P, > sync_level: DatastoreFSyncLevel, > + reserved_space: Option, > ) -> Result { > 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) { > + 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(); This test fails as the provided optional value does not contain a HumanByte, should be: diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs index 515f007d3..4b4653f0c 100644 --- a/pbs-datastore/src/chunk_store.rs +++ b/pbs-datastore/src/chunk_store.rs @@ -1002,6 +1002,7 @@ impl ChunkExt { #[test] fn test_chunk_store1() { use tempfile::TempDir; + use proxmox_human_byte::HumanByte; let temp_dir = TempDir::new().unwrap(); let path = temp_dir.path(); @@ -1017,7 +1018,7 @@ fn test_chunk_store1() { user.uid, user.gid, DatastoreFSyncLevel::None, - Some(1 << 20), + Some(HumanByte::from(1u64 << 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',