public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings
@ 2021-01-25 13:42 Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 01/15] report: type-alias function call tuple Fabian Grünbichler
                   ` (17 more replies)
  0 siblings, 18 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

various parameter/types refactorings to simplify function
signatures/return types/...

I am grateful for suggestions on better names ;)

I tried to order/group the patches so that they also apply
individually/as sub-groups.

with all of these, we are now down to some very recently touched files
with trivial fixes that I didn't want to change since I suspected them
to be still under development, and one missing docs lint:

warning: unsafe function's docs miss `# Safety` section
  --> src/tools/fs.rs:78:5
   |
78 | /     pub unsafe fn file_name_utf8_unchecked(&self) -> &str {
79 | |         std::str::from_utf8_unchecked(self.file_name().to_bytes())
80 | |     }
   | |_____^

proxmox-backup:
  report: type-alias function call tuple
  broadcast_future: refactor broadcast/future binding
  client: refactor catalog upload spawning
  allow complex Futures in tower_service impl
  async index reader: typedef ReadFuture
  systemd/time: extract Time/DateSpec structs
  client: factor out UploadOptions
  pxar: typedef on_error as ErrorHandler
  pxar: factor out PxarCreateOptions
  pxar: extract PxarExtractOptions
  authid: make Tokenname(Ref) derive Eq
  derive/impl and use Default for some structs
  verify: factor out common parameters
  clippy: allow api functions with many arguments
  clippy: more misc fixes

 examples/download-speed.rs             |   2 +-
 examples/upload-speed.rs               |   2 +-
 src/api2/access/acl.rs                 |   1 +
 src/api2/access/tfa.rs                 |   1 +
 src/api2/access/user.rs                |   1 +
 src/api2/admin/datastore.rs            |  19 +--
 src/api2/backup/environment.rs         |  10 +-
 src/api2/config/datastore.rs           |   1 +
 src/api2/config/remote.rs              |   5 +-
 src/api2/config/sync.rs                |   1 +
 src/api2/config/verify.rs              |   1 +
 src/api2/node/network.rs               |   2 +
 src/api2/node/tasks.rs                 |   1 +
 src/api2/types/userid.rs               |  52 +-------
 src/backup/async_index_reader.rs       |   4 +-
 src/backup/prune.rs                    |   1 +
 src/backup/verify.rs                   | 174 +++++++++++--------------
 src/bin/proxmox-backup-client.rs       | 162 +++++++++++++----------
 src/bin/proxmox-daily-update.rs        |   9 +-
 src/bin/proxmox_backup_client/key.rs   |  24 +---
 src/bin/proxmox_backup_client/mount.rs |   2 +-
 src/bin/proxmox_backup_manager/user.rs |   4 +-
 src/bin/pxar.rs                        |  54 ++++----
 src/client.rs                          |  11 +-
 src/client/backup_writer.rs            |  46 ++++---
 src/client/http_client.rs              |  38 ++++--
 src/client/pull.rs                     |   4 +-
 src/client/pxar_backup_stream.rs       |  28 +---
 src/config/acl.rs                      |   8 +-
 src/config/network.rs                  |   2 +-
 src/pxar/create.rs                     |  28 +++-
 src/pxar/extract.rs                    |  24 ++--
 src/pxar/mod.rs                        |   4 +-
 src/rrd/mod.rs                         |   1 +
 src/server/h2service.rs                |   5 +-
 src/server/report.rs                   |   6 +-
 src/server/rest.rs                     |   3 +-
 src/server/verify_job.rs               |   3 +-
 src/tools/broadcast_future.rs          |  38 ++----
 src/tools/http.rs                      |   5 +-
 src/tools/systemd/parse_time.rs        |  41 ++++--
 tests/catar.rs                         |  10 +-
 42 files changed, 414 insertions(+), 424 deletions(-)

proxmox-backup-qemu:
  use UploadOptions for uploading Blobs
  use new HttpClientOptions constructors

 src/backup.rs   |  7 ++++---
 src/commands.rs | 15 +++++++++++++--
 src/lib.rs      |  8 +++++---
 src/restore.rs  |  7 ++++---
 4 files changed, 26 insertions(+), 11 deletions(-)

-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 01/15] report: type-alias function call tuple
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 02/15] broadcast_future: refactor broadcast/future binding Fabian Grünbichler
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

to make clippy happy.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
could also be just #[allow(..)]ed

 src/server/report.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/server/report.rs b/src/server/report.rs
index 1b77f442..e89abbc5 100644
--- a/src/server/report.rs
+++ b/src/server/report.rs
@@ -29,8 +29,10 @@ fn commands() -> Vec<(&'static str, Vec<&'static str>)> {
     ]
 }
 
-    // (<description>, <function to call>)
-fn function_calls() -> Vec<(&'static str, fn() -> String)> {
+// (description, function())
+type FunctionMapping = (&'static str, fn() -> String);
+
+fn function_calls() -> Vec<FunctionMapping> {
     vec![
         ("Datastores", || {
             let config = match datastore::config() {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 02/15] broadcast_future: refactor broadcast/future binding
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 01/15] report: type-alias function call tuple Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 03/15] client: refactor catalog upload spawning Fabian Grünbichler
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

into its own, private struct.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/tools/broadcast_future.rs | 42 +++++++++++++++--------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/src/tools/broadcast_future.rs b/src/tools/broadcast_future.rs
index 94aedf18..88b7aaab 100644
--- a/src/tools/broadcast_future.rs
+++ b/src/tools/broadcast_future.rs
@@ -62,14 +62,16 @@ impl <T: Clone> BroadcastData<T> {
     }
 }
 
+type SourceFuture<T> = Pin<Box<dyn Future<Output = Result<T, Error>> + Send>>;
+
+struct BroadCastFutureBinding<T> {
+    broadcast: BroadcastData<T>,
+    future: Option<SourceFuture<T>>,
+}
+
 /// Broadcast future results to registered listeners
 pub struct BroadcastFuture<T> {
-    inner: Arc<
-        Mutex<(
-            BroadcastData<T>,
-            Option<Pin<Box<dyn Future<Output = Result<T, Error>> + Send>>>,
-        )>,
-    >,
+    inner: Arc<Mutex<BroadCastFutureBinding<T>>>,
 }
 
 impl<T: Clone + Send + 'static> BroadcastFuture<T> {
@@ -77,7 +79,11 @@ impl<T: Clone + Send + 'static> BroadcastFuture<T> {
     ///
     /// The result of the future is sent to all registered listeners.
     pub fn new(source: Box<dyn Future<Output = Result<T, Error>> + Send>) -> Self {
-        Self { inner: Arc::new(Mutex::new((BroadcastData::new(), Some(Pin::from(source))))) }
+        let inner = BroadCastFutureBinding {
+            broadcast: BroadcastData::new(),
+            future: Some(Pin::from(source)),
+        };
+        Self { inner: Arc::new(Mutex::new(inner)) }
     }
 
     /// Creates a new instance with a oneshot channel as trigger
@@ -92,29 +98,17 @@ impl<T: Clone + Send + 'static> BroadcastFuture<T> {
     }
 
     fn notify_listeners(
-        inner: Arc<
-            Mutex<(
-                BroadcastData<T>,
-                Option<Pin<Box<dyn Future<Output = Result<T, Error>> + Send>>>,
-            )>,
-        >,
+        inner: Arc<Mutex<BroadCastFutureBinding<T>>>,
         result: Result<T, String>,
     ) {
         let mut data = inner.lock().unwrap();
-        data.0.notify_listeners(result);
+        data.broadcast.notify_listeners(result);
     }
 
-    fn spawn(
-        inner: Arc<
-            Mutex<(
-                BroadcastData<T>,
-                Option<Pin<Box<dyn Future<Output = Result<T, Error>> + Send>>>,
-            )>,
-        >,
-    ) -> impl Future<Output = Result<T, Error>> {
+    fn spawn(inner: Arc<Mutex<BroadCastFutureBinding<T>>>) -> impl Future<Output = Result<T, Error>> {
         let mut data = inner.lock().unwrap();
 
-        if let Some(source) = data.1.take() {
+        if let Some(source) = data.future.take() {
 
             let inner1 = inner.clone();
 
@@ -127,7 +121,7 @@ impl<T: Clone + Send + 'static> BroadcastFuture<T> {
             tokio::spawn(task);
         }
 
-        data.0.listen()
+        data.broadcast.listen()
     }
 
     /// Register a listener
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 03/15] client: refactor catalog upload spawning
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 01/15] report: type-alias function call tuple Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 02/15] broadcast_future: refactor broadcast/future binding Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 04/15] allow complex Futures in tower_service impl Fabian Grünbichler
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

by pulling out Result type into separate struct

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
especially unhappy with the name here..

 src/bin/proxmox-backup-client.rs | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 041a670c..dce8f0b8 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -586,21 +586,21 @@ async fn start_garbage_collection(param: Value) -> Result<Value, Error> {
     Ok(Value::Null)
 }
 
+struct CatalogUploadResult {
+    catalog_writer: Arc<Mutex<CatalogWriter<crate::tools::StdChannelWriter>>>,
+    result: tokio::sync::oneshot::Receiver<Result<BackupStats, Error>>,
+}
+
 fn spawn_catalog_upload(
     client: Arc<BackupWriter>,
     encrypt: bool,
-) -> Result<
-        (
-            Arc<Mutex<CatalogWriter<crate::tools::StdChannelWriter>>>,
-            tokio::sync::oneshot::Receiver<Result<BackupStats, Error>>
-        ), Error>
-{
+) -> Result<CatalogUploadResult, Error> {
     let (catalog_tx, catalog_rx) = std::sync::mpsc::sync_channel(10); // allow to buffer 10 writes
     let catalog_stream = crate::tools::StdChannelStream(catalog_rx);
     let catalog_chunk_size = 512*1024;
     let catalog_chunk_stream = ChunkStream::new(catalog_stream, Some(catalog_chunk_size));
 
-    let catalog = Arc::new(Mutex::new(CatalogWriter::new(crate::tools::StdChannelWriter::new(catalog_tx))?));
+    let catalog_writer = Arc::new(Mutex::new(CatalogWriter::new(crate::tools::StdChannelWriter::new(catalog_tx))?));
 
     let (catalog_result_tx, catalog_result_rx) = tokio::sync::oneshot::channel();
 
@@ -617,7 +617,7 @@ fn spawn_catalog_upload(
         let _ = catalog_result_tx.send(catalog_upload_result);
     });
 
