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

[Robert Obkircher <r.obkircher@proxmox.com>]

On 2/2/26 12:38, Christian Ebner wrote:
> 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? 
I think reuse-csum without size could reasonably be supported by
copying the entire file and taking the size from there. Now that
add_chunk is a single operation it would even be possible to allow
overwriting the partial chunk in the end to continue growing.
>
> Should be better than failing the whole backup snapshot in case of
> multiple archives with mixed inputs.
As far as I can tell, "reuse-csum" is only set in one specific place
in proxmox-backup-qemu and only if the checksum matches exactly. Is
this actually relevant for anything else?
>
>>         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)?;
>