From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id DD4701FF141 for ; Fri, 30 Jan 2026 14:55:46 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5E58F1EA12; Fri, 30 Jan 2026 14:56:13 +0100 (CET) Message-ID: <0ebe7911-98b4-4dda-90cd-1682bea7e2b0@proxmox.com> Date: Fri, 30 Jan 2026 14:56:09 +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: Christian Ebner , 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> Content-Language: en-US, de-AT From: Robert Obkircher In-Reply-To: <02bfc738-fb21-4d14-a982-67cb42b5690e@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1769781300704 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.053 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 Message-ID-Hash: T2NHFT5QGODMNV2TPWSNMRPCUFB4XB2J X-Message-ID-Hash: T2NHFT5QGODMNV2TPWSNMRPCUFB4XB2J X-MailFrom: r.obkircher@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/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.