[pbs-devel] [PATCH proxmox-backup v11 34/46] backup writer: refactor parameters into backup writer options struct

[Christian Ebner <c.ebner@proxmox.com>]

Introduce a BackupWriterOptions struct, bundling the currently
present writer start parameters n order to limit their number
and make this easier extensible.

No functional changes intended.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since version 10:
 - no changes

 examples/upload-speed.rs               | 16 ++++++----
 pbs-client/src/backup_writer.rs        | 44 ++++++++++++++++----------
 proxmox-backup-client/src/benchmark.rs | 16 ++++++----
 proxmox-backup-client/src/main.rs      | 18 ++++++-----
 src/server/push.rs                     | 18 +++++++----
 5 files changed, 66 insertions(+), 46 deletions(-)

diff --git a/examples/upload-speed.rs b/examples/upload-speed.rs
index e4b570ec5..bbabb37df 100644
--- a/examples/upload-speed.rs
+++ b/examples/upload-speed.rs
@@ -1,7 +1,7 @@
 use anyhow::Error;
 
 use pbs_api_types::{Authid, BackupNamespace, BackupType};
-use pbs_client::{BackupWriter, HttpClient, HttpClientOptions};
+use pbs_client::{BackupWriter, BackupWriterOptions, HttpClient, HttpClientOptions};
 
 async fn upload_speed() -> Result<f64, Error> {
     let host = "localhost";
@@ -19,12 +19,14 @@ async fn upload_speed() -> Result<f64, Error> {
 
     let client = BackupWriter::start(
         &client,
-        None,
-        datastore,
-        &BackupNamespace::root(),
-        &(BackupType::Host, "speedtest".to_string(), backup_time).into(),
-        false,
-        true,
+        BackupWriterOptions {
+            datastore,
+            ns: &BackupNamespace::root(),
+            backup: &(BackupType::Host, "speedtest".to_string(), backup_time).into(),
+            crypt_config: None,
+            debug: false,
+            benchmark: true,
+        },
     )
     .await?;
 
diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
index 1253ef561..853b1cb4f 100644
--- a/pbs-client/src/backup_writer.rs
+++ b/pbs-client/src/backup_writer.rs
@@ -63,6 +63,22 @@ struct ChunkUploadResponse {
 type UploadQueueSender = mpsc::Sender<(MergedChunkInfo, Option<ChunkUploadResponse>)>;
 type UploadResultReceiver = oneshot::Receiver<Result<(), Error>>;
 
+/// Additional configuration options for BackupWriter instance
+pub struct BackupWriterOptions<'a> {
+    /// Target datastore
+    pub datastore: &'a str,
+    /// Target namespace
+    pub ns: &'a BackupNamespace,
+    /// Target snapshot
+    pub backup: &'a BackupDir,
+    /// Crypto configuration
+    pub crypt_config: Option<Arc<CryptConfig>>,
+    /// Run in debug mode
+    pub debug: bool,
+    /// Start benchmark
+    pub benchmark: bool,
+}
+
 impl BackupWriter {
     fn new(h2: H2Client, abort: AbortHandle, crypt_config: Option<Arc<CryptConfig>>) -> Arc<Self> {
         Arc::new(Self {
@@ -72,28 +88,22 @@ impl BackupWriter {
         })
     }
 
-    // FIXME: extract into (flattened) parameter struct?
     #[allow(clippy::too_many_arguments)]
-    pub async fn start(
+    pub async fn start<'a>(
         client: &HttpClient,
-        crypt_config: Option<Arc<CryptConfig>>,
-        datastore: &str,
-        ns: &BackupNamespace,
-        backup: &BackupDir,
-        debug: bool,
-        benchmark: bool,
+        writer_options: BackupWriterOptions<'a>,
     ) -> Result<Arc<BackupWriter>, Error> {
         let mut param = json!({
-            "backup-type": backup.ty(),
-            "backup-id": backup.id(),
-            "backup-time": backup.time,
-            "store": datastore,
-            "debug": debug,
-            "benchmark": benchmark
+            "backup-type": writer_options.backup.ty(),
+            "backup-id": writer_options.backup.id(),
+            "backup-time": writer_options.backup.time,
+            "store": writer_options.datastore,
+            "debug": writer_options.debug,
+            "benchmark": writer_options.benchmark,
         });
 
-        if !ns.is_root() {
-            param["ns"] = serde_json::to_value(ns)?;
+        if !writer_options.ns.is_root() {
+            param["ns"] = serde_json::to_value(writer_options.ns)?;
         }
 
         let req = HttpClient::request_builder(
@@ -109,7 +119,7 @@ impl BackupWriter {
             .start_h2_connection(req, String::from(PROXMOX_BACKUP_PROTOCOL_ID_V1!()))
             .await?;
 
-        Ok(BackupWriter::new(h2, abort, crypt_config))
+        Ok(BackupWriter::new(h2, abort, writer_options.crypt_config))
     }
 
     pub async fn get(&self, path: &str, param: Option<Value>) -> Result<Value, Error> {
diff --git a/proxmox-backup-client/src/benchmark.rs b/proxmox-backup-client/src/benchmark.rs
index a6f24d745..6b11e216d 100644
--- a/proxmox-backup-client/src/benchmark.rs
+++ b/proxmox-backup-client/src/benchmark.rs
@@ -16,7 +16,7 @@ use proxmox_schema::{api, ApiType, ReturnType};
 
 use pbs_api_types::{BackupNamespace, BackupType};
 use pbs_client::tools::key_source::get_encryption_key_password;
-use pbs_client::{BackupRepository, BackupWriter};
+use pbs_client::{BackupRepository, BackupWriter, BackupWriterOptions};
 use pbs_datastore::data_blob::{DataBlob, DataChunkBuilder};
 use pbs_key_config::{load_and_decrypt_key, KeyDerivationConfig};
 use pbs_tools::crypt_config::CryptConfig;
@@ -230,12 +230,14 @@ async fn test_upload_speed(
     log::debug!("Connecting to backup server");
     let client = BackupWriter::start(
         &client,
-        crypt_config.clone(),
-        repo.store(),
-        &BackupNamespace::root(),
-        &(BackupType::Host, "benchmark".to_string(), backup_time).into(),
-        false,
-        true,
+        BackupWriterOptions {
+            datastore: repo.store(),
+            ns: &BackupNamespace::root(),
+            backup: &(BackupType::Host, "benchmark".to_string(), backup_time).into(),
+            crypt_config: crypt_config.clone(),
+            debug: false,
+            benchmark: true,
+        },
     )
     .await?;
 
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 44f4f5db5..44c076627 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -46,8 +46,8 @@ use pbs_client::tools::{
 use pbs_client::{
     delete_ticket_info, parse_backup_specification, view_task_result, BackupDetectionMode,
     BackupReader, BackupRepository, BackupSpecificationType, BackupStats, BackupWriter,
-    ChunkStream, FixedChunkStream, HttpClient, InjectionData, PxarBackupStream, RemoteChunkReader,
-    UploadOptions, BACKUP_SOURCE_SCHEMA,
+    BackupWriterOptions, ChunkStream, FixedChunkStream, HttpClient, InjectionData,
+    PxarBackupStream, RemoteChunkReader, UploadOptions, BACKUP_SOURCE_SCHEMA,
 };
 use pbs_datastore::catalog::{BackupCatalogWriter, CatalogReader, CatalogWriter};
 use pbs_datastore::chunk_store::verify_chunk_size;
@@ -954,12 +954,14 @@ async fn create_backup(
 
     let client = BackupWriter::start(
         &http_client,
-        crypt_config.clone(),
-        repo.store(),
-        &backup_ns,
-        &snapshot,
-        true,
-        false,
+        BackupWriterOptions {
+            datastore: repo.store(),
+            ns: &backup_ns,
+            backup: &snapshot,
+            crypt_config: crypt_config.clone(),
+            debug: true,
+            benchmark: false,
+        },
     )
     .await?;
 
diff --git a/src/server/push.rs b/src/server/push.rs
index 528eed9ff..c78063662 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -16,7 +16,9 @@ use pbs_api_types::{
     MANIFEST_BLOB_NAME, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_READ, PRIV_REMOTE_DATASTORE_BACKUP,
     PRIV_REMOTE_DATASTORE_MODIFY, PRIV_REMOTE_DATASTORE_PRUNE,
 };
-use pbs_client::{BackupRepository, BackupWriter, HttpClient, MergedChunkInfo, UploadOptions};
+use pbs_client::{
+    BackupRepository, BackupWriter, BackupWriterOptions, HttpClient, MergedChunkInfo, UploadOptions,
+};
 use pbs_config::CachedUserInfo;
 use pbs_datastore::data_blob::ChunkInfo;
 use pbs_datastore::dynamic_index::DynamicIndexReader;
@@ -822,12 +824,14 @@ pub(crate) async fn push_snapshot(
     // Writer instance locks the snapshot on the remote side
     let backup_writer = BackupWriter::start(
         &params.target.client,
-        None,
-        params.target.repo.store(),
-        &target_ns,
-        snapshot,
-        false,
-        false,
+        BackupWriterOptions {
+            datastore: params.target.repo.store(),
+            ns: &target_ns,
+            backup: snapshot,
+            crypt_config: None,
+            debug: false,
+            benchmark: false,
+        },
     )
     .await?;
 
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel