Re: [pbs-devel] [PATCH v4 proxmox-backup 08/11] datastore: combine public FixedIndexWriter methods into add_chunk.

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

While this is fine code-wise, this refactoring should happen as 
preparatory patch before the introduction of 
FixedIndexWriter::grow_to_size(), as this will reduce the patch diff 
quite a bit.

On 1/23/26 4:43 PM, Robert Obkircher wrote:
> These operations were always perfomed together anyway, so it makes

typo: s/perfomed/performed

> sense to simplify the public api.
> 
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
>   pbs-datastore/src/fixed_index.rs | 24 +++++++++++++++++++++---
>   src/api2/backup/environment.rs   |  6 +-----
>   2 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/pbs-datastore/src/fixed_index.rs b/pbs-datastore/src/fixed_index.rs
> index ecfe9b28..056ae07b 100644
> --- a/pbs-datastore/src/fixed_index.rs
> +++ b/pbs-datastore/src/fixed_index.rs
> @@ -376,7 +376,7 @@ impl FixedIndexWriter {
>       /// The size also becomes fixed as soon as it is no longer divisible
>       /// by the block size, to ensure that only the last block can be
>       /// smaller.
> -    pub fn grow_to_size(&mut self, requested_size: usize) -> Result<(), Error> {
> +    fn grow_to_size(&mut self, requested_size: usize) -> Result<(), Error> {
>           if self.size < requested_size {
>               if !self.growable_size {
>                   bail!("refusing to resize from {} to {requested_size}", self.size);
> @@ -463,7 +463,7 @@ impl FixedIndexWriter {
>           Ok(index_csum)
>       }
>   
> -    pub fn check_chunk_alignment(&self, offset: usize, chunk_len: usize) -> Result<usize, Error> {
> +    fn check_chunk_alignment(&self, offset: usize, chunk_len: usize) -> Result<usize, Error> {
>           if offset < chunk_len {
>               bail!("got chunk with small offset ({} < {}", offset, chunk_len);
>           }
> @@ -493,7 +493,7 @@ impl FixedIndexWriter {
>           Ok(pos / self.chunk_size)
>       }
>   
> -    pub fn add_digest(&mut self, index: usize, digest: &[u8; 32]) -> Result<(), Error> {
> +    fn add_digest(&mut self, index: usize, digest: &[u8; 32]) -> Result<(), Error> {
>           if index >= self.index_length {
>               bail!(
>                   "add digest failed - index out of range ({} >= {})",
> @@ -515,6 +515,24 @@ impl FixedIndexWriter {
>           Ok(())
>       }
>   
> +    /// Write the digest of a chunk into this index file.
> +    ///
> +    /// The `start` and `size` parameters encode the range of
> +    /// content that is backed up. It is verified that `start` is
> +    /// aligned and that only the last chunk may be smaller.
> +    ///
> +    /// If this writer has been created without a fixed size, the
> +    /// index capacity and content size are increased automatically
> +    /// until an incomplete chunk is encountered.
> +    pub fn add_chunk(&mut self, start: u64, size: u32, digest: &[u8; 32]) -> Result<(), Error> {
> +        let Some(end) = start.checked_add(size.into()) else {
> +            bail!("add_chunk: start and size are too large: {start}+{size}");
> +        };
> +        self.grow_to_size(end as usize)?;
> +        let idx = self.check_chunk_alignment(end as usize, size as usize)?;
> +        self.add_digest(idx, digest)
> +    }
> +
>       pub fn clone_data_from(&mut self, reader: &FixedIndexReader) -> Result<(), Error> {
>           if self.growable_size {
>               bail!("reusing the index is only supported with known input size");
> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index dc4f9bf5..04c5bf84 100644
> --- a/src/api2/backup/environment.rs
> +++ b/src/api2/backup/environment.rs
> @@ -442,14 +442,10 @@ impl BackupEnvironment {
>               );
>           }
>   
> -        let end = (offset as usize) + (size as usize);
> -        data.index.grow_to_size(end)?;
> -        let idx = data.index.check_chunk_alignment(end, size as usize)?;
> +        data.index.add_chunk(offset, size, digest)?;
>   
>           data.chunk_count += 1;
>   
> -        data.index.add_digest(idx, digest)?;
> -
>           Ok(())
>       }
>   



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel