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 7F78B1FF141 for ; Fri, 30 Jan 2026 15:46:38 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0A97B1FE6C; Fri, 30 Jan 2026 15:47:05 +0100 (CET) Message-ID: <1d6116f5-f714-48b9-b612-9b37dba31cc6@proxmox.com> Date: Fri, 30 Jan 2026 15:46:29 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [pbs-devel] [PATCH v4 proxmox-backup 09/11] datastore: use u64 instead of usize for fidx writer content size To: Robert Obkircher , Proxmox Backup Server development discussion References: <20260123154147.222215-1-r.obkircher@proxmox.com> <20260123154147.222215-10-r.obkircher@proxmox.com> <767de04e-3f23-4cf3-8ad8-45d4c2d4013f@proxmox.com> <5aea0de2-2cc1-4c30-a609-2cdb1a2497a6@proxmox.com> <02bfc738-fb21-4d14-a982-67cb42b5690e@proxmox.com> <0ebe7911-98b4-4dda-90cd-1682bea7e2b0@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <0ebe7911-98b4-4dda-90cd-1682bea7e2b0@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1769784320365 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.047 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Message-ID-Hash: GT36G4V3EG4AUEVE2RK7GYN7DLFT7FYY X-Message-ID-Hash: GT36G4V3EG4AUEVE2RK7GYN7DLFT7FYY X-MailFrom: c.ebner@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: On 1/30/26 2:55 PM, Robert Obkircher wrote: > > 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 file format is fixed already [0], so this we cannot adapt. However, as you stated correctly, the currently used actual chunk size limits are way smaller, so systems with smaller word size are still able to process (with the additional overhead of course). > The linked client code on the other hand actually needs to > load the contents into memory, so isize::MAX is a also real > limitation. This limitation would however only come into play if we bump the actually allowed chunk sizes. > 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) Yes, this should be fixed independently. > > 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. Yes, unless I miss something, u32 makes more sense. And just because that is encoded as u64 value in the fixed index file header does not mean that it actually needs the full size range of that. [0] https://pbs.proxmox.com/docs/file-formats.html#fixed-index-format-fidx