Re: [pbs-devel] [PATCH v4 proxmox-backup 09/11] datastore: use u64 instead of usize for fidx writer content size

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

On 1/26/26 15:01, Christian Ebner wrote:
> On 1/26/26 2:19 PM, Robert Obkircher wrote:
>>
>> On 1/26/26 12:38, Christian Ebner wrote:
>>> Not sure about these changes, maybe other devs have a stronger
>>> opinion on this one.
>>>
>>> If we do want to adapt this, then IMHO this should however be done
>>> throughout the whole codebase, for the dynamic index as well.
>> The dynamic reader/writer and the FixedIndexReader already use u64 for
>> content
>> size and offsets. The API only supports 4 MiB chunks anyway, so it
>> shouldn't matter
>> if we keep using u32 there.
>
> Disregard that comment with respect to the dynamic index in that
> case, seems indeed to be the case and I got sidetracked by the index
> positions which are usize.
>
> But still this would be nice to have consistent, e.g. there is the
> check for chunk size used on the client side [0].
>
> [0]
> https://git.proxmox.com/?p=proxmox-backup.git;a=blob;f=pbs-datastore/src/chunk_store.rs;h=bd1dc353b5e8f5f0e7b8f2d6788f4d29433e2a9e;hb=HEAD#l42 

There is a slight difference between these two.

IMO the index file readers/writers should accurately represent
the file format. For example, a 32-bit system might still want
to process an index file, even if the referenced contents are
huge.

The linked client code on the other hand actually needs to 
load the contents into memory, so isize::MAX is a also real
limitation.

At the moment, this specific option also isn't relevant for fidx
backups, because the chunk size is hard-coded to 4 MiB on
the server. Specifying one of the other values actually leads
to a rather confusing error:

$ proxmox-backup-client backup --chunk-size 1024 big.img:big
...
Upload image 'big' to 'root@pam!tok@localhost:8007:test' as
big.img.fidx    
Previous manifest does not contain an archive called 'big.img.fidx',
skipping download..
Error: fixed writer '1' - detected multiple end chunks (chunk size too
small)


I do agree that the inconsistent types on the server are not
ideal though. u64 makes sense for the chunk_size, because
that's what we store in the file header and it's the same type
as the total size, but u32 is also reasonable, because DataBlob
is documented to be < 128MB. Using u32 everywhere might
actually be the best option.