From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 12271968B5 for ; Wed, 25 Jan 2023 13:20:00 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E11B3F9BF for ; Wed, 25 Jan 2023 13:19:29 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 25 Jan 2023 13:19:28 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4D3AA4612F for ; Wed, 25 Jan 2023 13:19:28 +0100 (CET) From: Christoph Heiss To: pbs-devel@lists.proxmox.com Date: Wed, 25 Jan 2023 13:18:59 +0100 Message-Id: <20230125121902.404950-5-c.heiss@proxmox.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230125121902.404950-1-c.heiss@proxmox.com> References: <20230125121902.404950-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.055 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [commands.rs, lib.rs, backup.rs] Subject: [pbs-devel] [RFC PATCH v2 proxmox-backup-qemu 4/7] api: Supply `protected` parameter to the `finish` API call X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Jan 2023 12:20:00 -0000 This can be used to set backups as protected as soon as the transfer is finished, to e.g. avoid races while trying to set it protected later on, while the PBS might have locked it already for verification. ! This is a breaking API/ABI change ! Signed-off-by: Christoph Heiss --- Depends on the previous proxmox-backup patches (1-3 in the series). RFC question: It would probably be sensible to issue a warning in BackupTask::finish() - but what is the right way for that? A simple eprintln!()? Changes v1 -> v2: * Use new server feature mechanism to detect support for API parameter current-api.h | 3 ++- simpletest.c | 2 +- src/backup.rs | 24 ++++++++++++++++++++---- src/commands.rs | 10 +++++++++- src/lib.rs | 5 ++++- 5 files changed, 36 insertions(+), 8 deletions(-) diff --git a/current-api.h b/current-api.h index f38ad4b..729a3a3 100644 --- a/current-api.h +++ b/current-api.h @@ -271,7 +271,7 @@ void proxmox_backup_close_image_async(struct ProxmoxBackupHandle *handle, /** * Finish the backup (sync) */ -int proxmox_backup_finish(struct ProxmoxBackupHandle *handle, char **error); +int proxmox_backup_finish(struct ProxmoxBackupHandle *handle, bool protected_, char **error); /** * Finish the backup @@ -280,6 +280,7 @@ int proxmox_backup_finish(struct ProxmoxBackupHandle *handle, char **error); * All registered images have to be closed before calling this. */ void proxmox_backup_finish_async(struct ProxmoxBackupHandle *handle, + bool protected_, void (*callback)(void*), void *callback_data, int *result, diff --git a/simpletest.c b/simpletest.c index ceb5afd..b061bde 100644 --- a/simpletest.c +++ b/simpletest.c @@ -77,7 +77,7 @@ void main(int argc, char **argv) { } printf("finish backup\n"); - if (proxmox_backup_finish(pbs, &pbs_error) < 0) { + if (proxmox_backup_finish(pbs, false, &pbs_error) < 0) { fprintf(stderr, "proxmox_backup_finish failed - %s\n", pbs_error); proxmox_backup_free_error(pbs_error); exit(-1); diff --git a/src/backup.rs b/src/backup.rs index bbe4f00..60ec453 100644 --- a/src/backup.rs +++ b/src/backup.rs @@ -10,8 +10,10 @@ use tokio::runtime::Runtime; use proxmox_async::runtime::get_runtime_with_builder; use proxmox_sys::fs::file_get_contents; -use pbs_api_types::{BackupType, CryptMode}; -use pbs_client::{BackupWriter, HttpClient, HttpClientOptions}; +use pbs_api_types::{BackupType, CryptMode, ServerFeature}; +use pbs_client::{ + tools::get_supported_server_features, BackupWriter, HttpClient, HttpClientOptions, +}; use pbs_datastore::BackupManifest; use pbs_key_config::{load_and_decrypt_key, rsa_encrypt_key_config, KeyConfig}; use pbs_tools::crypt_config::CryptConfig; @@ -35,6 +37,7 @@ pub(crate) struct BackupTask { known_chunks: Arc>>, abort: tokio::sync::broadcast::Sender<()>, aborted: OnceCell, // set on abort, conatins abort reason + server_features: OnceCell>, } impl BackupTask { @@ -92,6 +95,7 @@ impl BackupTask { writer: OnceCell::new(), last_manifest: OnceCell::new(), aborted: OnceCell::new(), + server_features: OnceCell::new(), }) } @@ -141,10 +145,15 @@ impl BackupTask { &self.setup.auth_id, options, )?; + + self.server_features + .set(get_supported_server_features(&http).await) + .map_err(|_| format_err!("already connected!"))?; + let mut backup_dir = self.setup.backup_dir.clone(); backup_dir.group.ty = BackupType::Vm; let writer = BackupWriter::start( - http, + &http, self.crypt_config.clone(), &self.setup.store, &self.setup.backup_ns, @@ -286,14 +295,21 @@ impl BackupTask { abortable_command(command_future, abort_rx.recv()).await } - pub async fn finish(&self) -> Result { + pub async fn finish(&self, protected: bool) -> Result { self.check_aborted()?; + let protected_supported = self + .server_features + .get() + .map(|sf| sf.contains(&ServerFeature::FinishHasProtectedParam)) + .unwrap_or(false); + let command_future = finish_backup( self.need_writer()?, self.crypt_config.clone(), self.rsa_encrypted_key.clone(), Arc::clone(&self.manifest), + protected && protected_supported, ); let mut abort_rx = self.abort.subscribe(); diff --git a/src/commands.rs b/src/commands.rs index 37d653c..4258711 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -468,11 +468,15 @@ pub(crate) async fn write_data( Ok(if reused { 0 } else { size as c_int }) } +/// The caller must ensure that all used server features are actually supported by the server. +/// Currently, this only applies to `protected`. If this is set to true, the server has to support +/// the `FinishHasProtectedParam` feature flag. pub(crate) async fn finish_backup( client: Arc, crypt_config: Option>, rsa_encrypted_key: Option>, manifest: Arc>, + protected: bool, ) -> Result { if let Some(rsa_encrypted_key) = rsa_encrypted_key { let target = ENCRYPTED_KEY_BLOB_NAME; @@ -521,7 +525,11 @@ pub(crate) async fn finish_backup( .upload_blob_from_data(manifest.into_bytes(), MANIFEST_BLOB_NAME, options) .await?; - client.finish().await?; + let mut param = json!({}); + if protected { + param["protected"] = json!(true); + } + client.finish(Some(param)).await?; Ok(0) } diff --git a/src/lib.rs b/src/lib.rs index b3c7b85..2fe234b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -703,6 +703,7 @@ pub extern "C" fn proxmox_backup_close_image_async( #[allow(clippy::not_unsafe_ptr_arg_deref)] pub extern "C" fn proxmox_backup_finish( handle: *mut ProxmoxBackupHandle, + protected: bool, error: *mut *mut c_char, ) -> c_int { let mut result: c_int = -1; @@ -713,6 +714,7 @@ pub extern "C" fn proxmox_backup_finish( proxmox_backup_finish_async( handle, + protected, callback_info.callback, callback_info.callback_data, callback_info.result, @@ -732,6 +734,7 @@ pub extern "C" fn proxmox_backup_finish( #[allow(clippy::not_unsafe_ptr_arg_deref)] pub extern "C" fn proxmox_backup_finish_async( handle: *mut ProxmoxBackupHandle, + protected: bool, callback: extern "C" fn(*mut c_void), callback_data: *mut c_void, result: *mut c_int, @@ -746,7 +749,7 @@ pub extern "C" fn proxmox_backup_finish_async( }; task.runtime().spawn(async move { - let result = task.finish().await; + let result = task.finish(protected).await; callback_info.send_result(result); }); } -- 2.34.1