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 361491FF38F for ; Tue, 21 May 2024 12:29:43 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A4FFF1634D; Tue, 21 May 2024 12:29:58 +0200 (CEST) Message-ID: Date: Tue, 21 May 2024 12:29:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox Backup Server development discussion , Christian Ebner References: <20240514103421.289431-1-c.ebner@proxmox.com> <20240514103421.289431-18-c.ebner@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20240514103421.289431-18-c.ebner@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 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 Subject: Re: [pbs-devel] [PATCH v6 proxmox-backup 17/65] client: pxar: combine writers into struct 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: , Reply-To: Proxmox Backup Server development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" nit inline: On 5/14/24 12:33, Christian Ebner wrote: > Introduce a `PxarWriters` struct to bundle all writer instances > required for the pxar archive creation into a single object to limit > the number of function call parameters, allowing to extend this > further by e.g. the optional payload writer instance. > > Signed-off-by: Christian Ebner > --- > pbs-client/src/pxar/create.rs | 18 ++++++++++++++---- > pbs-client/src/pxar/mod.rs | 2 +- > pbs-client/src/pxar_backup_stream.rs | 5 +++-- > .../src/proxmox_restore_daemon/api.rs | 6 ++++-- > pxar-bin/src/main.rs | 6 +++--- > tests/catar.rs | 3 +-- > 6 files changed, 26 insertions(+), 14 deletions(-) > > diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs > index c9bf6df85..82f05889b 100644 > --- a/pbs-client/src/pxar/create.rs > +++ b/pbs-client/src/pxar/create.rs > @@ -135,12 +135,22 @@ struct Archiver { > > type Encoder<'a, T> = pxar::encoder::aio::Encoder<'a, T>; > > +pub struct PxarWriters { > + writer: T, nit: imho, i wouldn't call that 'writer' as we're already know that from the struct name and often from the variable name (e.g. see below 'writers.writer' is just confusing) how about archive/main/pxar/... ? writers.pxar & writers.catalog is imho more readable than writers.writer & writers.catalog but no hard feelings > + catalog: Option>>, > +} > + > +impl PxarWriters { > + pub fn new(writer: T, catalog: Option>>) -> Self { > + Self { writer, catalog } > + } > +} > + > pub async fn create_archive( > source_dir: Dir, > - mut writer: T, > + mut writers: PxarWriters, > feature_flags: Flags, > callback: F, > - catalog: Option>>, > options: PxarCreateOptions, > ) -> Result<(), Error> > where > @@ -170,7 +180,7 @@ where > set.insert(stat.st_dev); > } > > - let mut encoder = Encoder::new(&mut writer, &metadata, None).await?; > + let mut encoder = Encoder::new(&mut writers.writer, &metadata, None).await?; > > let mut patterns = options.patterns; > > @@ -188,7 +198,7 @@ where > fs_magic, > callback: Box::new(callback), > patterns, > - catalog, > + catalog: writers.catalog, > path: PathBuf::new(), > entry_counter: 0, > entry_limit: options.entries_max, > diff --git a/pbs-client/src/pxar/mod.rs b/pbs-client/src/pxar/mod.rs > index 14674b9b9..b7dcf8362 100644 > --- a/pbs-client/src/pxar/mod.rs > +++ b/pbs-client/src/pxar/mod.rs > @@ -56,7 +56,7 @@ pub(crate) mod tools; > mod flags; > pub use flags::Flags; > > -pub use create::{create_archive, PxarCreateOptions}; > +pub use create::{create_archive, PxarCreateOptions, PxarWriters}; > pub use extract::{ > create_tar, create_zip, extract_archive, extract_sub_dir, extract_sub_dir_seq, ErrorHandler, > OverwriteFlags, PxarExtractContext, PxarExtractOptions, > diff --git a/pbs-client/src/pxar_backup_stream.rs b/pbs-client/src/pxar_backup_stream.rs > index 22a6ffdc2..bfa108a8b 100644 > --- a/pbs-client/src/pxar_backup_stream.rs > +++ b/pbs-client/src/pxar_backup_stream.rs > @@ -17,6 +17,8 @@ use proxmox_io::StdChannelWriter; > > use pbs_datastore::catalog::CatalogWriter; > > +use crate::pxar::create::PxarWriters; > + > /// Stream implementation to encode and upload .pxar archives. > /// > /// The hyper client needs an async Stream for file upload, so we > @@ -56,13 +58,12 @@ impl PxarBackupStream { > let writer = pxar::encoder::sync::StandardWriter::new(writer); > if let Err(err) = crate::pxar::create_archive( > dir, > - writer, > + PxarWriters::new(writer, Some(catalog)), > crate::pxar::Flags::DEFAULT, > move |path| { > log::debug!("{:?}", path); > Ok(()) > }, > - Some(catalog), > options, > ) > .await > diff --git a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs > index c20552225..1ee200573 100644 > --- a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs > +++ b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs > @@ -23,7 +23,9 @@ use proxmox_sortable_macro::sortable; > use proxmox_sys::fs::read_subdir; > > use pbs_api_types::file_restore::{FileRestoreFormat, RestoreDaemonStatus}; > -use pbs_client::pxar::{create_archive, Flags, PxarCreateOptions, ENCODER_MAX_ENTRIES}; > +use pbs_client::pxar::{ > + create_archive, Flags, PxarCreateOptions, PxarWriters, ENCODER_MAX_ENTRIES, > +}; > use pbs_datastore::catalog::{ArchiveEntry, DirEntryAttribute}; > use pbs_tools::json::required_string_param; > > @@ -356,7 +358,7 @@ fn extract( > }; > > let pxar_writer = TokioWriter::new(writer); > - create_archive(dir, pxar_writer, Flags::DEFAULT, |_| Ok(()), None, options) > + create_archive(dir, PxarWriters::new(pxar_writer, None), Flags::DEFAULT, |_| Ok(()), options) > .await > } > .await; > diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs > index d475da4e3..ae2325078 100644 > --- a/pxar-bin/src/main.rs > +++ b/pxar-bin/src/main.rs > @@ -13,7 +13,8 @@ use tokio::signal::unix::{signal, SignalKind}; > > use pathpatterns::{MatchEntry, MatchType, PatternFlag}; > use pbs_client::pxar::{ > - format_single_line_entry, Flags, OverwriteFlags, PxarExtractOptions, ENCODER_MAX_ENTRIES, > + format_single_line_entry, Flags, OverwriteFlags, PxarExtractOptions, PxarWriters, > + ENCODER_MAX_ENTRIES, > }; > > use proxmox_router::cli::*; > @@ -376,13 +377,12 @@ async fn create_archive( > let writer = pxar::encoder::sync::StandardWriter::new(writer); > pbs_client::pxar::create_archive( > dir, > - writer, > + PxarWriters::new(writer, None), > feature_flags, > move |path| { > log::debug!("{:?}", path); > Ok(()) > }, > - None, > options, > ) > .await?; > diff --git a/tests/catar.rs b/tests/catar.rs > index 36bb4f3bc..f414da8c9 100644 > --- a/tests/catar.rs > +++ b/tests/catar.rs > @@ -35,10 +35,9 @@ fn run_test(dir_name: &str) -> Result<(), Error> { > let rt = tokio::runtime::Runtime::new().unwrap(); > rt.block_on(create_archive( > dir, > - writer, > + PxarWriters::new(writer, None), > Flags::DEFAULT, > |_| Ok(()), > - None, > options, > ))?; > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel