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 B82531FF139 for ; Tue, 10 Feb 2026 16:07:00 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7061F68F5; Tue, 10 Feb 2026 16:07:45 +0100 (CET) From: Robert Obkircher To: pbs-devel@lists.proxmox.com Subject: [PATCH v6 proxmox-backup 06/18] datastore: use fixed size types for FixedIndexWriter Date: Tue, 10 Feb 2026 16:06:22 +0100 Message-ID: <20260210150642.469670-7-r.obkircher@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260210150642.469670-1-r.obkircher@proxmox.com> References: <20260210150642.469670-1-r.obkircher@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1770735946557 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.060 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: K32UBWQPMOS3JBVQCNUH5FHMOMOW3KLH X-Message-ID-Hash: K32UBWQPMOS3JBVQCNUH5FHMOMOW3KLH X-MailFrom: r.obkircher@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: The file format is independent of the systems pointer size, so it is better to use fixed size types. For consistency with most other code, use u64 for the total content size and u32 for the chunk size. Internally, the latter is also a u64 to avoid some casts and becasue that is what is stored in the header anyway. Signed-off-by: Robert Obkircher --- pbs-datastore/src/datastore.rs | 4 ++-- pbs-datastore/src/fixed_index.rs | 33 +++++++++++++++++--------------- src/api2/backup/environment.rs | 6 +++--- src/api2/backup/mod.rs | 2 +- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index b023b0d3..3f9c222d 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -695,8 +695,8 @@ impl DataStore { pub fn create_fixed_writer>( &self, filename: P, - size: usize, - chunk_size: usize, + size: u64, + chunk_size: u32, ) -> Result { let full_path = self.inner.chunk_store.relative_path(filename.as_ref()); FixedIndexWriter::create(full_path, size, chunk_size) diff --git a/pbs-datastore/src/fixed_index.rs b/pbs-datastore/src/fixed_index.rs index 081dc51f..7f0ef91c 100644 --- a/pbs-datastore/src/fixed_index.rs +++ b/pbs-datastore/src/fixed_index.rs @@ -214,8 +214,10 @@ pub struct FixedIndexWriter { file: File, filename: PathBuf, tmp_filename: PathBuf, - chunk_size: usize, - size: usize, + /// Most places use u32 because values are just a few MiB, but here + /// u64 is sightly more convenient for calculations involving size. + chunk_size: u64, + size: u64, index_length: usize, index: *mut u8, pub uuid: [u8; 16], @@ -239,8 +241,8 @@ impl FixedIndexWriter { // Requires obtaining a shared chunk store lock beforehand pub fn create( full_path: impl Into, - size: usize, - chunk_size: usize, + size: u64, + chunk_size: u32, ) -> Result { let full_path = full_path.into(); let mut tmp_path = full_path.clone(); @@ -260,6 +262,8 @@ impl FixedIndexWriter { panic!("got unexpected header size"); } + let chunk_size = u64::from(chunk_size); + let ctime = proxmox_time::epoch_i64(); let uuid = Uuid::generate(); @@ -269,15 +273,15 @@ impl FixedIndexWriter { header.magic = file_formats::FIXED_SIZED_CHUNK_INDEX_1_0; header.ctime = i64::to_le(ctime); - header.size = u64::to_le(size as u64); - header.chunk_size = u64::to_le(chunk_size as u64); + header.size = u64::to_le(size); + header.chunk_size = u64::to_le(chunk_size); header.uuid = *uuid.as_bytes(); header.index_csum = [0u8; 32]; file.write_all(&buffer)?; - let index_length = size.div_ceil(chunk_size); + let index_length = size.div_ceil(chunk_size).try_into()?; let index_size = index_length * 32; nix::unistd::ftruncate(&file, (header_size + index_size) as i64)?; @@ -351,12 +355,10 @@ impl FixedIndexWriter { Ok(index_csum) } - fn check_chunk_alignment(&self, offset: usize, chunk_len: usize) -> Result { - if offset < chunk_len { + fn check_chunk_alignment(&self, offset: u64, chunk_len: u64) -> Result { + let Some(pos) = offset.checked_sub(chunk_len) else { bail!("got chunk with small offset ({} < {}", offset, chunk_len); - } - - let pos = offset - chunk_len; + }; if offset > self.size { bail!("chunk data exceeds size ({} >= {})", offset, self.size); @@ -378,7 +380,7 @@ impl FixedIndexWriter { bail!("got unaligned chunk (pos = {})", pos); } - Ok(pos / self.chunk_size) + Ok((pos / self.chunk_size) as usize) } fn add_digest(&mut self, index: usize, digest: &[u8; 32]) -> Result<(), Error> { @@ -409,10 +411,11 @@ impl FixedIndexWriter { /// content that is backed up. It is verified that `start` is /// aligned and that only the last chunk may be smaller. pub fn add_chunk(&mut self, start: u64, size: u32, digest: &[u8; 32]) -> Result<(), Error> { - let Some(end) = start.checked_add(size.into()) else { + let size = u64::from(size); + let Some(end) = start.checked_add(size) else { bail!("add_chunk: start and size are too large: {start}+{size}"); }; - let idx = self.check_chunk_alignment(end as usize, size as usize)?; + let idx = self.check_chunk_alignment(end, size)?; self.add_digest(idx, digest) } diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs index 517f698e..234b2582 100644 --- a/src/api2/backup/environment.rs +++ b/src/api2/backup/environment.rs @@ -67,7 +67,7 @@ struct DynamicWriterState { struct FixedWriterState { name: String, index: FixedIndexWriter, - size: usize, + size: u64, chunk_size: u32, chunk_count: u64, small_chunk_count: usize, // allow 0..1 small chunks (last chunk may be smaller) @@ -349,7 +349,7 @@ impl BackupEnvironment { &self, index: FixedIndexWriter, name: String, - size: usize, + size: u64, chunk_size: u32, incremental: bool, ) -> Result { @@ -621,7 +621,7 @@ impl BackupEnvironment { ); } - if size != (data.size as u64) { + if size != data.size { bail!( "fixed writer '{}' close failed - unexpected file size ({} != {})", data.name, diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs index 3e6b7a95..6e3b46c2 100644 --- a/src/api2/backup/mod.rs +++ b/src/api2/backup/mod.rs @@ -480,7 +480,7 @@ fn create_fixed_index( let env: &BackupEnvironment = rpcenv.as_ref(); let name = required_string_param(¶m, "archive-name")?.to_owned(); - let size = required_integer_param(¶m, "size")? as usize; + let size = required_integer_param(¶m, "size")? as u64; let reuse_csum = param["reuse-csum"].as_str(); let archive_name = name.clone(); -- 2.47.3