Re: [PATCH v5 proxmox-backup 07/16] api: backup: make fixed index file size optional

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

On 1/30/26 5:45 PM, Robert Obkircher wrote:
> Pass the optional size to the FixedIndexWriter to make it grow as
> necessary.
> 
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
>   src/api2/backup/environment.rs | 21 ++++++++++++---------
>   src/api2/backup/mod.rs         |  8 +++-----
>   2 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index bd9c5211..90a84ed2 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: Option<usize>,
>       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: Option<usize>,
>           chunk_size: u32,
>           incremental: bool,
>       ) -> Result<usize, Error> {
> @@ -443,6 +443,7 @@ 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.chunk_count += 1;
> @@ -624,13 +625,15 @@ impl BackupEnvironment {
>                   );
>               }
>   
> -            if size != (data.size as u64) {
> -                bail!(
> -                    "fixed writer '{}' close failed - unexpected file size ({} != {})",
> -                    data.name,
> -                    data.size,
> -                    size
> -                );
> +            if let Some(known_size) = data.size {
> +                if size != known_size as u64 {
> +                    bail!(
> +                        "fixed writer '{}' close failed - unexpected file size ({} != {})",
> +                        data.name,
> +                        known_size,
> +                        size,
> +                    );
> +                }
>               }
>           }
>   
> diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
> index c625a2dc..c2822c18 100644
> --- a/src/api2/backup/mod.rs
> +++ b/src/api2/backup/mod.rs
> @@ -456,7 +456,7 @@ pub const API_METHOD_CREATE_FIXED_INDEX: ApiMethod = ApiMethod::new(
>               ("archive-name", false, &BACKUP_ARCHIVE_NAME_SCHEMA),
>               (
>                   "size",
> -                false,
> +                true,
>                   &IntegerSchema::new("File size.").minimum(1).schema()
>               ),
>               (
> @@ -480,7 +480,7 @@ fn create_fixed_index(
>       let env: &BackupEnvironment = rpcenv.as_ref();
>   
>       let name = required_string_param(&param, "archive-name")?.to_owned();
> -    let size = required_integer_param(&param, "size")? as usize;
> +    let size = param["size"].as_u64().map(usize::try_from).transpose()?;
>       let reuse_csum = param["reuse-csum"].as_str();

The fixed index writer does not allow for incremental backups, failing 
if it is requested.

question: should we allow to proceed the backup without failing by 
simply switching to a non incremental backup for those index writers 
where no size is specified, including a log warning that that a 
non-incremental mode is used?

Should be better than failing the whole backup snapshot in case of 
multiple archives with mixed inputs.

>   
>       let archive_name = name.clone();
> @@ -528,9 +528,7 @@ fn create_fixed_index(
>           reader = Some(index);
>       }
>   
> -    let mut writer = env
> -        .datastore
> -        .create_fixed_writer(&path, Some(size), chunk_size)?;
> +    let mut writer = env.datastore.create_fixed_writer(&path, size, chunk_size)?;
>   
>       if let Some(reader) = reader {
>           writer.clone_data_from(&reader)?;