From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id A60E61FF0A7 for ; Wed, 02 Sep 2026 15:56:27 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5BBE6216BA; Wed, 02 Sep 2026 15:55:50 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Subject: [RFC proxmox-backup-qemu 4/9] close #6169: track checksums for incremental backups per target Date: Wed, 2 Sep 2026 15:54:35 +0200 Message-ID: <20260902135536.525194-5-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260902135536.525194-1-f.ebner@proxmox.com> References: <20260902135536.525194-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788357336019 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.738 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: OBUCPG4SOZY5LL4EW2K6EREOASQRRFT6 X-Message-ID-Hash: OBUCPG4SOZY5LL4EW2K6EREOASQRRFT6 X-MailFrom: f.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 VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: To decide whether an incremental backup can be done, the checksum of the latest backup on the target is compared against the checksum from the previous backup the QEMU client library did. This was previously tracked per device as the PVE backup code in QEMU also only used a single bitmap per device. So backing up to target A, dirtying the disk and backing up to target B would mean that the checksum for target A is overwritten and a subsequent backup to target A needs to discard the dirty bitmap. Allow for tracking multiple targets with different bitmaps, by saving the checksums per device+target combination, rather than just per device. Whether the new behavior or the old behavior is used is controlled by QEMU, optionally passing along the target ID, and a flag for the export and import functions for migration. These are machine version guarded on the QEMU/qemu-server side to avoid a mismatch. Signed-off-by: Fiona Ebner --- src/backup.rs | 27 ++++++++++----- src/commands.rs | 88 +++++++++++++++++++++++++++++++++++++++++-------- src/lib.rs | 25 +++++++++----- 3 files changed, 111 insertions(+), 29 deletions(-) diff --git a/src/backup.rs b/src/backup.rs index cf1806a..8b3a736 100644 --- a/src/backup.rs +++ b/src/backup.rs @@ -234,7 +234,12 @@ impl BackupTask { self.last_manifest.get().map(Arc::clone) } - pub fn check_incremental(&self, device_name: String, size: u64) -> bool { + pub fn check_incremental( + &self, + device_name: String, + size: u64, + target_id: Option, + ) -> bool { match self.last_manifest() { Some(ref manifest) => { let archive_name = if let Ok(archive) = archive_name_from_device_name(&device_name) @@ -244,13 +249,17 @@ impl BackupTask { return false; }; - check_last_incremental_csum(Arc::clone(manifest), &archive_name, &device_name, size) - && check_last_encryption_mode( - Arc::clone(manifest), - &archive_name, - self.crypt_mode, - ) - && check_last_encryption_key(self.crypt_config.clone()) + check_last_incremental_csum( + Arc::clone(manifest), + &archive_name, + &device_name, + size, + target_id.as_deref(), + ) && check_last_encryption_mode( + Arc::clone(manifest), + &archive_name, + self.crypt_mode, + ) && check_last_encryption_key(self.crypt_config.clone()) } None => false, } @@ -260,6 +269,7 @@ impl BackupTask { &self, device_name: String, size: u64, + target_id: Option, incremental: bool, ) -> Result { self.check_aborted()?; @@ -273,6 +283,7 @@ impl BackupTask { Arc::clone(&self.known_chunks), device_name, size, + target_id, self.setup.chunk_size, incremental, ); diff --git a/src/commands.rs b/src/commands.rs index 749a0d2..050c1e3 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -4,6 +4,7 @@ use std::os::raw::c_int; use std::sync::{Arc, Mutex}; use futures::future::{Future, TryFutureExt}; +use serde::{Deserialize, Serialize}; use serde_json::json; use pbs_api_types::{BackupArchiveName, CryptMode, ENCRYPTED_KEY_BLOB_NAME, MANIFEST_BLOB_NAME}; @@ -21,6 +22,12 @@ use crate::upload_queue::{ use lazy_static::lazy_static; +#[derive(PartialEq, Eq, Hash, Serialize, Deserialize)] +struct DeviceOnTarget { + device_name: String, + target_id: String, +} + lazy_static! { // Note: Any state stored here that needs to be sent along with migration // needs to be specified in (de)serialize_state as well! @@ -29,6 +36,10 @@ lazy_static! { Mutex::new(HashMap::new()) }; + static ref PREVIOUS_CSUMS_PER_TARGET: Mutex> = { + Mutex::new(HashMap::new()) + }; + static ref PREVIOUS_KEY_FINGERPRINT: Mutex> = { Mutex::new(None) }; @@ -39,21 +50,66 @@ pub struct ImageUploadInfo { device_name: String, zero_chunk_digest: [u8; 32], device_size: u64, + target_id: Option, upload_queue: Option, upload_result: Option, } -pub(crate) fn serialize_state() -> Vec { - let prev_csums = &*PREVIOUS_CSUMS.lock().unwrap(); - let prev_key_fingerprint = &*PREVIOUS_KEY_FINGERPRINT.lock().unwrap(); - bincode::serialize(&(prev_csums, prev_key_fingerprint)).unwrap() +fn get_last_incremental_csum(device_name: &str, target_id: Option<&str>) -> Option<[u8; 32]> { + if let Some(target_id) = target_id { + let device_on_target = DeviceOnTarget { + device_name: device_name.to_string(), + target_id: target_id.to_string(), + }; + PREVIOUS_CSUMS_PER_TARGET + .lock() + .unwrap() + .get(&device_on_target) + .copied() + } else { + PREVIOUS_CSUMS.lock().unwrap().get(device_name).copied() + } } -pub(crate) fn deserialize_state(data: &[u8]) -> Result<(), Error> { - let (prev_csums, prev_key_fingerprint) = bincode::deserialize(data)?; - let mut prev_csums_guard = PREVIOUS_CSUMS.lock().unwrap(); +fn set_last_incremental_csum(device_name: String, target_id: Option, csum: [u8; 32]) { + if let Some(target_id) = target_id { + let mut prev_csum_guard = PREVIOUS_CSUMS_PER_TARGET.lock().unwrap(); + let device_on_target = DeviceOnTarget { + device_name: device_name.to_string(), + target_id: target_id.to_string(), + }; + prev_csum_guard.insert(device_on_target, csum); + } else { + let mut prev_csum_guard = PREVIOUS_CSUMS.lock().unwrap(); + prev_csum_guard.insert(device_name, csum); + } +} + +pub(crate) fn serialize_state(per_target_bitmaps: bool) -> Vec { + let prev_key_fingerprint = &*PREVIOUS_KEY_FINGERPRINT.lock().unwrap(); + if per_target_bitmaps { + let prev_csums = &*PREVIOUS_CSUMS_PER_TARGET.lock().unwrap(); + bincode::serialize(&(prev_csums, prev_key_fingerprint)).unwrap() + } else { + let prev_csums = &*PREVIOUS_CSUMS.lock().unwrap(); + bincode::serialize(&(prev_csums, prev_key_fingerprint)).unwrap() + } +} + +pub(crate) fn deserialize_state(data: &[u8], per_target_bitmaps: bool) -> Result<(), Error> { + let prev_key_fingerprint; + if per_target_bitmaps { + let prev_csums; + (prev_csums, prev_key_fingerprint) = bincode::deserialize(data)?; + let mut prev_csums_guard = PREVIOUS_CSUMS_PER_TARGET.lock().unwrap(); + *prev_csums_guard = prev_csums; + } else { + let prev_csums; + (prev_csums, prev_key_fingerprint) = bincode::deserialize(data)?; + let mut prev_csums_guard = PREVIOUS_CSUMS.lock().unwrap(); + *prev_csums_guard = prev_csums; + } let mut prev_key_fingerprint_guard = PREVIOUS_KEY_FINGERPRINT.lock().unwrap(); - *prev_csums_guard = prev_csums; *prev_key_fingerprint_guard = prev_key_fingerprint; Ok(()) } @@ -139,10 +195,11 @@ pub(crate) fn check_last_incremental_csum( archive_name: &BackupArchiveName, device_name: &str, device_size: u64, + target_id: Option<&str>, ) -> bool { - match PREVIOUS_CSUMS.lock().unwrap().get(device_name) { + match get_last_incremental_csum(device_name, target_id) { Some(csum) => manifest - .verify_file(archive_name, csum, device_size) + .verify_file(archive_name, &csum, device_size) .is_ok(), None => false, } @@ -188,6 +245,7 @@ pub(crate) async fn register_image( known_chunks: Arc>>, device_name: String, device_size: u64, + target_id: Option, chunk_size: u64, incremental: bool, ) -> Result { @@ -208,7 +266,7 @@ pub(crate) async fn register_image( let mut initial_index = Arc::new(None); if incremental { - let csum = PREVIOUS_CSUMS.lock().unwrap().get(&device_name).copied(); + let csum = get_last_incremental_csum(&device_name, target_id.as_deref()); if let Some(csum) = csum { param["reuse-csum"] = hex::encode(csum).into(); @@ -264,6 +322,7 @@ pub(crate) async fn register_image( device_name, zero_chunk_digest, device_size, + target_id, upload_queue: Some(upload_queue), upload_result: Some(upload_result), }; @@ -320,8 +379,11 @@ pub(crate) async fn close_image( let mut guard = registry.lock().unwrap(); let info = guard.lookup(dev_id)?; - let mut prev_csum_guard = PREVIOUS_CSUMS.lock().unwrap(); - prev_csum_guard.insert(info.device_name.clone(), upload_result.csum); + set_last_incremental_csum( + info.device_name.clone(), + info.target_id.clone(), + upload_result.csum, + ); let mut guard = manifest.lock().unwrap(); guard.add_file( diff --git a/src/lib.rs b/src/lib.rs index 1d4ea21..a21573a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -408,6 +408,7 @@ pub extern "C" fn proxmox_backup_check_incremental( handle: *mut ProxmoxBackupHandle, device_name: *const c_char, // expect utf8 here size: u64, + target_id: *const c_char, // expect utf8 here ) -> c_int { let task = backup_handle_to_task(handle); @@ -415,9 +416,11 @@ pub extern "C" fn proxmox_backup_check_incremental( return 0; } - match tools::utf8_c_string_lossy(device_name) { - None => 0, - Some(device_name) => i32::from(task.check_incremental(device_name, size)), + if let Some(device_name) = tools::utf8_c_string_lossy(device_name) { + let target_id = tools::utf8_c_string_lossy(target_id); + i32::from(task.check_incremental(device_name, size, target_id)) + } else { + 0 } } @@ -428,6 +431,7 @@ pub extern "C" fn proxmox_backup_register_image( handle: *mut ProxmoxBackupHandle, device_name: *const c_char, // expect utf8 here size: u64, + target_id: *const c_char, // expect utf8 here incremental: bool, error: *mut *mut c_char, ) -> c_int { @@ -441,6 +445,7 @@ pub extern "C" fn proxmox_backup_register_image( handle, device_name, size, + target_id, incremental, callback_info.callback, callback_info.callback_data, @@ -464,6 +469,7 @@ pub extern "C" fn proxmox_backup_register_image_async( handle: *mut ProxmoxBackupHandle, device_name: *const c_char, // expect utf8 here size: u64, + target_id: *const c_char, // expect utf8 here incremental: bool, callback: extern "C" fn(*mut c_void), callback_data: *mut c_void, @@ -481,9 +487,12 @@ pub extern "C" fn proxmox_backup_register_image_async( param_not_null!(device_name, callback_info); let device_name = unsafe { tools::utf8_c_string_lossy_non_null(device_name) }; + let target_id = tools::utf8_c_string_lossy(target_id); task.runtime().spawn(async move { - let result = task.register_image(device_name, size, incremental).await; + let result = task + .register_image(device_name, size, target_id, incremental) + .await; callback_info.send_result(result); }); } @@ -1175,8 +1184,8 @@ pub extern "C" fn proxmox_restore_read_image_at_async( /// be freed with proxmox_free_state_buf. #[no_mangle] #[allow(clippy::not_unsafe_ptr_arg_deref)] -pub extern "C" fn proxmox_export_state(buf_size: *mut usize) -> *mut u8 { - let data = commands::serialize_state(); +pub extern "C" fn proxmox_export_state(buf_size: *mut usize, per_target_bitmaps: bool) -> *mut u8 { + let data = commands::serialize_state(per_target_bitmaps); let len = data.len(); // Allocate via libc::malloc so the matching free in proxmox_free_state_buf // does not need to know the layout used by Rust's global allocator. @@ -1200,10 +1209,10 @@ pub extern "C" fn proxmox_export_state(buf_size: *mut usize) -> *mut u8 { /// will be logged to stderr, but the function will not fail. #[no_mangle] #[allow(clippy::not_unsafe_ptr_arg_deref)] -pub extern "C" fn proxmox_import_state(buf: *const u8, buf_size: usize) { +pub extern "C" fn proxmox_import_state(buf: *const u8, buf_size: usize, per_target_bitmaps: bool) { let data = unsafe { std::slice::from_raw_parts(buf, buf_size) }; // ignore errors, just log what happened - if let Err(err) = commands::deserialize_state(data) { + if let Err(err) = commands::deserialize_state(data, per_target_bitmaps) { eprintln!("error deserializing PBS state - {}", err); } } -- 2.47.3