-    Ok((catalog, catalog_result_rx))
+    Ok(CatalogUploadResult { catalog_writer, result: catalog_result_rx })
 }
 
 fn keyfile_parameters(param: &Value) -> Result<(Option<Vec<u8>>, CryptMode), Error> {
@@ -990,7 +990,7 @@ async fn create_backup(
     let mut manifest = BackupManifest::new(snapshot);
 
     let mut catalog = None;
-    let mut catalog_result_tx = None;
+    let mut catalog_result_rx = None;
 
     for (backup_type, filename, target, size) in upload_list {
         match backup_type {
@@ -1011,9 +1011,9 @@ async fn create_backup(
             BackupSpecificationType::PXAR => {
                 // start catalog upload on first use
                 if catalog.is_none() {
-                    let (cat, res) = spawn_catalog_upload(client.clone(), crypt_mode == CryptMode::Encrypt)?;
-                    catalog = Some(cat);
-                    catalog_result_tx = Some(res);
+                    let catalog_upload_res = spawn_catalog_upload(client.clone(), crypt_mode == CryptMode::Encrypt)?;
+                    catalog = Some(catalog_upload_res.catalog_writer);
+                    catalog_result_rx = Some(catalog_upload_res.result);
                 }
                 let catalog = catalog.as_ref().unwrap();
 
@@ -1065,7 +1065,7 @@ async fn create_backup(
 
         drop(catalog); // close upload stream
 
-        if let Some(catalog_result_rx) = catalog_result_tx {
+        if let Some(catalog_result_rx) = catalog_result_rx {
             let stats = catalog_result_rx.await??;
             manifest.add_file(CATALOG_NAME.to_owned(), stats.size, stats.csum, crypt_mode)?;
         }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 04/15] allow complex Futures in tower_service impl
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (2 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 03/15] client: refactor catalog upload spawning Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 05/15] async index reader: typedef ReadFuture Fabian Grünbichler
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/server/h2service.rs | 5 +++--
 src/server/rest.rs      | 3 ++-
 src/tools/http.rs       | 5 ++---
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/server/h2service.rs b/src/server/h2service.rs
index 989618ec..332b3b1a 100644
--- a/src/server/h2service.rs
+++ b/src/server/h2service.rs
@@ -1,6 +1,7 @@
 use anyhow::{Error};
 
 use std::collections::HashMap;
+use std::pin::Pin;
 use std::sync::Arc;
 use std::task::{Context, Poll};
 
@@ -85,8 +86,8 @@ impl <E: RpcEnvironment + Clone> H2Service<E> {
 impl <E: RpcEnvironment + Clone> tower_service::Service<Request<Body>> for H2Service<E> {
     type Response = Response<Body>;
     type Error = Error;
-    type Future =
-        std::pin::Pin<Box<dyn Future<Output = Result<Response<Body>, Self::Error>> + Send>>;
+    #[allow(clippy::type_complexity)]
+    type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send>>;
 
     fn poll_ready(&mut self, _cx: &mut Context) -> Poll<Result<(), Self::Error>> {
         Poll::Ready(Ok(()))
diff --git a/src/server/rest.rs b/src/server/rest.rs
index 0586979a..fc59be9a 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -198,7 +198,8 @@ fn get_user_agent(headers: &HeaderMap) -> Option<String> {
 impl tower_service::Service<Request<Body>> for ApiService {
     type Response = Response<Body>;
     type Error = Error;
-    type Future = Pin<Box<dyn Future<Output = Result<Response<Body>, Self::Error>> + Send>>;
+    #[allow(clippy::type_complexity)]
+    type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send>>;
 
     fn poll_ready(&mut self, _cx: &mut Context) -> Poll<Result<(), Self::Error>> {
         Poll::Ready(Ok(()))
diff --git a/src/tools/http.rs b/src/tools/http.rs
index 47d6e1f6..0fbc85fb 100644
--- a/src/tools/http.rs
+++ b/src/tools/http.rs
@@ -108,9 +108,8 @@ type MaybeTlsStream = EitherStream<
 impl hyper::service::Service<Uri> for HttpsConnector {
     type Response = MaybeTlsStream;
     type Error = Error;
-    type Future = std::pin::Pin<Box<
-        dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static
-    >>;
+    #[allow(clippy::type_complexity)]
+    type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static>>;
 
     fn poll_ready(&mut self, _: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
         // This connector is always ready, but others might not be.
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 05/15] async index reader: typedef ReadFuture
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (3 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 04/15] allow complex Futures in tower_service impl Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 06/15] systemd/time: extract Time/DateSpec structs Fabian Grünbichler
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
there is also still a FIXME here..

 src/backup/async_index_reader.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/backup/async_index_reader.rs b/src/backup/async_index_reader.rs
index 60c68d4f..20a37e7e 100644
--- a/src/backup/async_index_reader.rs
+++ b/src/backup/async_index_reader.rs
@@ -15,6 +15,8 @@ use super::IndexFile;
 use super::read_chunk::AsyncReadChunk;
 use super::index::ChunkReadInfo;
 
+type ReadFuture<S> = dyn Future<Output = Result<(S, Vec<u8>), Error>> + Send + 'static;
+
 // FIXME: This enum may not be required?
 // - Put the `WaitForData` case directly into a `read_future: Option<>`
 // - make the read loop as follows:
@@ -28,7 +30,7 @@ use super::index::ChunkReadInfo;
 #[allow(clippy::enum_variant_names)]
 enum AsyncIndexReaderState<S> {
     NoData,
-    WaitForData(Pin<Box<dyn Future<Output = Result<(S, Vec<u8>), Error>> + Send + 'static>>),
+    WaitForData(Pin<Box<ReadFuture<S>>>),
     HaveData,
 }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 06/15] systemd/time: extract Time/DateSpec structs
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (4 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 05/15] async index reader: typedef ReadFuture Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 07/15] client: factor out UploadOptions Fabian Grünbichler
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

could be pulled up into CalendarEvent if desired..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/tools/systemd/parse_time.rs | 41 ++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/src/tools/systemd/parse_time.rs b/src/tools/systemd/parse_time.rs
index 88f5c92d..c0f58b04 100644
--- a/src/tools/systemd/parse_time.rs
+++ b/src/tools/systemd/parse_time.rs
@@ -86,6 +86,19 @@ lazy_static! {
         map
     };
 }
+
+struct TimeSpec {
+    hour: Vec<DateTimeValue>,
+    minute: Vec<DateTimeValue>,
+    second: Vec<DateTimeValue>,
+}
+
+struct DateSpec {
+    year: Vec<DateTimeValue>,
+    month: Vec<DateTimeValue>,
+    day: Vec<DateTimeValue>,
+}
+
 fn parse_time_comp(max: usize) -> impl Fn(&str) -> IResult<&str, u32> {
     move |i: &str| {
         let (i, v) = map_res(recognize(digit1), str::parse)(i)?;
@@ -176,7 +189,7 @@ fn parse_date_time_comp_list(start: u32, max: usize) -> impl Fn(&str) -> IResult
     }
 }
 
-fn parse_time_spec(i: &str) -> IResult<&str, (Vec<DateTimeValue>, Vec<DateTimeValue>, Vec<DateTimeValue>)> {
+fn parse_time_spec(i: &str) -> IResult<&str, TimeSpec> {
 
     let (i, (hour, minute, opt_second)) = tuple((
         parse_date_time_comp_list(0, 24),
@@ -185,13 +198,13 @@ fn parse_time_spec(i: &str) -> IResult<&str, (Vec<DateTimeValue>, Vec<DateTimeVa
     ))(i)?;
 
     if let Some(second) = opt_second {
-        Ok((i, (hour, minute, second)))
+        Ok((i, TimeSpec { hour, minute, second }))
     } else {
-        Ok((i, (hour, minute, vec![DateTimeValue::Single(0)])))
+        Ok((i, TimeSpec { hour, minute, second: vec![DateTimeValue::Single(0)] }))
     }
 }
 
-fn parse_date_spec(i: &str) -> IResult<&str, (Vec<DateTimeValue>, Vec<DateTimeValue>, Vec<DateTimeValue>)> {
+fn parse_date_spec(i: &str) -> IResult<&str, DateSpec> {
 
     // TODO: implement ~ for days (man systemd.time)
     if let Ok((i, (year, month, day))) = tuple((
@@ -199,12 +212,12 @@ fn parse_date_spec(i: &str) -> IResult<&str, (Vec<DateTimeValue>, Vec<DateTimeVa
         preceded(tag("-"), parse_date_time_comp_list(1, 13)),
         preceded(tag("-"), parse_date_time_comp_list(1, 32)),
     ))(i) {
-        Ok((i, (year, month, day)))
+        Ok((i, DateSpec { year, month, day }))
     } else if let Ok((i, (month, day))) = tuple((
         parse_date_time_comp_list(1, 13),
         preceded(tag("-"), parse_date_time_comp_list(1, 32)),
     ))(i) {
-        Ok((i, (Vec::new(), month, day)))
+        Ok((i, DateSpec { year: Vec::new(), month, day }))
     } else {
         Err(parse_error(i, "invalid date spec"))
     }
@@ -317,18 +330,18 @@ fn parse_calendar_event_incomplete(mut i: &str) -> IResult<&str, CalendarEvent>
         for range in range_list  { event.days.insert(range); }
     }
 
-    if let (n, Some((year, month, day))) = opt(parse_date_spec)(i)? {
-        event.year = year;
-        event.month = month;
-        event.day = day;
+    if let (n, Some(date)) = opt(parse_date_spec)(i)? {
+        event.year = date.year;
+        event.month = date.month;
+        event.day = date.day;
         has_datespec = true;
         i = space0(n)?.0;
     }
 
-    if let (n, Some((hour, minute, second))) = opt(parse_time_spec)(i)? {
-        event.hour = hour;
-        event.minute = minute;
-        event.second = second;
+    if let (n, Some(time)) = opt(parse_time_spec)(i)? {
+        event.hour = time.hour;
+        event.minute = time.minute;
+        event.second = time.second;
         has_timespec = true;
         i = n;
     } else {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 07/15] client: factor out UploadOptions
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (5 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 06/15] systemd/time: extract Time/DateSpec structs Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 08/15] pxar: typedef on_error as ErrorHandler Fabian Grünbichler
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

to reduce function signature complexity.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
breaks/requires corresponding patch in proxmox-backup-qemu

 src/bin/proxmox-backup-client.rs | 79 ++++++++++++++++++++++----------
 src/client/backup_writer.rs      | 44 ++++++++++--------
 2 files changed, 80 insertions(+), 43 deletions(-)

diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index dce8f0b8..af3b8464 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -280,7 +280,6 @@ pub async fn api_datastore_latest_snapshot(
 
 async fn backup_directory<P: AsRef<Path>>(
     client: &BackupWriter,
-    previous_manifest: Option<Arc<BackupManifest>>,
     dir_path: P,
     archive_name: &str,
     chunk_size: Option<usize>,
@@ -290,8 +289,7 @@ async fn backup_directory<P: AsRef<Path>>(
     catalog: Arc<Mutex<CatalogWriter<crate::tools::StdChannelWriter>>>,
     exclude_pattern: Vec<MatchEntry>,
     entries_max: usize,
-    compress: bool,
-    encrypt: bool,
+    upload_options: UploadOptions,
 ) -> Result<BackupStats, Error> {
 
     let pxar_stream = PxarBackupStream::open(
@@ -317,8 +315,12 @@ async fn backup_directory<P: AsRef<Path>>(
         }
     });
 
+    if upload_options.fixed_size.is_some() {
+        bail!("cannot backup directory with fixed chunk size!");
+    }
+
     let stats = client
-        .upload_stream(previous_manifest, archive_name, stream, "dynamic", None, compress, encrypt)
+        .upload_stream(archive_name, stream, upload_options)
         .await?;
 
     Ok(stats)
@@ -326,14 +328,10 @@ async fn backup_directory<P: AsRef<Path>>(
 
 async fn backup_image<P: AsRef<Path>>(
     client: &BackupWriter,
-    previous_manifest: Option<Arc<BackupManifest>>,
     image_path: P,
     archive_name: &str,
-    image_size: u64,
     chunk_size: Option<usize>,
-    compress: bool,
-    encrypt: bool,
-    _verbose: bool,
+    upload_options: UploadOptions,
 ) -> Result<BackupStats, Error> {
 
     let path = image_path.as_ref().to_owned();
@@ -345,8 +343,12 @@ async fn backup_image<P: AsRef<Path>>(
 
     let stream = FixedChunkStream::new(stream, chunk_size.unwrap_or(4*1024*1024));
 
+    if upload_options.fixed_size.is_none() {
+        bail!("cannot backup image with dynamic chunk size!");
+    }
+
     let stats = client
-        .upload_stream(previous_manifest, archive_name, stream, "fixed", Some(image_size), compress, encrypt)
+        .upload_stream(archive_name, stream, upload_options)
         .await?;
 
     Ok(stats)
@@ -604,9 +606,15 @@ fn spawn_catalog_upload(
 
     let (catalog_result_tx, catalog_result_rx) = tokio::sync::oneshot::channel();
 
+    let upload_options = UploadOptions {
+        encrypt,
+        compress: true,
+        ..UploadOptions::default()
+    };
+
     tokio::spawn(async move {
         let catalog_upload_result = client
-            .upload_stream(None, CATALOG_NAME, catalog_chunk_stream, "dynamic", None, true, encrypt)
+            .upload_stream(CATALOG_NAME, catalog_chunk_stream, upload_options)
             .await;
 
         if let Err(ref err) = catalog_upload_result {
@@ -995,16 +1003,28 @@ async fn create_backup(
     for (backup_type, filename, target, size) in upload_list {
         match backup_type {
             BackupSpecificationType::CONFIG => {
+                let upload_options = UploadOptions {
+                    compress: true,
+                    encrypt: crypt_mode == CryptMode::Encrypt,
+                    ..UploadOptions::default()
+                };
+
                 println!("Upload config file '{}' to '{}' as {}", filename, repo, target);
                 let stats = client
-                    .upload_blob_from_file(&filename, &target, true, crypt_mode == CryptMode::Encrypt)
+                    .upload_blob_from_file(&filename, &target, upload_options)
                     .await?;
                 manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
             }
             BackupSpecificationType::LOGFILE => { // fixme: remove - not needed anymore ?
+                let upload_options = UploadOptions {
+                    compress: true,
+                    encrypt: crypt_mode == CryptMode::Encrypt,
+                    ..UploadOptions::default()
+                };
+
                 println!("Upload log file '{}' to '{}' as {}", filename, repo, target);
                 let stats = client
-                    .upload_blob_from_file(&filename, &target, true, crypt_mode == CryptMode::Encrypt)
+                    .upload_blob_from_file(&filename, &target, upload_options)
                     .await?;
                 manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
             }
@@ -1019,9 +1039,15 @@ async fn create_backup(
 
                 println!("Upload directory '{}' to '{}' as {}", filename, repo, target);
                 catalog.lock().unwrap().start_directory(std::ffi::CString::new(target.as_str())?.as_c_str())?;
+                let upload_options = UploadOptions {
+                    previous_manifest: previous_manifest.clone(),
+                    compress: true,
+                    encrypt: crypt_mode == CryptMode::Encrypt,
+                    ..UploadOptions::default()
+                };
+
                 let stats = backup_directory(
                     &client,
-                    previous_manifest.clone(),
                     &filename,
                     &target,
                     chunk_size_opt,
@@ -1031,24 +1057,27 @@ async fn create_backup(
                     catalog.clone(),
                     pattern_list.clone(),
                     entries_max as usize,
-                    true,
-                    crypt_mode == CryptMode::Encrypt,
+                    upload_options,
                 ).await?;
                 manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
                 catalog.lock().unwrap().end_directory()?;
             }
             BackupSpecificationType::IMAGE => {
                 println!("Upload image '{}' to '{:?}' as {}", filename, repo, target);
+
+                let upload_options = UploadOptions {
+                    previous_manifest: previous_manifest.clone(),
+                    fixed_size: Some(size),
+                    compress: true,
+                    encrypt: crypt_mode == CryptMode::Encrypt,
+                };
+
                 let stats = backup_image(
                     &client,
-                    previous_manifest.clone(),
-                     &filename,
+                    &filename,
                     &target,
-                    size,
                     chunk_size_opt,
-                    true,
-                    crypt_mode == CryptMode::Encrypt,
-                    verbose,
+                    upload_options,
                 ).await?;
                 manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
             }
@@ -1074,8 +1103,9 @@ async fn create_backup(
     if let Some(rsa_encrypted_key) = rsa_encrypted_key {
         let target = ENCRYPTED_KEY_BLOB_NAME;
         println!("Upload RSA encoded key to '{:?}' as {}", repo, target);
+        let options = UploadOptions { compress: false, encrypt: false, ..UploadOptions::default() };
         let stats = client
-            .upload_blob_from_data(rsa_encrypted_key, target, false, false)
+            .upload_blob_from_data(rsa_encrypted_key, target, options)
             .await?;
         manifest.add_file(target.to_string(), stats.size, stats.csum, crypt_mode)?;
 
@@ -1087,8 +1117,9 @@ async fn create_backup(
 
 
     if verbose { println!("Upload index.json to '{}'", repo) };
+    let options = UploadOptions { compress: true, encrypt: false, ..UploadOptions::default() };
     client
-        .upload_blob_from_data(manifest.into_bytes(), MANIFEST_BLOB_NAME, true, false)
+        .upload_blob_from_data(manifest.into_bytes(), MANIFEST_BLOB_NAME, options)
         .await?;
 
     client.finish().await?;
diff --git a/src/client/backup_writer.rs b/src/client/backup_writer.rs
index e7a6d6bd..38953a45 100644
--- a/src/client/backup_writer.rs
+++ b/src/client/backup_writer.rs
@@ -39,6 +39,15 @@ pub struct BackupStats {
     pub csum: [u8; 32],
 }
 
+/// Options for uploading blobs/streams to the server
+#[derive(Default, Clone)]
+pub struct UploadOptions {
+    pub previous_manifest: Option<Arc<BackupManifest>>,
+    pub compress: bool,
+    pub encrypt: bool,
+    pub fixed_size: Option<u64>,
+}
+
 type UploadQueueSender = mpsc::Sender<(MergedChunkInfo, Option<h2::client::ResponseFuture>)>;
 type UploadResultReceiver = oneshot::Receiver<Result<(), Error>>;
 
@@ -168,13 +177,12 @@ impl BackupWriter {
         &self,
         data: Vec<u8>,
         file_name: &str,
-        compress: bool,
-        encrypt: bool,
+        options: UploadOptions,
     ) -> Result<BackupStats, Error> {
-        let blob = match (encrypt, &self.crypt_config) {
-             (false, _) => DataBlob::encode(&data, None, compress)?,
+        let blob = match (options.encrypt, &self.crypt_config) {
+             (false, _) => DataBlob::encode(&data, None, options.compress)?,
              (true, None) => bail!("requested encryption without a crypt config"),
-             (true, Some(crypt_config)) => DataBlob::encode(&data, Some(crypt_config), compress)?,
+             (true, Some(crypt_config)) => DataBlob::encode(&data, Some(crypt_config), options.compress)?,
         };
 
         let raw_data = blob.into_inner();
@@ -190,8 +198,7 @@ impl BackupWriter {
         &self,
         src_path: P,
         file_name: &str,
-        compress: bool,
-        encrypt: bool,
+        options: UploadOptions,
     ) -> Result<BackupStats, Error> {
 
         let src_path = src_path.as_ref();
@@ -206,34 +213,33 @@ impl BackupWriter {
             .await
             .map_err(|err| format_err!("unable to read file {:?} - {}", src_path, err))?;
 
-        self.upload_blob_from_data(contents, file_name, compress, encrypt).await
+        self.upload_blob_from_data(contents, file_name, options).await
     }
 
     pub async fn upload_stream(
         &self,
-        previous_manifest: Option<Arc<BackupManifest>>,
         archive_name: &str,
         stream: impl Stream<Item = Result<bytes::BytesMut, Error>>,
-        prefix: &str,
-        fixed_size: Option<u64>,
-        compress: bool,
-        encrypt: bool,
+        options: UploadOptions,
     ) -> Result<BackupStats, Error> {
         let known_chunks = Arc::new(Mutex::new(HashSet::new()));
 
         let mut param = json!({ "archive-name": archive_name });
-        if let Some(size) = fixed_size {
+        let prefix = if let Some(size) = options.fixed_size {
             param["size"] = size.into();
-        }
+            "fixed"
+        } else {
+            "dynamic"
+        };
 
-        if encrypt && self.crypt_config.is_none() {
+        if options.encrypt && self.crypt_config.is_none() {
             bail!("requested encryption without a crypt config");
         }
 
         let index_path = format!("{}_index", prefix);
         let close_path = format!("{}_close", prefix);
 
-        if let Some(manifest) = previous_manifest {
+        if let Some(manifest) = options.previous_manifest {
             // try, but ignore errors
             match archive_type(archive_name) {
                 Ok(ArchiveType::FixedIndex) => {
@@ -255,8 +261,8 @@ impl BackupWriter {
                 stream,
                 &prefix,
                 known_chunks.clone(),
-                if encrypt { self.crypt_config.clone() } else { None },
-                compress,
+                if options.encrypt { self.crypt_config.clone() } else { None },
+                options.compress,
                 self.verbose,
             )
             .await?;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 08/15] pxar: typedef on_error as ErrorHandler
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (6 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 07/15] client: factor out UploadOptions Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 09/15] pxar: factor out PxarCreateOptions Fabian Grünbichler
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/bin/pxar.rs     | 4 ++--
 src/pxar/extract.rs | 6 ++++--
 src/pxar/mod.rs     | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/bin/pxar.rs b/src/bin/pxar.rs
index 4d2308bf..3d92dbe0 100644
--- a/src/bin/pxar.rs
+++ b/src/bin/pxar.rs
@@ -17,7 +17,7 @@ use proxmox::api::cli::*;
 use proxmox::api::api;
 
 use proxmox_backup::tools;
-use proxmox_backup::pxar::{fuse, format_single_line_entry, ENCODER_MAX_ENTRIES, Flags};
+use proxmox_backup::pxar::{fuse, format_single_line_entry, ENCODER_MAX_ENTRIES, ErrorHandler, Flags};
 
 fn extract_archive_from_reader<R: std::io::Read>(
     reader: &mut R,
@@ -27,7 +27,7 @@ fn extract_archive_from_reader<R: std::io::Read>(
     verbose: bool,
     match_list: &[MatchEntry],
     extract_match_default: bool,
-    on_error: Option<Box<dyn FnMut(Error) -> Result<(), Error> + Send>>,
+    on_error: Option<ErrorHandler>,
 ) -> Result<(), Error> {
     proxmox_backup::pxar::extract_archive(
         pxar::decoder::Decoder::from_std(reader)?,
diff --git a/src/pxar/extract.rs b/src/pxar/extract.rs
index 12f9054d..e22fc847 100644
--- a/src/pxar/extract.rs
+++ b/src/pxar/extract.rs
@@ -24,6 +24,8 @@ use crate::pxar::dir_stack::PxarDirStack;
 use crate::pxar::metadata;
 use crate::pxar::Flags;
 
+pub type ErrorHandler = Box<dyn FnMut(Error) -> Result<(), Error> + Send>;
+
 pub fn extract_archive<T, F>(
     mut decoder: pxar::decoder::Decoder<T>,
     destination: &Path,
@@ -32,7 +34,7 @@ pub fn extract_archive<T, F>(
     feature_flags: Flags,
     allow_existing_dirs: bool,
     mut callback: F,
-    on_error: Option<Box<dyn FnMut(Error) -> Result<(), Error> + Send>>,
+    on_error: Option<ErrorHandler>,
 ) -> Result<(), Error>
 where
     T: pxar::decoder::SeqRead,
@@ -212,7 +214,7 @@ pub(crate) struct Extractor {
 
     /// Error callback. Includes `current_path` in the reformatted error, should return `Ok` to
     /// continue extracting or the passed error as `Err` to bail out.
-    on_error: Box<dyn FnMut(Error) -> Result<(), Error> + Send>,
+    on_error: ErrorHandler,
 }
 
 impl Extractor {
diff --git a/src/pxar/mod.rs b/src/pxar/mod.rs
index 6e910667..82998cf8 100644
--- a/src/pxar/mod.rs
+++ b/src/pxar/mod.rs
@@ -59,7 +59,7 @@ mod flags;
 pub use flags::Flags;
 
 pub use create::create_archive;
-pub use extract::extract_archive;
+pub use extract::{extract_archive, ErrorHandler};
 
 /// The format requires to build sorted directory lookup tables in
 /// memory, so we restrict the number of allowed entries to limit
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 09/15] pxar: factor out PxarCreateOptions
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (7 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 08/15] pxar: typedef on_error as ErrorHandler Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 10/15] pxar: extract PxarExtractOptions Fabian Grünbichler
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

containing the CLI parameters that are mostly passed-through from the
client to our pxar archive creation wrapper in pxar::create

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/bin/proxmox-backup-client.rs | 27 ++++++++++++---------------
 src/bin/pxar.rs                  | 22 ++++++++++++++--------
 src/client/pxar_backup_stream.rs | 28 ++++++----------------------
 src/pxar/create.rs               | 28 ++++++++++++++++++++++------
 src/pxar/mod.rs                  |  2 +-
 tests/catar.rs                   | 10 ++++++----
 6 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index af3b8464..c3d6a0ed 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -283,23 +283,15 @@ async fn backup_directory<P: AsRef<Path>>(
     dir_path: P,
     archive_name: &str,
     chunk_size: Option<usize>,
-    device_set: Option<HashSet<u64>>,
-    verbose: bool,
-    skip_lost_and_found: bool,
     catalog: Arc<Mutex<CatalogWriter<crate::tools::StdChannelWriter>>>,
-    exclude_pattern: Vec<MatchEntry>,
-    entries_max: usize,
+    pxar_create_options: proxmox_backup::pxar::PxarCreateOptions,
     upload_options: UploadOptions,
 ) -> Result<BackupStats, Error> {
 
     let pxar_stream = PxarBackupStream::open(
         dir_path.as_ref(),
-        device_set,
-        verbose,
-        skip_lost_and_found,
         catalog,
-        exclude_pattern,
-        entries_max,
+        pxar_create_options,
     )?;
     let mut chunk_stream = ChunkStream::new(pxar_stream, chunk_size);
 
@@ -1039,6 +1031,15 @@ async fn create_backup(
 
                 println!("Upload directory '{}' to '{}' as {}", filename, repo, target);
                 catalog.lock().unwrap().start_directory(std::ffi::CString::new(target.as_str())?.as_c_str())?;
+
+                let pxar_options = proxmox_backup::pxar::PxarCreateOptions {
+                    device_set: devices.clone(),
+                    patterns: pattern_list.clone(),
+                    entries_max: entries_max as usize,
+                    skip_lost_and_found,
+                    verbose,
+                };
+
                 let upload_options = UploadOptions {
                     previous_manifest: previous_manifest.clone(),
                     compress: true,
@@ -1051,12 +1052,8 @@ async fn create_backup(
                     &filename,
                     &target,
                     chunk_size_opt,
-                    devices.clone(),
-                    verbose,
-                    skip_lost_and_found,
                     catalog.clone(),
-                    pattern_list.clone(),
-                    entries_max as usize,
+                    pxar_options,
                     upload_options,
                 ).await?;
                 manifest.add_file(target, stats.size, stats.csum, crypt_mode)?;
diff --git a/src/bin/pxar.rs b/src/bin/pxar.rs
index 3d92dbe0..85606c1d 100644
--- a/src/bin/pxar.rs
+++ b/src/bin/pxar.rs
@@ -311,16 +311,16 @@ fn create_archive(
     exclude: Option<Vec<String>>,
     entries_max: isize,
 ) -> Result<(), Error> {
-    let pattern_list = {
+    let patterns = {
         let input = exclude.unwrap_or_else(Vec::new);
-        let mut pattern_list = Vec::with_capacity(input.len());
+        let mut patterns = Vec::with_capacity(input.len());
         for entry in input {
-            pattern_list.push(
+            patterns.push(
                 MatchEntry::parse_pattern(entry, PatternFlag::PATH_NAME, MatchType::Exclude)
                     .map_err(|err| format_err!("error in exclude pattern: {}", err))?,
             );
         }
-        pattern_list
+        patterns
     };
 
     let device_set = if all_file_systems {
@@ -329,6 +329,15 @@ fn create_archive(
         Some(HashSet::new())
     };
 
+    let options = proxmox_backup::pxar::PxarCreateOptions {
+        entries_max: entries_max as usize,
+        device_set,
+        patterns,
+        verbose,
+        skip_lost_and_found: false,
+    };
+
+
     let source = PathBuf::from(source);
 
     let dir = nix::dir::Dir::open(
@@ -368,18 +377,15 @@ fn create_archive(
     proxmox_backup::pxar::create_archive(
         dir,
         writer,
-        pattern_list,
         feature_flags,
-        device_set,
-        false,
         |path| {
             if verbose {
                 println!("{:?}", path);
             }
             Ok(())
         },
-        entries_max as usize,
         None,
+        options,
     )?;
 
     Ok(())
diff --git a/src/client/pxar_backup_stream.rs b/src/client/pxar_backup_stream.rs
index aa3355fe..5fb28fd5 100644
--- a/src/client/pxar_backup_stream.rs
+++ b/src/client/pxar_backup_stream.rs
@@ -1,4 +1,3 @@
-use std::collections::HashSet;
 use std::io::Write;
 //use std::os::unix::io::FromRawFd;
 use std::path::Path;
@@ -13,8 +12,6 @@ use nix::dir::Dir;
 use nix::fcntl::OFlag;
 use nix::sys::stat::Mode;
 
-use pathpatterns::MatchEntry;
-
 use crate::backup::CatalogWriter;
 
 /// Stream implementation to encode and upload .pxar archives.
@@ -38,12 +35,8 @@ impl Drop for PxarBackupStream {
 impl PxarBackupStream {
     pub fn new<W: Write + Send + 'static>(
         dir: Dir,
-        device_set: Option<HashSet<u64>>,
-        verbose: bool,
-        skip_lost_and_found: bool,
         catalog: Arc<Mutex<CatalogWriter<W>>>,
-        patterns: Vec<MatchEntry>,
-        entries_max: usize,
+        options: crate::pxar::PxarCreateOptions,
     ) -> Result<Self, Error> {
         let (tx, rx) = std::sync::mpsc::sync_channel(10);
 
@@ -61,22 +54,21 @@ impl PxarBackupStream {
                         crate::tools::StdChannelWriter::new(tx),
                     );
 
+                    let verbose = options.verbose;
+
                     let writer = pxar::encoder::sync::StandardWriter::new(writer);
                     if let Err(err) = crate::pxar::create_archive(
                         dir,
                         writer,
-                        patterns,
                         crate::pxar::Flags::DEFAULT,
-                        device_set,
-                        skip_lost_and_found,
                         |path| {
                             if verbose {
                                 println!("{:?}", path);
                             }
                             Ok(())
                         },
-                        entries_max,
                         Some(&mut *catalog_guard),
+                        options,
                     ) {
                         let mut error = error.lock().unwrap();
                         *error = Some(err.to_string());
@@ -93,23 +85,15 @@ impl PxarBackupStream {
 
     pub fn open<W: Write + Send + 'static>(
         dirname: &Path,
-        device_set: Option<HashSet<u64>>,
-        verbose: bool,
-        skip_lost_and_found: bool,
         catalog: Arc<Mutex<CatalogWriter<W>>>,
-        patterns: Vec<MatchEntry>,
-        entries_max: usize,
+        options: crate::pxar::PxarCreateOptions,
     ) -> Result<Self, Error> {
         let dir = nix::dir::Dir::open(dirname, OFlag::O_DIRECTORY, Mode::empty())?;
 
         Self::new(
             dir,
-            device_set,
-            verbose,
-            skip_lost_and_found,
             catalog,
-            patterns,
-            entries_max,
+            options,
         )
     }
 }
diff --git a/src/pxar/create.rs b/src/pxar/create.rs
index a21511c3..229e3f59 100644
--- a/src/pxar/create.rs
+++ b/src/pxar/create.rs
@@ -27,6 +27,22 @@ use crate::pxar::Flags;
 use crate::pxar::tools::assert_single_path_component;
 use crate::tools::{acl, fs, xattr, Fd};
 
+/// Pxar options for creating a pxar archive/stream
+#[derive(Default, Clone)]
+pub struct PxarCreateOptions {
+    /// Device/mountpoint st_dev numbers that should be included. None for no limitation.
+    pub device_set: Option<HashSet<u64>>,
+    /// Exclusion patterns
+    pub patterns: Vec<MatchEntry>,
+    /// Maximum number of entries to hold in memory
+    pub entries_max: usize,
+    /// Skip lost+found directory
+    pub skip_lost_and_found: bool,
+    /// Verbose output
+    pub verbose: bool,
+}
+
+
 fn detect_fs_type(fd: RawFd) -> Result<i64, Error> {
     let mut fs_stat = std::mem::MaybeUninit::uninit();
     let res = unsafe { libc::fstatfs(fd, fs_stat.as_mut_ptr()) };
@@ -136,13 +152,10 @@ type Encoder<'a, 'b> = pxar::encoder::Encoder<'a, &'b mut dyn pxar::encoder::Seq
 pub fn create_archive<T, F>(
     source_dir: Dir,
     mut writer: T,
-    mut patterns: Vec<MatchEntry>,
     feature_flags: Flags,
-    mut device_set: Option<HashSet<u64>>,
-    skip_lost_and_found: bool,
     mut callback: F,
-    entry_limit: usize,
     catalog: Option<&mut dyn BackupCatalogWriter>,
+    options: PxarCreateOptions,
 ) -> Result<(), Error>
 where
     T: pxar::encoder::SeqWrite,
@@ -164,6 +177,7 @@ where
     )
     .map_err(|err| format_err!("failed to get metadata for source directory: {}", err))?;
 
+    let mut device_set = options.device_set.clone();
     if let Some(ref mut set) = device_set {
         set.insert(stat.st_dev);
     }
@@ -171,7 +185,9 @@ where
     let writer = &mut writer as &mut dyn pxar::encoder::SeqWrite;
     let mut encoder = Encoder::new(writer, &metadata)?;
 
-    if skip_lost_and_found {
+    let mut patterns = options.patterns.clone();
+
+    if options.skip_lost_and_found {
         patterns.push(MatchEntry::parse_pattern(
             "lost+found",
             PatternFlag::PATH_NAME,
@@ -188,7 +204,7 @@ where
         catalog,
         path: PathBuf::new(),
         entry_counter: 0,
-        entry_limit,
+        entry_limit: options.entries_max,
         current_st_dev: stat.st_dev,
         device_set,
         hardlinks: HashMap::new(),
diff --git a/src/pxar/mod.rs b/src/pxar/mod.rs
index 82998cf8..c10bb6fb 100644
--- a/src/pxar/mod.rs
+++ b/src/pxar/mod.rs
@@ -58,7 +58,7 @@ pub(crate) mod tools;
 mod flags;
 pub use flags::Flags;
 
-pub use create::create_archive;
+pub use create::{create_archive, PxarCreateOptions};
 pub use extract::{extract_archive, ErrorHandler};
 
 /// The format requires to build sorted directory lookup tables in
diff --git a/tests/catar.rs b/tests/catar.rs
index a6acf9af..2d9dea71 100644
--- a/tests/catar.rs
+++ b/tests/catar.rs
@@ -25,16 +25,18 @@ fn run_test(dir_name: &str) -> Result<(), Error> {
         dir_name, nix::fcntl::OFlag::O_NOFOLLOW,
         nix::sys::stat::Mode::empty())?;
 
+    let options = PxarCreateOptions {
+        entries_max: ENCODER_MAX_ENTRIES,
+        ..PxarCreateOptions::default()
+    };
+
     create_archive(
         dir,
         writer,
-        Vec::new(),
         Flags::DEFAULT,
-        None,
-        false,
         |_| Ok(()),
-        ENCODER_MAX_ENTRIES,
         None,
+        options,
     )?;
 
     Command::new("cmp")
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 10/15] pxar: extract PxarExtractOptions
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (8 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 09/15] pxar: factor out PxarCreateOptions Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 11/15] authid: make Tokenname(Ref) derive Eq Fabian Grünbichler
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

same as PxarCreateOptions, but for extraction/restore rather than
create.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/bin/proxmox-backup-client.rs | 12 ++++++++----
 src/bin/pxar.rs                  | 31 ++++++++++++++-----------------
 src/pxar/extract.rs              | 20 ++++++++++++--------
 src/pxar/mod.rs                  |  2 +-
 4 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index c3d6a0ed..d31e47ae 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -1354,20 +1354,24 @@ async fn restore(param: Value) -> Result<Value, Error> {
 
         let mut reader = BufferedDynamicReader::new(index, chunk_reader);
 
+        let options = proxmox_backup::pxar::PxarExtractOptions {
+            match_list: &[],
+            extract_match_default: true,
+            allow_existing_dirs,
+            on_error: None,
+        };
+
         if let Some(target) = target {
             proxmox_backup::pxar::extract_archive(
                 pxar::decoder::Decoder::from_std(reader)?,
                 Path::new(target),
-                &[],
-                true,
                 proxmox_backup::pxar::Flags::DEFAULT,
-                allow_existing_dirs,
                 |path| {
                     if verbose {
                         println!("{:?}", path);
                     }
                 },
-                None,
+                options,
             )
             .map_err(|err| format_err!("error extracting archive - {}", err))?;
         } else {
diff --git a/src/bin/pxar.rs b/src/bin/pxar.rs
index 85606c1d..b2fe6d52 100644
--- a/src/bin/pxar.rs
+++ b/src/bin/pxar.rs
@@ -17,31 +17,26 @@ use proxmox::api::cli::*;
 use proxmox::api::api;
 
 use proxmox_backup::tools;
-use proxmox_backup::pxar::{fuse, format_single_line_entry, ENCODER_MAX_ENTRIES, ErrorHandler, Flags};
+use proxmox_backup::pxar::{fuse, format_single_line_entry, ENCODER_MAX_ENTRIES, Flags, PxarExtractOptions};
 
 fn extract_archive_from_reader<R: std::io::Read>(
     reader: &mut R,
     target: &str,
     feature_flags: Flags,
-    allow_existing_dirs: bool,
     verbose: bool,
-    match_list: &[MatchEntry],
-    extract_match_default: bool,
-    on_error: Option<ErrorHandler>,
+    options: PxarExtractOptions,
 ) -> Result<(), Error> {
+
     proxmox_backup::pxar::extract_archive(
         pxar::decoder::Decoder::from_std(reader)?,
         Path::new(target),
-        &match_list,
-        extract_match_default,
         feature_flags,
-        allow_existing_dirs,
         |path| {
             if verbose {
                 println!("{:?}", path);
             }
         },
-        on_error,
+        options,
     )
 }
 
@@ -190,6 +185,13 @@ fn extract_archive(
         }) as Box<dyn FnMut(Error) -> Result<(), Error> + Send>)
     };
 
+    let options = PxarExtractOptions {
+        match_list: &match_list,
+        allow_existing_dirs,
+        extract_match_default,
+        on_error,
+    };
+
     if archive == "-" {
         let stdin = std::io::stdin();
         let mut reader = stdin.lock();
@@ -197,11 +199,8 @@ fn extract_archive(
             &mut reader,
             &target,
             feature_flags,
-            allow_existing_dirs,
             verbose,
-            &match_list,
-            extract_match_default,
-            on_error,
+            options,
         )?;
     } else {
         if verbose {
@@ -213,11 +212,8 @@ fn extract_archive(
             &mut reader,
             &target,
             feature_flags,
-            allow_existing_dirs,
             verbose,
-            &match_list,
-            extract_match_default,
-            on_error,
+            options,
         )?;
     }
 
@@ -297,6 +293,7 @@ fn extract_archive(
     },
 )]
 /// Create a new .pxar archive.
+#[allow(clippy::too_many_arguments)]
 fn create_archive(
     archive: String,
     source: String,
diff --git a/src/pxar/extract.rs b/src/pxar/extract.rs
index e22fc847..9cf3f928 100644
--- a/src/pxar/extract.rs
+++ b/src/pxar/extract.rs
@@ -24,17 +24,21 @@ use crate::pxar::dir_stack::PxarDirStack;
 use crate::pxar::metadata;
 use crate::pxar::Flags;
 
+pub struct PxarExtractOptions<'a> {
+    pub match_list: &'a[MatchEntry],
+    pub extract_match_default: bool,
+    pub allow_existing_dirs: bool,
+    pub on_error: Option<ErrorHandler>,
+}
+
 pub type ErrorHandler = Box<dyn FnMut(Error) -> Result<(), Error> + Send>;
 
 pub fn extract_archive<T, F>(
     mut decoder: pxar::decoder::Decoder<T>,
     destination: &Path,
-    match_list: &[MatchEntry],
-    extract_match_default: bool,
     feature_flags: Flags,
-    allow_existing_dirs: bool,
     mut callback: F,
-    on_error: Option<ErrorHandler>,
+    options: PxarExtractOptions,
 ) -> Result<(), Error>
 where
     T: pxar::decoder::SeqRead,
@@ -69,17 +73,17 @@ where
     let mut extractor = Extractor::new(
         dir,
         root.metadata().clone(),
-        allow_existing_dirs,
+        options.allow_existing_dirs,
         feature_flags,
     );
 
-    if let Some(on_error) = on_error {
+    if let Some(on_error) = options.on_error {
         extractor.on_error(on_error);
     }
 
     let mut match_stack = Vec::new();
     let mut err_path_stack = vec![OsString::from("/")];
-    let mut current_match = extract_match_default;
+    let mut current_match = options.extract_match_default;
     while let Some(entry) = decoder.next() {
         use pxar::EntryKind;
 
@@ -99,7 +103,7 @@ where
 
         extractor.set_path(entry.path().as_os_str().to_owned());
 
-        let match_result = match_list.matches(
+        let match_result = options.match_list.matches(
             entry.path().as_os_str().as_bytes(),
             Some(metadata.file_type() as u32),
         );
diff --git a/src/pxar/mod.rs b/src/pxar/mod.rs
index c10bb6fb..5d03591b 100644
--- a/src/pxar/mod.rs
+++ b/src/pxar/mod.rs
@@ -59,7 +59,7 @@ mod flags;
 pub use flags::Flags;
 
 pub use create::{create_archive, PxarCreateOptions};
-pub use extract::{extract_archive, ErrorHandler};
+pub use extract::{extract_archive, ErrorHandler, PxarExtractOptions};
 
 /// The format requires to build sorted directory lookup tables in
 /// memory, so we restrict the number of allowed entries to limit
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 11/15] authid: make Tokenname(Ref) derive Eq
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (9 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 10/15] pxar: extract PxarExtractOptions Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 12/15] derive/impl and use Default for some structs Fabian Grünbichler
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

it's needed to derive Hash, and we always compare Authids or their
Userid components, never just the Tokenname part anyway..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/types/userid.rs | 50 +++++-----------------------------------
 1 file changed, 6 insertions(+), 44 deletions(-)

diff --git a/src/api2/types/userid.rs b/src/api2/types/userid.rs
index 20ce9370..7c73e69f 100644
--- a/src/api2/types/userid.rs
+++ b/src/api2/types/userid.rs
@@ -16,10 +16,10 @@
 //! * [`Authid`]: an owned Authentication ID (a `Userid` with an optional `Tokenname`).
 //! Note that `Userid` and `Authid` do not have a separate borrowed type.
 //!
-//! Note that `Username`s and `Tokenname`s are not unique, therefore they do not implement `Eq` and cannot be
+//! Note that `Username`s are not unique, therefore they do not implement `Eq` and cannot be
 //! compared directly. If a direct comparison is really required, they can be compared as strings
-//! via the `as_str()` method. [`Realm`]s, [`Userid`]s and [`Authid`]s on the other
-//! hand can be compared with each other, as in those cases the comparison has meaning.
+//! via the `as_str()` method. [`Realm`]s, [`Userid`]s and [`Authid`]s on the other hand can be
+//! compared with each other, as in those cases the comparison has meaning.
 
 use std::borrow::Borrow;
 use std::convert::TryFrom;
@@ -299,16 +299,8 @@ impl PartialEq<Realm> for &RealmRef {
 )]
 /// The token ID part of an API token authentication id.
 ///
-/// This alone does NOT uniquely identify the API token and therefore does not implement `Eq`. In
-/// order to compare token IDs directly, they need to be explicitly compared as strings by calling
-/// `.as_str()`.
-///
-/// ```compile_fail
-/// fn test(a: Tokenname, b: Tokenname) -> bool {
-///     a == b // illegal and does not compile
-/// }
-/// ```
-#[derive(Clone, Debug, Hash, Deserialize, Serialize)]
+/// This alone does NOT uniquely identify the API token - use a full `Authid` for such use cases.
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Deserialize, Serialize)]
 pub struct Tokenname(String);
 
 /// A reference to a token name part of an authentication id. This alone does NOT uniquely identify
@@ -336,24 +328,6 @@ pub struct TokennameRef(str);
 /// let b: &UsernameRef = unsafe { std::mem::zeroed() };
 /// let _ = <&UsernameRef as PartialEq>::eq(&a, &b);
 /// ```
-///
-/// ```compile_fail
-/// let a: Tokenname = unsafe { std::mem::zeroed() };
-/// let b: Tokenname = unsafe { std::mem::zeroed() };
-/// let _ = <Tokenname as PartialEq>::eq(&a, &b);
-/// ```
-///
-/// ```compile_fail
-/// let a: &TokennameRef = unsafe { std::mem::zeroed() };
-/// let b: &TokennameRef = unsafe { std::mem::zeroed() };
-/// let _ = <&TokennameRef as PartialEq>::eq(a, b);
-/// ```
-///
-/// ```compile_fail
-/// let a: &TokennameRef = unsafe { std::mem::zeroed() };
-/// let b: &TokennameRef = unsafe { std::mem::zeroed() };
-/// let _ = <&TokennameRef as PartialEq>::eq(&a, &b);
-/// ```
 struct _AssertNoEqImpl;
 
 impl TokennameRef {
@@ -548,7 +522,7 @@ impl PartialEq<String> for Userid {
 }
 
 /// A complete authentication id consisting of a user id and an optional token name.
-#[derive(Clone, Debug, Hash)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct Authid {
     user: Userid,
     tokenname: Option<Tokenname>
@@ -590,18 +564,6 @@ lazy_static! {
     pub static ref ROOT_AUTHID: Authid = Authid::from(Userid::new("root@pam".to_string(), 4));
 }
 
-impl Eq for Authid {}
-
-impl PartialEq for Authid {
-    fn eq(&self, rhs: &Self) -> bool {
-        self.user == rhs.user && match (&self.tokenname, &rhs.tokenname) {
-            (Some(ours), Some(theirs)) => ours.as_str() == theirs.as_str(),
-            (None, None) => true,
-            _ => false,
-        }
-    }
-}
-
 impl From<Userid> for Authid {
     fn from(parts: Userid) -> Self {
         Self::new(parts, None)
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 12/15] derive/impl and use Default for some structs
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (10 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 11/15] authid: make Tokenname(Ref) derive Eq Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 13/15] verify: factor out common parameters Fabian Grünbichler
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

and revamp HttpClientOptions with two constructors for the common use
cases

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
breaks proxmox-backup-qemu, corresponding patch comes later in this
series

 examples/download-speed.rs       |  2 +-
 examples/upload-speed.rs         |  2 +-
 src/api2/config/remote.rs        |  4 +---
 src/backup/prune.rs              |  1 +
 src/bin/proxmox-backup-client.rs | 18 ++++-----------
 src/client.rs                    | 11 +++++----
 src/client/http_client.rs        | 38 +++++++++++++++++++++++++-------
 src/client/pull.rs               |  4 +---
 src/config/acl.rs                |  8 ++++---
 src/config/network.rs            |  2 +-
 10 files changed, 50 insertions(+), 40 deletions(-)

diff --git a/examples/download-speed.rs b/examples/download-speed.rs
index 3ccf4ce7..a4afb7ba 100644
--- a/examples/download-speed.rs
+++ b/examples/download-speed.rs
@@ -28,7 +28,7 @@ async fn run() -> Result<(), Error> {
 
     let auth_id = Authid::root_auth_id();
 
-    let options = HttpClientOptions::new()
+    let options = HttpClientOptions::default()
         .interactive(true)
         .ticket_cache(true);
 
diff --git a/examples/upload-speed.rs b/examples/upload-speed.rs
index 641ed952..05e44aaf 100644
--- a/examples/upload-speed.rs
+++ b/examples/upload-speed.rs
@@ -10,7 +10,7 @@ async fn upload_speed() -> Result<f64, Error> {
 
     let auth_id = Authid::root_auth_id();
 
-    let options = HttpClientOptions::new()
+    let options = HttpClientOptions::default()
         .interactive(true)
         .ticket_cache(true);
 
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index fe7dc451..28221358 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -310,9 +310,7 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
 
 /// Helper to get client for remote.cfg entry
 pub async fn remote_client(remote: remote::Remote) -> Result<HttpClient, Error> {
-    let options = HttpClientOptions::new()
-        .password(Some(remote.password.clone()))
-        .fingerprint(remote.fingerprint.clone());
+    let options = HttpClientOptions::new_non_interactive(remote.password.clone(), remote.fingerprint.clone());
 
     let client = HttpClient::new(
         &remote.host,
diff --git a/src/backup/prune.rs b/src/backup/prune.rs
index baec57d6..dd038055 100644
--- a/src/backup/prune.rs
+++ b/src/backup/prune.rs
@@ -67,6 +67,7 @@ fn remove_incomplete_snapshots(
     }
 }
 
+#[derive(Default)]
 pub struct PruneOptions {
     pub keep_last: Option<u64>,
     pub keep_hourly: Option<u64>,
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index d31e47ae..fe305f63 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -211,13 +211,7 @@ fn connect_do(server: &str, port: u16, auth_id: &Authid) -> Result<HttpClient, E
         Err(NotPresent) => None,
     };
 
-    let options = HttpClientOptions::new()
-        .prefix(Some("proxmox-backup".to_string()))
-        .password(password)
-        .interactive(true)
-        .fingerprint(fingerprint)
-        .fingerprint_cache(true)
-        .ticket_cache(true);
+    let options = HttpClientOptions::new_interactive(password, fingerprint);
 
     HttpClient::new(server, port, auth_id, options)
 }
@@ -1565,13 +1559,9 @@ async fn try_get(repo: &BackupRepository, url: &str) -> Value {
     let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
     let password = std::env::var(ENV_VAR_PBS_PASSWORD).ok();
 
-    let options = HttpClientOptions::new()
-        .prefix(Some("proxmox-backup".to_string()))
-        .password(password)
-        .interactive(false)
-        .fingerprint(fingerprint)
-        .fingerprint_cache(true)
-        .ticket_cache(true);
+    // ticket cache, but no questions asked
+    let options = HttpClientOptions::new_interactive(password, fingerprint)
+        .interactive(false);
 
     let client = match HttpClient::new(repo.host(), repo.port(), repo.auth_id(), options) {
         Ok(v) => v,
diff --git a/src/client.rs b/src/client.rs
index 8c4542b6..d50c26c2 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -49,17 +49,16 @@ pub fn connect_to_localhost() -> Result<HttpClient, Error> {
 
     let uid = nix::unistd::Uid::current();
 
-    let mut options = HttpClientOptions::new()
-        .prefix(Some("proxmox-backup".to_string()))
-        .verify_cert(false); // not required for connection to localhost
-
     let client = if uid.is_root()  {
         let ticket = Ticket::new("PBS", Userid::root_userid())?
             .sign(private_auth_key(), None)?;
-        options = options.password(Some(ticket));
+        let fingerprint = crate::tools::cert::CertInfo::new()?.fingerprint()?;
+        let options = HttpClientOptions::new_non_interactive(ticket, Some(fingerprint));
+
         HttpClient::new("localhost", 8007, Authid::root_auth_id(), options)?
     } else {
-        options = options.ticket_cache(true).interactive(true);
+        let options = HttpClientOptions::new_interactive(None, None);
+
         HttpClient::new("localhost", 8007, Authid::root_auth_id(), options)?
     };
 
diff --git a/src/client/http_client.rs b/src/client/http_client.rs
index f279d9dd..9fd1c013 100644
--- a/src/client/http_client.rs
+++ b/src/client/http_client.rs
@@ -52,15 +52,23 @@ pub struct HttpClientOptions {
 
 impl HttpClientOptions {
 
-    pub fn new() -> Self {
+    pub fn new_interactive(password: Option<String>, fingerprint: Option<String>) -> Self {
         Self {
-            prefix: None,
-            password: None,
-            fingerprint: None,
-            interactive: false,
-            ticket_cache: false,
-            fingerprint_cache: false,
-            verify_cert: true,
+            password,
+            fingerprint,
+            fingerprint_cache: true,
+            ticket_cache: true,
+            interactive: true,
+            prefix: Some("proxmox-backup".to_string()),
+            ..Self::default()
+        }
+    }
+
+    pub fn new_non_interactive(password: String, fingerprint: Option<String>) -> Self {
+        Self {
+            password: Some(password),
+            fingerprint,
+            ..Self::default()
         }
     }
 
@@ -100,6 +108,20 @@ impl HttpClientOptions {
     }
 }
 
+impl Default for HttpClientOptions {
+    fn default() -> Self {
+        Self {
+            prefix: None,
+            password: None,
+            fingerprint: None,
+            interactive: false,
+            ticket_cache: false,
+            fingerprint_cache: false,
+            verify_cert: true,
+        }
+    }
+}
+
 /// HTTP(S) API client
 pub struct HttpClient {
     client: Client<HttpsConnector>,
diff --git a/src/client/pull.rs b/src/client/pull.rs
index 15514374..95720973 100644
--- a/src/client/pull.rs
+++ b/src/client/pull.rs
@@ -502,9 +502,7 @@ pub async fn pull_group(
         // get updated auth_info (new tickets)
         let auth_info = client.login().await?;
 
-        let options = HttpClientOptions::new()
-            .password(Some(auth_info.ticket.clone()))
-            .fingerprint(fingerprint.clone());
+        let options = HttpClientOptions::new_non_interactive(auth_info.ticket.clone(), fingerprint.clone());
 
         let new_client = HttpClient::new(
             src_repo.host(),
diff --git a/src/config/acl.rs b/src/config/acl.rs
index 6ef54e30..e02ac5c7 100644
--- a/src/config/acl.rs
+++ b/src/config/acl.rs
@@ -299,6 +299,7 @@ pub fn check_acl_path(path: &str) -> Result<(), Error> {
 }
 
 /// Tree representing a parsed acl.cfg
+#[derive(Default)]
 pub struct AclTree {
     /// Root node of the tree.
     ///
@@ -308,6 +309,7 @@ pub struct AclTree {
 }
 
 /// Node representing ACLs for a certain ACL path.
+#[derive(Default)]
 pub struct AclTreeNode {
     /// [User](crate::config::user::User) or
     /// [Token](crate::config::user::ApiToken) ACLs for this node.
@@ -412,7 +414,7 @@ impl AclTreeNode {
     }
 
     fn insert_group_role(&mut self, group: String, role: String, propagate: bool) {
-        let map = self.groups.entry(group).or_insert_with(HashMap::new);
+        let map = self.groups.entry(group).or_default();
         if role == ROLE_NAME_NO_ACCESS {
             map.clear();
             map.insert(role, propagate);
@@ -423,7 +425,7 @@ impl AclTreeNode {
     }
 
     fn insert_user_role(&mut self, auth_id: Authid, role: String, propagate: bool) {
-        let map = self.users.entry(auth_id).or_insert_with(HashMap::new);
+        let map = self.users.entry(auth_id).or_default();
         if role == ROLE_NAME_NO_ACCESS {
             map.clear();
             map.insert(role, propagate);
@@ -465,7 +467,7 @@ impl AclTree {
             node = node
                 .children
                 .entry(String::from(*comp))
-                .or_insert_with(AclTreeNode::new);
+                .or_default();
         }
         node
     }
diff --git a/src/config/network.rs b/src/config/network.rs
index 4241261a..99ea0d08 100644
--- a/src/config/network.rs
+++ b/src/config/network.rs
@@ -318,7 +318,7 @@ enum NetworkOrderEntry {
     Option(String),
 }
 
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub struct NetworkConfig {
     pub interfaces: BTreeMap<String, Interface>,
     order: Vec<NetworkOrderEntry>,
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 13/15] verify: factor out common parameters
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (11 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 12/15] derive/impl and use Default for some structs Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 14/15] clippy: allow api functions with many arguments Fabian Grünbichler
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

all the verify methods pass along the following:
- task worker
- datastore
- corrupt and verified chunks

might as well pull that out into a common type, with the added bonus of
now having a single point for construction instead of copying the
default capacaties in three different modules..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/admin/datastore.rs    |  17 +---
 src/api2/backup/environment.rs |  10 +-
 src/backup/verify.rs           | 174 ++++++++++++++-------------------
 src/server/verify_job.rs       |   3 +-
 4 files changed, 85 insertions(+), 119 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index ba8f3417..3d5b6af6 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -3,7 +3,6 @@
 use std::collections::HashSet;
 use std::ffi::OsStr;
 use std::os::unix::ffi::OsStrExt;
-use std::sync::{Arc, Mutex};
 use std::path::{Path, PathBuf};
 use std::pin::Pin;
 
@@ -672,17 +671,12 @@ pub fn verify(
         auth_id.clone(),
         to_stdout,
         move |worker| {
-            let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024*16)));
-            let corrupt_chunks = Arc::new(Mutex::new(HashSet::with_capacity(64)));
-
+            let verify_worker = crate::backup::VerifyWorker::new(worker.clone(), datastore);
             let failed_dirs = if let Some(backup_dir) = backup_dir {
                 let mut res = Vec::new();
                 if !verify_backup_dir(
-                    datastore,
+                    &verify_worker,
                     &backup_dir,
-                    verified_chunks,
-                    corrupt_chunks,
-                    worker.clone(),
                     worker.upid().clone(),
                     None,
                 )? {
@@ -691,12 +685,9 @@ pub fn verify(
                 res
             } else if let Some(backup_group) = backup_group {
                 let failed_dirs = verify_backup_group(
-                    datastore,
+                    &verify_worker,
                     &backup_group,
-                    verified_chunks,
-                    corrupt_chunks,
                     &mut StoreProgress::new(1),
-                    worker.clone(),
                     worker.upid(),
                     None,
                 )?;
@@ -711,7 +702,7 @@ pub fn verify(
                     None
                 };
 
-                verify_all_backups(datastore, worker.clone(), worker.upid(), owner, None)?
+                verify_all_backups(&verify_worker, worker.upid(), owner, None)?
             };
             if !failed_dirs.is_empty() {
                 worker.log("Failed to verify the following snapshots/groups:");
diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 38061816..c8f52b6e 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -1,6 +1,6 @@
 use anyhow::{bail, format_err, Error};
 use std::sync::{Arc, Mutex};
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use nix::dir::Dir;
 
 use ::serde::{Serialize};
@@ -525,15 +525,11 @@ impl BackupEnvironment {
             move |worker| {
                 worker.log("Automatically verifying newly added snapshot");
 
-                let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024*16)));
-                let corrupt_chunks = Arc::new(Mutex::new(HashSet::with_capacity(64)));
 
+                let verify_worker = crate::backup::VerifyWorker::new(worker.clone(), datastore);
                 if !verify_backup_dir_with_lock(
-                    datastore,
+                    &verify_worker,
                     &backup_dir,
-                    verified_chunks,
-                    corrupt_chunks,
-                    worker.clone(),
                     worker.upid().clone(),
                     None,
                     snap_lock,
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 5e4bc7fb..ac4a6c29 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -29,6 +29,29 @@ use crate::{
     tools::fs::lock_dir_noblock_shared,
 };
 
+/// A VerifyWorker encapsulates a task worker, datastore and information about which chunks have
+/// already been verified or detected as corrupt.
+pub struct VerifyWorker {
+    worker: Arc<dyn TaskState + Send + Sync>,
+    datastore: Arc<DataStore>,
+    verified_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
+    corrupt_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
+}
+
+impl VerifyWorker {
+    /// Creates a new VerifyWorker for a given task worker and datastore.
+    pub fn new(worker: Arc<dyn TaskState + Send + Sync>, datastore: Arc<DataStore>) -> Self {
+        Self {
+            worker,
+            datastore,
+            // start with 16k chunks == up to 64G data
+            verified_chunks: Arc::new(Mutex::new(HashSet::with_capacity(16*1024))),
+            // start with 64 chunks since we assume there are few corrupt ones
+            corrupt_chunks: Arc::new(Mutex::new(HashSet::with_capacity(64))),
+        }
+    }
+}
+
 fn verify_blob(datastore: Arc<DataStore>, backup_dir: &BackupDir, info: &FileInfo) -> Result<(), Error> {
 
     let blob = datastore.load_blob(backup_dir, &info.filename)?;
@@ -82,12 +105,9 @@ fn rename_corrupted_chunk(
 }
 
 fn verify_index_chunks(
-    datastore: Arc<DataStore>,
+    verify_worker: &VerifyWorker,
     index: Box<dyn IndexFile + Send>,
-    verified_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    corrupt_chunks: Arc<Mutex<HashSet<[u8; 32]>>>,
     crypt_mode: CryptMode,
-    worker: Arc<dyn TaskState + Send + Sync>,
 ) -> Result<(), Error> {
 
     let errors = Arc::new(AtomicUsize::new(0));
@@ -97,10 +117,10 @@ fn verify_index_chunks(
     let mut read_bytes = 0;
     let mut decoded_bytes = 0;
 
-    let worker2 = Arc::clone(&worker);
-    let datastore2 = Arc::clone(&datastore);
-    let corrupt_chunks2 = Arc::clone(&corrupt_chunks);
-    let verified_chunks2 = Arc::clone(&verified_chunks);
+    let worker2 = Arc::clone(&verify_worker.worker);
+    let datastore2 = Arc::clone(&verify_worker.datastore);
+    let corrupt_chunks2 = Arc::clone(&verify_worker.corrupt_chunks);
+    let verified_chunks2 = Arc::clone(&verify_worker.verified_chunks);
     let errors2 = Arc::clone(&errors);
 
     let decoder_pool = ParallelHandler::new(
@@ -141,29 +161,29 @@ fn verify_index_chunks(
 
     for pos in 0..index.index_count() {
 
-        worker.check_abort()?;
+        verify_worker.worker.check_abort()?;
         crate::tools::fail_on_shutdown()?;
 
         let info = index.chunk_info(pos).unwrap();
         let size = info.size();
 
-        if verified_chunks.lock().unwrap().contains(&info.digest) {
+        if verify_worker.verified_chunks.lock().unwrap().contains(&info.digest) {
             continue; // already verified
         }
 
-        if corrupt_chunks.lock().unwrap().contains(&info.digest) {
+        if verify_worker.corrupt_chunks.lock().unwrap().contains(&info.digest) {
             let digest_str = proxmox::tools::digest_to_hex(&info.digest);
-            task_log!(worker, "chunk {} was marked as corrupt", digest_str);
+            task_log!(verify_worker.worker, "chunk {} was marked as corrupt", digest_str);
             errors.fetch_add(1, Ordering::SeqCst);
             continue;
         }
 
-        match datastore.load_chunk(&info.digest) {
+        match verify_worker.datastore.load_chunk(&info.digest) {
             Err(err) => {
-                corrupt_chunks.lock().unwrap().insert(info.digest);
-                task_log!(worker, "can't verify chunk, load failed - {}", err);
+                verify_worker.corrupt_chunks.lock().unwrap().insert(info.digest);
+                task_log!(verify_worker.worker, "can't verify chunk, load failed - {}", err);
                 errors.fetch_add(1, Ordering::SeqCst);
-                rename_corrupted_chunk(datastore.clone(), &info.digest, &worker);
+                rename_corrupted_chunk(verify_worker.datastore.clone(), &info.digest, &verify_worker.worker);
                 continue;
             }
             Ok(chunk) => {
@@ -187,7 +207,7 @@ fn verify_index_chunks(
     let error_count = errors.load(Ordering::SeqCst);
 
     task_log!(
-        worker,
+        verify_worker.worker,
         "  verified {:.2}/{:.2} MiB in {:.2} seconds, speed {:.2}/{:.2} MiB/s ({} errors)",
         read_bytes_mib,
         decoded_bytes_mib,
@@ -205,18 +225,15 @@ fn verify_index_chunks(
 }
 
 fn verify_fixed_index(
-    datastore: Arc<DataStore>,
+    verify_worker: &VerifyWorker,
     backup_dir: &BackupDir,
     info: &FileInfo,
-    verified_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    corrupt_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    worker: Arc<dyn TaskState + Send + Sync>,
 ) -> Result<(), Error> {
 
     let mut path = backup_dir.relative_path();
     path.push(&info.filename);
 
-    let index = datastore.open_fixed_reader(&path)?;
+    let index = verify_worker.datastore.open_fixed_reader(&path)?;
 
     let (csum, size) = index.compute_csum();
     if size != info.size {
@@ -228,28 +245,22 @@ fn verify_fixed_index(
     }
 
     verify_index_chunks(
-        datastore,
+        verify_worker,
         Box::new(index),
-        verified_chunks,
-        corrupt_chunks,
         info.chunk_crypt_mode(),
-        worker,
     )
 }
 
 fn verify_dynamic_index(
-    datastore: Arc<DataStore>,
+    verify_worker: &VerifyWorker,
     backup_dir: &BackupDir,
     info: &FileInfo,
-    verified_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    corrupt_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    worker: Arc<dyn TaskState + Send + Sync>,
 ) -> Result<(), Error> {
 
     let mut path = backup_dir.relative_path();
     path.push(&info.filename);
 
-    let index = datastore.open_dynamic_reader(&path)?;
+    let index = verify_worker.datastore.open_dynamic_reader(&path)?;
 
     let (csum, size) = index.compute_csum();
     if size != info.size {
@@ -261,12 +272,9 @@ fn verify_dynamic_index(
     }
 
     verify_index_chunks(
-        datastore,
+        verify_worker,
         Box::new(index),
-        verified_chunks,
-        corrupt_chunks,
         info.chunk_crypt_mode(),
-        worker,
     )
 }
 
@@ -280,34 +288,28 @@ fn verify_dynamic_index(
 /// - Ok(false) if there were verification errors
 /// - Err(_) if task was aborted
 pub fn verify_backup_dir(
-    datastore: Arc<DataStore>,
+    verify_worker: &VerifyWorker,
     backup_dir: &BackupDir,
-    verified_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    corrupt_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    worker: Arc<dyn TaskState + Send + Sync>,
     upid: UPID,
     filter: Option<&dyn Fn(&BackupManifest) -> bool>,
 ) -> Result<bool, Error> {
     let snap_lock = lock_dir_noblock_shared(
-        &datastore.snapshot_path(&backup_dir),
+        &verify_worker.datastore.snapshot_path(&backup_dir),
         "snapshot",
         "locked by another operation");
     match snap_lock {
         Ok(snap_lock) => verify_backup_dir_with_lock(
-            datastore,
+            verify_worker,
             backup_dir,
-            verified_chunks,
-            corrupt_chunks,
-            worker,
             upid,
             filter,
             snap_lock
         ),
         Err(err) => {
             task_log!(
-                worker,
+                verify_worker.worker,
                 "SKIPPED: verify {}:{} - could not acquire snapshot lock: {}",
-                datastore.name(),
+                verify_worker.datastore.name(),
                 backup_dir,
                 err,
             );
@@ -318,22 +320,19 @@ pub fn verify_backup_dir(
 
 /// See verify_backup_dir
 pub fn verify_backup_dir_with_lock(
-    datastore: Arc<DataStore>,
+    verify_worker: &VerifyWorker,
     backup_dir: &BackupDir,
-    verified_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    corrupt_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    worker: Arc<dyn TaskState + Send + Sync>,
     upid: UPID,
     filter: Option<&dyn Fn(&BackupManifest) -> bool>,
     _snap_lock: Dir,
 ) -> Result<bool, Error> {
-    let manifest = match datastore.load_manifest(&backup_dir) {
+    let manifest = match verify_worker.datastore.load_manifest(&backup_dir) {
         Ok((manifest, _)) => manifest,
         Err(err) => {
             task_log!(
-                worker,
+                verify_worker.worker,
                 "verify {}:{} - manifest load error: {}",
-                datastore.name(),
+                verify_worker.datastore.name(),
                 backup_dir,
                 err,
             );
@@ -344,54 +343,48 @@ pub fn verify_backup_dir_with_lock(
     if let Some(filter) = filter {
         if !filter(&manifest) {
             task_log!(
-                worker,
+                verify_worker.worker,
                 "SKIPPED: verify {}:{} (recently verified)",
-                datastore.name(),
+                verify_worker.datastore.name(),
                 backup_dir,
             );
             return Ok(true);
         }
     }
 
-    task_log!(worker, "verify {}:{}", datastore.name(), backup_dir);
+    task_log!(verify_worker.worker, "verify {}:{}", verify_worker.datastore.name(), backup_dir);
 
     let mut error_count = 0;
 
     let mut verify_result = VerifyState::Ok;
     for info in manifest.files() {
         let result = proxmox::try_block!({
-            task_log!(worker, "  check {}", info.filename);
+            task_log!(verify_worker.worker, "  check {}", info.filename);
             match archive_type(&info.filename)? {
                 ArchiveType::FixedIndex =>
                     verify_fixed_index(
-                        datastore.clone(),
+                        verify_worker,
                         &backup_dir,
                         info,
-                        verified_chunks.clone(),
-                        corrupt_chunks.clone(),
-                        worker.clone(),
                     ),
                 ArchiveType::DynamicIndex =>
                     verify_dynamic_index(
-                        datastore.clone(),
+                        verify_worker,
                         &backup_dir,
                         info,
-                        verified_chunks.clone(),
-                        corrupt_chunks.clone(),
-                        worker.clone(),
                     ),
-                ArchiveType::Blob => verify_blob(datastore.clone(), &backup_dir, info),
+                ArchiveType::Blob => verify_blob(verify_worker.datastore.clone(), &backup_dir, info),
             }
         });
 
-        worker.check_abort()?;
+        verify_worker.worker.check_abort()?;
         crate::tools::fail_on_shutdown()?;
 
         if let Err(err) = result {
             task_log!(
-                worker,
+                verify_worker.worker,
                 "verify {}:{}/{} failed: {}",
-                datastore.name(),
+                verify_worker.datastore.name(),
                 backup_dir,
                 info.filename,
                 err,
@@ -407,7 +400,7 @@ pub fn verify_backup_dir_with_lock(
         upid,
     };
     let verify_state = serde_json::to_value(verify_state)?;
-    datastore.update_manifest(&backup_dir, |manifest| {
+    verify_worker.datastore.update_manifest(&backup_dir, |manifest| {
         manifest.unprotected["verify_state"] = verify_state;
     }).map_err(|err| format_err!("unable to update manifest blob - {}", err))?;
 
@@ -422,24 +415,21 @@ pub fn verify_backup_dir_with_lock(
 /// - Ok((count, failed_dirs)) where failed_dirs had verification errors
 /// - Err(_) if task was aborted
 pub fn verify_backup_group(
-    datastore: Arc<DataStore>,
+    verify_worker: &VerifyWorker,
     group: &BackupGroup,
-    verified_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
-    corrupt_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
     progress: &mut StoreProgress,
-    worker: Arc<dyn TaskState + Send + Sync>,
     upid: &UPID,
     filter: Option<&dyn Fn(&BackupManifest) -> bool>,
 ) -> Result<Vec<String>, Error> {
 
     let mut errors = Vec::new();
-    let mut list = match group.list_backups(&datastore.base_path()) {
+    let mut list = match group.list_backups(&verify_worker.datastore.base_path()) {
         Ok(list) => list,
         Err(err) => {
             task_log!(
-                worker,
+                verify_worker.worker,
                 "verify group {}:{} - unable to list backups: {}",
-                datastore.name(),
+                verify_worker.datastore.name(),
                 group,
                 err,
             );
@@ -448,18 +438,15 @@ pub fn verify_backup_group(
     };
 
     let snapshot_count = list.len();
-    task_log!(worker, "verify group {}:{} ({} snapshots)", datastore.name(), group, snapshot_count);
+    task_log!(verify_worker.worker, "verify group {}:{} ({} snapshots)", verify_worker.datastore.name(), group, snapshot_count);
 
     progress.group_snapshots = snapshot_count as u64;
 
     BackupInfo::sort_list(&mut list, false); // newest first
     for (pos, info) in list.into_iter().enumerate() {
         if !verify_backup_dir(
-            datastore.clone(),
+            verify_worker,
             &info.backup_dir,
-            verified_chunks.clone(),
-            corrupt_chunks.clone(),
-            worker.clone(),
             upid.clone(),
             filter,
         )? {
@@ -467,7 +454,7 @@ pub fn verify_backup_group(
         }
         progress.done_snapshots = pos as u64 + 1;
         task_log!(
-            worker,
+            verify_worker.worker,
             "percentage done: {}",
             progress
         );
@@ -484,22 +471,22 @@ pub fn verify_backup_group(
 /// - Ok(failed_dirs) where failed_dirs had verification errors
 /// - Err(_) if task was aborted
 pub fn verify_all_backups(
-    datastore: Arc<DataStore>,
-    worker: Arc<dyn TaskState + Send + Sync>,
+    verify_worker: &VerifyWorker,
     upid: &UPID,
     owner: Option<Authid>,
     filter: Option<&dyn Fn(&BackupManifest) -> bool>,
 ) -> Result<Vec<String>, Error> {
     let mut errors = Vec::new();
+    let worker = Arc::clone(&verify_worker.worker);
 
-    task_log!(worker, "verify datastore {}", datastore.name());
+    task_log!(worker, "verify datastore {}", verify_worker.datastore.name());
 
     if let Some(owner) = &owner {
         task_log!(worker, "limiting to backups owned by {}", owner);
     }
 
     let filter_by_owner = |group: &BackupGroup| {
-        match (datastore.get_owner(group), &owner) {
+        match (verify_worker.datastore.get_owner(group), &owner) {
             (Ok(ref group_owner), Some(owner)) => {
                 group_owner == owner
                     || (group_owner.is_token()
@@ -527,7 +514,7 @@ pub fn verify_all_backups(
         }
     };
 
-    let mut list = match BackupInfo::list_backup_groups(&datastore.base_path()) {
+    let mut list = match BackupInfo::list_backup_groups(&verify_worker.datastore.base_path()) {
         Ok(list) => list
             .into_iter()
             .filter(|group| !(group.backup_type() == "host" && group.backup_id() == "benchmark"))
@@ -545,12 +532,6 @@ pub fn verify_all_backups(
 
     list.sort_unstable();
 
-    // start with 16384 chunks (up to 65GB)
-    let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024*16)));
-
-    // start with 64 chunks since we assume there are few corrupt ones
-    let corrupt_chunks = Arc::new(Mutex::new(HashSet::with_capacity(64)));
-
     let group_count = list.len();
     task_log!(worker, "found {} groups", group_count);
 
@@ -562,12 +543,9 @@ pub fn verify_all_backups(
         progress.group_snapshots = 0;
 
         let mut group_errors = verify_backup_group(
-            datastore.clone(),
+            verify_worker,
             &group,
-            verified_chunks.clone(),
-            corrupt_chunks.clone(),
             &mut progress,
-            worker.clone(),
             upid,
             filter,
         )?;
diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
index ca6eb554..1dd8baa7 100644
--- a/src/server/verify_job.rs
+++ b/src/server/verify_job.rs
@@ -67,7 +67,8 @@ pub fn do_verification_job(
                 task_log!(worker,"task triggered by schedule '{}'", event_str);
             }
 
-            let result = verify_all_backups(datastore, worker.clone(), worker.upid(), None, Some(&filter));
+            let verify_worker = crate::backup::VerifyWorker::new(worker.clone(), datastore);
+            let result = verify_all_backups(&verify_worker, worker.upid(), None, Some(&filter));
             let job_result = match result {
                 Ok(ref failed_dirs) if failed_dirs.is_empty() => Ok(()),
                 Ok(ref failed_dirs) => {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 14/15] clippy: allow api functions with many arguments
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (12 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 13/15] verify: factor out common parameters Fabian Grünbichler
@ 2021-01-25 13:42 ` Fabian Grünbichler
  2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup 15/15] clippy: more misc fixes Fabian Grünbichler
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:42 UTC (permalink / raw)
  To: pbs-devel

some of those can be reduced/cleaned up when we have updater support in
the api macro.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/access/acl.rs       | 1 +
 src/api2/access/tfa.rs       | 1 +
 src/api2/access/user.rs      | 1 +
 src/api2/admin/datastore.rs  | 2 --
 src/api2/config/datastore.rs | 1 +
 src/api2/config/remote.rs    | 1 +
 src/api2/config/sync.rs      | 1 +
 src/api2/config/verify.rs    | 1 +
 src/api2/node/network.rs     | 2 ++
 src/api2/node/tasks.rs       | 1 +
 src/bin/pxar.rs              | 1 +
 src/client/backup_writer.rs  | 2 ++
 12 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/api2/access/acl.rs b/src/api2/access/acl.rs
index 6f1ba327..be9ad380 100644
--- a/src/api2/access/acl.rs
+++ b/src/api2/access/acl.rs
@@ -165,6 +165,7 @@ pub fn read_acl(
     },
 )]
 /// Update Access Control List (ACLs).
+#[allow(clippy::too_many_arguments)]
 pub fn update_acl(
     path: String,
     role: String,
diff --git a/src/api2/access/tfa.rs b/src/api2/access/tfa.rs
index 5df0baec..ba583323 100644
--- a/src/api2/access/tfa.rs
+++ b/src/api2/access/tfa.rs
@@ -421,6 +421,7 @@ impl TfaUpdateInfo {
     },
 )]
 /// Add a TFA entry to the user.
+#[allow(clippy::too_many_arguments)]
 fn add_tfa_entry(
     userid: Userid,
     description: Option<String>,
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index c6032818..c49b12b1 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -353,6 +353,7 @@ pub enum DeletableProperty {
     },
 )]
 /// Update user configuration.
+#[allow(clippy::too_many_arguments)]
 pub fn update_user(
     userid: Userid,
     comment: Option<String>,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 3d5b6af6..6f02e460 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1293,8 +1293,6 @@ pub fn catalog(
     backup_id: String,
     backup_time: i64,
     filepath: String,
-    _param: Value,
-    _info: &ApiMethod,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
     let datastore = DataStore::lookup_datastore(&store)?;
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 00009b74..3a3dc176 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -277,6 +277,7 @@ pub enum DeletableProperty {
     },
 )]
 /// Update datastore config.
+#[allow(clippy::too_many_arguments)]
 pub fn update_datastore(
     name: String,
     comment: Option<String>,
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index 28221358..446a2604 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -203,6 +203,7 @@ pub enum DeletableProperty {
     },
 )]
 /// Update remote configuration.
+#[allow(clippy::too_many_arguments)]
 pub fn update_remote(
     name: String,
     comment: Option<String>,
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index e7360051..00a8c0b3 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -279,6 +279,7 @@ pub enum DeletableProperty {
     },
 )]
 /// Update sync job config.
+#[allow(clippy::too_many_arguments)]
 pub fn update_sync_job(
     id: String,
     store: Option<String>,
diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
index 08a9e717..db5f4d83 100644
--- a/src/api2/config/verify.rs
+++ b/src/api2/config/verify.rs
@@ -215,6 +215,7 @@ pub enum DeletableProperty {
     },
 )]
 /// Update verification job config.
+#[allow(clippy::too_many_arguments)]
 pub fn update_verification_job(
     id: String,
     store: Option<String>,
diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs
index d9c031ae..0dc321b8 100644
--- a/src/api2/node/network.rs
+++ b/src/api2/node/network.rs
@@ -213,6 +213,7 @@ pub fn read_interface(iface: String) -> Result<Value, Error> {
     },
 )]
 /// Create network interface configuration.
+#[allow(clippy::too_many_arguments)]
 pub fn create_interface(
     iface: String,
     autostart: Option<bool>,
@@ -477,6 +478,7 @@ pub enum DeletableProperty {
     },
 )]
 /// Update network interface config.
+#[allow(clippy::too_many_arguments)]
 pub fn update_interface(
     iface: String,
     autostart: Option<bool>,
diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
index 99470531..ff6ed726 100644
--- a/src/api2/node/tasks.rs
+++ b/src/api2/node/tasks.rs
@@ -425,6 +425,7 @@ fn stop_task(
     },
 )]
 /// List tasks.
+#[allow(clippy::too_many_arguments)]
 pub fn list_tasks(
     start: u64,
     limit: u64,
diff --git a/src/bin/pxar.rs b/src/bin/pxar.rs
index b2fe6d52..814b3346 100644
--- a/src/bin/pxar.rs
+++ b/src/bin/pxar.rs
@@ -112,6 +112,7 @@ fn extract_archive_from_reader<R: std::io::Read>(
     },
 )]
 /// Extract an archive.
+#[allow(clippy::too_many_arguments)]
 fn extract_archive(
     archive: String,
     pattern: Option<Vec<String>>,
diff --git a/src/client/backup_writer.rs b/src/client/backup_writer.rs
index 38953a45..01ea7704 100644
--- a/src/client/backup_writer.rs
+++ b/src/client/backup_writer.rs
@@ -57,6 +57,8 @@ impl BackupWriter {
         Arc::new(Self { h2, abort, crypt_config, verbose })
     }
 
+    // FIXME: extract into (flattened) parameter struct?
+    #[allow(clippy::too_many_arguments)]
     pub async fn start(
         client: HttpClient,
         crypt_config: Option<Arc<CryptConfig>>,
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 15/15] clippy: more misc fixes
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (13 preceding siblings ...)
  2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 14/15] clippy: allow api functions with many arguments Fabian Grünbichler
@ 2021-01-25 13:43 ` Fabian Grünbichler
  2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup-qemu 1/2] use UploadOptions for uploading Blobs Fabian Grünbichler
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:43 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/bin/proxmox-daily-update.rs        |  9 +++------
 src/bin/proxmox_backup_client/key.rs   | 24 +++++-------------------
 src/bin/proxmox_backup_client/mount.rs |  2 +-
 src/bin/proxmox_backup_manager/user.rs |  4 ++--
 src/rrd/mod.rs                         |  1 +
 5 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs
index 99f5152e..83c6b80c 100644
--- a/src/bin/proxmox-daily-update.rs
+++ b/src/bin/proxmox-daily-update.rs
@@ -63,11 +63,8 @@ fn main() {
     let mut rpcenv = CliEnvironment::new();
     rpcenv.set_auth_id(Some(String::from("root@pam")));
 
-    match proxmox_backup::tools::runtime::main(do_update(&mut rpcenv)) {
-        Err(err) => {
-            eprintln!("error during update: {}", err);
-            std::process::exit(1);
-        },
-        _ => (),
+    if let Err(err) = proxmox_backup::tools::runtime::main(do_update(&mut rpcenv)) {
+        eprintln!("error during update: {}", err);
+        std::process::exit(1);
     }
 }
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index ef3123bb..405cb818 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -315,22 +315,13 @@ fn change_passphrase(
     },
 )]
 /// Print the encryption key's metadata.
-fn show_key(
-    path: Option<String>,
-    param: Value,
-) -> Result<(), Error> {
+fn show_key(path: Option<String>, param: Value) -> Result<(), Error> {
     let path = match path {
         Some(path) => PathBuf::from(path),
-        None => {
-            let path = find_default_encryption_key()?
-                .ok_or_else(|| {
-                    format_err!("no encryption file provided and no default file found")
-                })?;
-            path
-        }
+        None => find_default_encryption_key()?
+            .ok_or_else(|| format_err!("no encryption file provided and no default file found"))?,
     };
 
-
     let config: KeyConfig = serde_json::from_slice(&file_get_contents(path.clone())?)?;
 
     let output_format = get_output_format(&param);
@@ -442,13 +433,8 @@ fn paper_key(
 ) -> Result<(), Error> {
     let path = match path {
         Some(path) => PathBuf::from(path),
-        None => {
-            let path = find_default_encryption_key()?
-                .ok_or_else(|| {
-                    format_err!("no encryption file provided and no default file found")
-                })?;
-            path
-        }
+        None => find_default_encryption_key()?
+            .ok_or_else(|| format_err!("no encryption file provided and no default file found"))?,
     };
 
     let data = file_get_contents(&path)?;
diff --git a/src/bin/proxmox_backup_client/mount.rs b/src/bin/proxmox_backup_client/mount.rs
index 72ed9166..24100752 100644
--- a/src/bin/proxmox_backup_client/mount.rs
+++ b/src/bin/proxmox_backup_client/mount.rs
@@ -345,7 +345,7 @@ fn unmap(
             let mut any = false;
             for (backing, loopdev) in tools::fuse_loop::find_all_mappings()? {
                 let name = tools::systemd::unescape_unit(&backing)?;
-                println!("{}:\t{}", loopdev.unwrap_or("(unmapped)".to_owned()), name);
+                println!("{}:\t{}", loopdev.unwrap_or_else(|| "(unmapped)".to_string()), name);
                 any = true;
             }
             if !any {
diff --git a/src/bin/proxmox_backup_manager/user.rs b/src/bin/proxmox_backup_manager/user.rs
index d68e9e81..6603db1b 100644
--- a/src/bin/proxmox_backup_manager/user.rs
+++ b/src/bin/proxmox_backup_manager/user.rs
@@ -133,7 +133,7 @@ fn list_permissions(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Val
     let output_format = get_output_format(&param);
 
     let info = &api2::access::API_METHOD_LIST_PERMISSIONS;
-    let mut data = match info.handler {
+    let data = match info.handler {
         ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
         _ => unreachable!(),
     };
@@ -161,7 +161,7 @@ fn list_permissions(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Val
             }
         }
     } else {
-        format_and_print_result(&mut data, &output_format);
+        format_and_print_result(&data, &output_format);
     }
 
     Ok(Value::Null)
diff --git a/src/rrd/mod.rs b/src/rrd/mod.rs
index c09efebf..03e4c9de 100644
--- a/src/rrd/mod.rs
+++ b/src/rrd/mod.rs
@@ -1,3 +1,4 @@
+#[allow(clippy::module_inception)]
 mod rrd;
 pub use rrd::*;
 mod cache;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup-qemu 1/2] use UploadOptions for uploading Blobs
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (14 preceding siblings ...)
  2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup 15/15] clippy: more misc fixes Fabian Grünbichler
@ 2021-01-25 13:43 ` Fabian Grünbichler
  2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup-qemu 2/2] use new HttpClientOptions constructors Fabian Grünbichler
  2021-01-26  9:44 ` [pbs-devel] applied series: [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Wolfgang Bumiller
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:43 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    needs corresponding proxmox-backup commit

 src/commands.rs | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/commands.rs b/src/commands.rs
index 2f6eb0f..c63c4f7 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -93,7 +93,13 @@ pub(crate) async fn add_config(
 
     let blob_name = format!("{}.blob", name);
 
-    let stats = client.upload_blob_from_data(data, &blob_name, compress, crypt_mode == CryptMode::Encrypt).await?;
+    let options = UploadOptions {
+        compress,
+        encrypt: crypt_mode == CryptMode::Encrypt,
+        ..UploadOptions::default()
+    };
+
+    let stats = client.upload_blob_from_data(data, &blob_name, options).await?;
 
     let mut guard = manifest.lock().unwrap();
     guard.add_file(blob_name, stats.size, stats.csum, crypt_mode)?;
@@ -457,8 +463,13 @@ pub(crate) async fn finish_backup(
         *key_fingerprint_guard = key_fingerprint;
     }
 
+    let options = UploadOptions {
+        compress: true,
+        ..UploadOptions::default()
+    };
+
     client
-        .upload_blob_from_data(manifest.into_bytes(), MANIFEST_BLOB_NAME, true, false)
+        .upload_blob_from_data(manifest.into_bytes(), MANIFEST_BLOB_NAME, options)
         .await?;
 
     client.finish().await?;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] [PATCH proxmox-backup-qemu 2/2] use new HttpClientOptions constructors
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (15 preceding siblings ...)
  2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup-qemu 1/2] use UploadOptions for uploading Blobs Fabian Grünbichler
@ 2021-01-25 13:43 ` Fabian Grünbichler
  2021-01-26  9:44 ` [pbs-devel] applied series: [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Wolfgang Bumiller
  17 siblings, 0 replies; 19+ messages in thread
From: Fabian Grünbichler @ 2021-01-25 13:43 UTC (permalink / raw)
  To: pbs-devel

and make password non-optional from the start.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    needs corresponding proxmox_backup commit

 src/backup.rs  | 7 ++++---
 src/lib.rs     | 8 +++++---
 src/restore.rs | 7 ++++---
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/backup.rs b/src/backup.rs
index b5e1263..e2062e7 100644
--- a/src/backup.rs
+++ b/src/backup.rs
@@ -107,9 +107,10 @@ impl BackupTask {
         self.check_aborted()?;
 
         let command_future = async  {
-            let options = HttpClientOptions::new()
-                .fingerprint(self.setup.fingerprint.clone())
-                .password(self.setup.password.clone());
+            let options = HttpClientOptions::new_non_interactive(
+                self.setup.password.clone(),
+                self.setup.fingerprint.clone(),
+            );
 
             let http = HttpClient::new(&self.setup.host, self.setup.port, &self.setup.auth_id, options)?;
             let writer = BackupWriter::start(
diff --git a/src/lib.rs b/src/lib.rs
index b755014..fd7c064 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -134,7 +134,7 @@ pub(crate) struct BackupSetup {
     pub backup_type: String,
     pub backup_id: String,
     pub backup_time: i64,
-    pub password: Option<String>,
+    pub password: String,
     pub keyfile: Option<std::path::PathBuf>,
     pub key_password: Option<String>,
     pub fingerprint: Option<String>,
@@ -222,7 +222,8 @@ pub extern "C" fn proxmox_backup_new(
         let backup_id = tools::utf8_c_string(backup_id)?
             .ok_or_else(|| format_err!("backup_id must not be NULL"))?;
 
-        let password = tools::utf8_c_string(password)?;
+        let password = tools::utf8_c_string(password)?
+            .ok_or_else(|| format_err!("password must not be NULL"))?;
         let keyfile = tools::utf8_c_string(keyfile)?.map(std::path::PathBuf::from);
         let key_password = tools::utf8_c_string(key_password)?;
         let fingerprint = tools::utf8_c_string(fingerprint)?;
@@ -701,7 +702,8 @@ pub extern "C" fn proxmox_restore_new(
         let backup_id = snapshot.group().backup_id().to_owned();
         let backup_time = snapshot.backup_time();
 
-        let password = tools::utf8_c_string(password)?;
+        let password = tools::utf8_c_string(password)?
+            .ok_or_else(|| format_err!("password must not be null"))?;
         let keyfile = tools::utf8_c_string(keyfile)?.map(std::path::PathBuf::from);
         let key_password = tools::utf8_c_string(key_password)?;
         let fingerprint = tools::utf8_c_string(fingerprint)?;
diff --git a/src/restore.rs b/src/restore.rs
index 1cc99a6..0790d7f 100644
--- a/src/restore.rs
+++ b/src/restore.rs
@@ -75,9 +75,10 @@ impl RestoreTask {
 
     pub async fn connect(&self) -> Result<libc::c_int, Error> {
 
-        let options = HttpClientOptions::new()
-            .fingerprint(self.setup.fingerprint.clone())
-            .password(self.setup.password.clone());
+        let options = HttpClientOptions::new_non_interactive(
+            self.setup.password.clone(),
+            self.setup.fingerprint.clone(),
+        );
 
         let http = HttpClient::new(&self.setup.host, self.setup.port, &self.setup.auth_id, options)?;
         let client = BackupReader::start(
-- 
2.20.1





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [pbs-devel] applied series: [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings
  2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
                   ` (16 preceding siblings ...)
  2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup-qemu 2/2] use new HttpClientOptions constructors Fabian Grünbichler
@ 2021-01-26  9:44 ` Wolfgang Bumiller
  17 siblings, 0 replies; 19+ messages in thread
From: Wolfgang Bumiller @ 2021-01-26  9:44 UTC (permalink / raw)
  To: Fabian Grünbichler; +Cc: pbs-devel

applied series

On Mon, Jan 25, 2021 at 02:42:45PM +0100, Fabian Grünbichler wrote:
> various parameter/types refactorings to simplify function
> signatures/return types/...
> 
> I am grateful for suggestions on better names ;)
> 
> I tried to order/group the patches so that they also apply
> individually/as sub-groups.
> 
> with all of these, we are now down to some very recently touched files
> with trivial fixes that I didn't want to change since I suspected them
> to be still under development, and one missing docs lint:
> 
> warning: unsafe function's docs miss `# Safety` section
>   --> src/tools/fs.rs:78:5
>    |
> 78 | /     pub unsafe fn file_name_utf8_unchecked(&self) -> &str {
> 79 | |         std::str::from_utf8_unchecked(self.file_name().to_bytes())
> 80 | |     }
>    | |_____^
> 
> proxmox-backup:
>   report: type-alias function call tuple
>   broadcast_future: refactor broadcast/future binding
>   client: refactor catalog upload spawning
>   allow complex Futures in tower_service impl
>   async index reader: typedef ReadFuture
>   systemd/time: extract Time/DateSpec structs
>   client: factor out UploadOptions
>   pxar: typedef on_error as ErrorHandler
>   pxar: factor out PxarCreateOptions
>   pxar: extract PxarExtractOptions
>   authid: make Tokenname(Ref) derive Eq
>   derive/impl and use Default for some structs
>   verify: factor out common parameters
>   clippy: allow api functions with many arguments
>   clippy: more misc fixes
> 
>  examples/download-speed.rs             |   2 +-
>  examples/upload-speed.rs               |   2 +-
>  src/api2/access/acl.rs                 |   1 +
>  src/api2/access/tfa.rs                 |   1 +
>  src/api2/access/user.rs                |   1 +
>  src/api2/admin/datastore.rs            |  19 +--
>  src/api2/backup/environment.rs         |  10 +-
>  src/api2/config/datastore.rs           |   1 +
>  src/api2/config/remote.rs              |   5 +-
>  src/api2/config/sync.rs                |   1 +
>  src/api2/config/verify.rs              |   1 +
>  src/api2/node/network.rs               |   2 +
>  src/api2/node/tasks.rs                 |   1 +
>  src/api2/types/userid.rs               |  52 +-------
>  src/backup/async_index_reader.rs       |   4 +-
>  src/backup/prune.rs                    |   1 +
>  src/backup/verify.rs                   | 174 +++++++++++--------------
>  src/bin/proxmox-backup-client.rs       | 162 +++++++++++++----------
>  src/bin/proxmox-daily-update.rs        |   9 +-
>  src/bin/proxmox_backup_client/key.rs   |  24 +---
>  src/bin/proxmox_backup_client/mount.rs |   2 +-
>  src/bin/proxmox_backup_manager/user.rs |   4 +-
>  src/bin/pxar.rs                        |  54 ++++----
>  src/client.rs                          |  11 +-
>  src/client/backup_writer.rs            |  46 ++++---
>  src/client/http_client.rs              |  38 ++++--
>  src/client/pull.rs                     |   4 +-
>  src/client/pxar_backup_stream.rs       |  28 +---
>  src/config/acl.rs                      |   8 +-
>  src/config/network.rs                  |   2 +-
>  src/pxar/create.rs                     |  28 +++-
>  src/pxar/extract.rs                    |  24 ++--
>  src/pxar/mod.rs                        |   4 +-
>  src/rrd/mod.rs                         |   1 +
>  src/server/h2service.rs                |   5 +-
>  src/server/report.rs                   |   6 +-
>  src/server/rest.rs                     |   3 +-
>  src/server/verify_job.rs               |   3 +-
>  src/tools/broadcast_future.rs          |  38 ++----
>  src/tools/http.rs                      |   5 +-
>  src/tools/systemd/parse_time.rs        |  41 ++++--
>  tests/catar.rs                         |  10 +-
>  42 files changed, 414 insertions(+), 424 deletions(-)
> 
> proxmox-backup-qemu:
>   use UploadOptions for uploading Blobs
>   use new HttpClientOptions constructors
> 
>  src/backup.rs   |  7 ++++---
>  src/commands.rs | 15 +++++++++++++--
>  src/lib.rs      |  8 +++++---
>  src/restore.rs  |  7 ++++---
>  4 files changed, 26 insertions(+), 11 deletions(-)
> 
> -- 
> 2.20.1




^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-01-26  9:45 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-25 13:42 [pbs-devel] [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 01/15] report: type-alias function call tuple Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 02/15] broadcast_future: refactor broadcast/future binding Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 03/15] client: refactor catalog upload spawning Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 04/15] allow complex Futures in tower_service impl Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 05/15] async index reader: typedef ReadFuture Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 06/15] systemd/time: extract Time/DateSpec structs Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 07/15] client: factor out UploadOptions Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 08/15] pxar: typedef on_error as ErrorHandler Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 09/15] pxar: factor out PxarCreateOptions Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 10/15] pxar: extract PxarExtractOptions Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 11/15] authid: make Tokenname(Ref) derive Eq Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 12/15] derive/impl and use Default for some structs Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 13/15] verify: factor out common parameters Fabian Grünbichler
2021-01-25 13:42 ` [pbs-devel] [PATCH proxmox-backup 14/15] clippy: allow api functions with many arguments Fabian Grünbichler
2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup 15/15] clippy: more misc fixes Fabian Grünbichler
2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup-qemu 1/2] use UploadOptions for uploading Blobs Fabian Grünbichler
2021-01-25 13:43 ` [pbs-devel] [PATCH proxmox-backup-qemu 2/2] use new HttpClientOptions constructors Fabian Grünbichler
2021-01-26  9:44 ` [pbs-devel] applied series: [PATCH proxmox-backup(-qemu) 00/17] clippy refactorings Wolfgang Bumiller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal