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 0B0771FF136 for ; Mon, 20 Apr 2026 10:59:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9E92D220FF; Mon, 20 Apr 2026 10:59:33 +0200 (CEST) Message-ID: <571f30e6-23d8-4ec0-ba77-404abdbecc81@proxmox.com> Date: Mon, 20 Apr 2026 10:59:28 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup v3 18/30] sync: push: add helper for loading known chunks from previous snapshot To: Thomas Lamprecht References: <20260419210610.3915597-1-t.lamprecht@proxmox.com> <20260419210610.3915597-5-t.lamprecht@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <20260419210610.3915597-5-t.lamprecht@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: 1776675485802 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.070 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 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: HJCOVDU3JH6GSBSO63D5EQ2O325FYWZO X-Message-ID-Hash: HJCOVDU3JH6GSBSO63D5EQ2O325FYWZO 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 CC: pbs-devel@lists.proxmox.com 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 4/19/26 11:06 PM, Thomas Lamprecht wrote: > Am 14.04.26 um 14:59 schrieb Christian Ebner: >> diff --git a/src/server/push.rs b/src/server/push.rs >> @@ -808,6 +808,41 @@ pub(crate) async fn push_group( >> >> +async fn load_previous_snapshot_known_chunks( >> + ... >> +) { >> + if let Some(manifest) = upload_options.previous_manifest.as_ref() { >> + if let Some((_id, crypt_config)) = ¶ms.crypt_config { >> + if let Ok(Some(fingerprint)) = manifest.fingerprint() { >> + if *fingerprint.bytes() == crypt_config.fingerprint() { >> + // needs encryption during push, cannot reuse chunks from previous manifest >> + return; >> + } >> + } >> + } > > I think this has an issue when transitioning from unencrypted to encrypted > push. The guard returns early only when the previous manifest has a > matching fingerprint, but when params.crypt_config is Some and the > previous manifest has no fingerprint (was unencrypted), the `if let > Ok(Some(...))` doesn't match and we fall through to loading plaintext > chunk digests into known_chunks, if I'm not mistaken. > > In push_index, those plaintext digests won't be in encrypted_mapping (the > chunk was never read/encrypted), so the else branch uses the plaintext > digest as-is for MergedChunkInfo::Known. The remote target then ends up > referencing old unencrypted chunks while the manifest says it's encrypted. True, however the intention was to keep the optimization of loading known chunks for already encrypted source snapshots, since then chunks are not re-encypted, and re-upload can happen as is. > Simplest fix might be to just skip known chunk loading entirely when > crypt_config is set - the optimization can't really help when we need to > re-encrypt anyway, since chunk digests change with encryption. Should be enough to extend the current check by the early return in case there is no fingerprint on the source manifest (or failed to load it), but there is a crypt config? ``` + if let Some(manifest) = upload_options.previous_manifest.as_ref() { + if let Some((_id, crypt_config)) = ¶ms.crypt_config { + if let Ok(Some(fingerprint)) = manifest.fingerprint() { + if *fingerprint.bytes() == crypt_config.fingerprint() { + // needs encryption during push, cannot reuse chunks fro + return; + } + } else { + // previous manifest is not pre-encrypted or failed to check + // fallback to not reuse chunks + return; + } + }