* [pbs-devel] [PATCH proxmox-backup] fix #3323: dry-run for cli backup subcommand
@ 2022-02-17 12:58 Markus Frank
2022-02-18 7:38 ` Thomas Lamprecht
0 siblings, 1 reply; 2+ messages in thread
From: Markus Frank @ 2022-02-17 12:58 UTC (permalink / raw)
To: pbs-devel
adds a dry-run parameter for "proxmox-backup-client backup".
With this parameter on, it simply prints out what would be uploaded,
instead of uploading it.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
proxmox-backup-client/src/main.rs | 194 +++++++++++++++++-------------
1 file changed, 112 insertions(+), 82 deletions(-)
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 081028f2..b992695c 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -613,6 +613,11 @@ fn spawn_catalog_upload(
description: "Verbose output.",
optional: true,
},
+ "dry-run": {
+ type: Boolean,
+ description: "Just show what backup would do, but do not upload anything.",
+ optional: true,
+ },
}
}
)]
@@ -633,6 +638,8 @@ async fn create_backup(
let verbose = param["verbose"].as_bool().unwrap_or(false);
+ let dry_run = param["dry-run"].as_bool().unwrap_or(false);
+
let backup_time_opt = param["backup-time"].as_i64();
let chunk_size_opt = param["chunk-size"].as_u64().map(|v| (v*1024) as usize);
@@ -845,94 +852,117 @@ async fn create_backup(
let mut catalog_result_rx = None;
for (backup_type, filename, target, size) in upload_list {
- match backup_type {
- BackupSpecificationType::CONFIG => {
- let upload_options = UploadOptions {
- compress: true,
- encrypt: crypto.mode == CryptMode::Encrypt,
- ..UploadOptions::default()
- };
-
- println!("Upload config file '{}' to '{}' as {}", filename, repo, target);
- let stats = client
- .upload_blob_from_file(&filename, &target, upload_options)
- .await?;
- manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
- }
- BackupSpecificationType::LOGFILE => { // fixme: remove - not needed anymore ?
- let upload_options = UploadOptions {
- compress: true,
- encrypt: crypto.mode == CryptMode::Encrypt,
- ..UploadOptions::default()
- };
-
- println!("Upload log file '{}' to '{}' as {}", filename, repo, target);
- let stats = client
- .upload_blob_from_file(&filename, &target, upload_options)
- .await?;
- manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
- }
- BackupSpecificationType::PXAR => {
- // start catalog upload on first use
- if catalog.is_none() {
- let catalog_upload_res = spawn_catalog_upload(client.clone(), crypto.mode == CryptMode::Encrypt)?;
- catalog = Some(catalog_upload_res.catalog_writer);
- catalog_result_rx = Some(catalog_upload_res.result);
+ if dry_run {
+ match backup_type {
+ BackupSpecificationType::CONFIG => {
+ println!("Would upload config file'{}' to '{}' as {}", filename, repo, target);
+ }
+ BackupSpecificationType::LOGFILE => {
+ println!("Would upload log file '{}' to '{}' as {}", filename, repo, target);
+ }
+ BackupSpecificationType::PXAR => {
+ println!("Would upload directory '{}' to '{}' as {}", filename, repo, target);
+ }
+ BackupSpecificationType::IMAGE => {
+ println!("Would upload image'{}' to '{}' as {}", filename, repo, target);
}
- let catalog = catalog.as_ref().unwrap();
-
- 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 = pbs_client::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,
- encrypt: crypto.mode == CryptMode::Encrypt,
- ..UploadOptions::default()
- };
-
- let stats = backup_directory(
- &client,
- &filename,
- &target,
- chunk_size_opt,
- catalog.clone(),
- pxar_options,
- upload_options,
- ).await?;
- manifest.add_file(target, stats.size, stats.csum, crypto.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: crypto.mode == CryptMode::Encrypt,
- };
-
- let stats = backup_image(
- &client,
- &filename,
- &target,
- chunk_size_opt,
- upload_options,
- ).await?;
- manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
+ } else {
+ match backup_type {
+ BackupSpecificationType::CONFIG => {
+ let upload_options = UploadOptions {
+ compress: true,
+ encrypt: crypto.mode == CryptMode::Encrypt,
+ ..UploadOptions::default()
+ };
+
+ println!("Upload config file '{}' to '{}' as {}", filename, repo, target);
+ let stats = client
+ .upload_blob_from_file(&filename, &target, upload_options)
+ .await?;
+ manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
+ }
+ BackupSpecificationType::LOGFILE => { // fixme: remove - not needed anymore ?
+ let upload_options = UploadOptions {
+ compress: true,
+ encrypt: crypto.mode == CryptMode::Encrypt,
+ ..UploadOptions::default()
+ };
+
+ println!("Upload log file '{}' to '{}' as {}", filename, repo, target);
+ let stats = client
+ .upload_blob_from_file(&filename, &target, upload_options)
+ .await?;
+ manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
+ }
+ BackupSpecificationType::PXAR => {
+ // start catalog upload on first use
+ if catalog.is_none() {
+ let catalog_upload_res = spawn_catalog_upload(client.clone(), crypto.mode == CryptMode::Encrypt)?;
+ catalog = Some(catalog_upload_res.catalog_writer);
+ catalog_result_rx = Some(catalog_upload_res.result);
+ }
+ let catalog = catalog.as_ref().unwrap();
+
+ 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 = pbs_client::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,
+ encrypt: crypto.mode == CryptMode::Encrypt,
+ ..UploadOptions::default()
+ };
+
+ let stats = backup_directory(
+ &client,
+ &filename,
+ &target,
+ chunk_size_opt,
+ catalog.clone(),
+ pxar_options,
+ upload_options,
+ ).await?;
+ manifest.add_file(target, stats.size, stats.csum, crypto.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: crypto.mode == CryptMode::Encrypt,
+ };
+
+ let stats = backup_image(
+ &client,
+ &filename,
+ &target,
+ chunk_size_opt,
+ upload_options,
+ ).await?;
+ manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
+ }
}
}
}
+ if dry_run {
+ println!("dry-run: no upload happend");
+ return Ok(Value::Null);
+ }
+
// finalize and upload catalog
if let Some(catalog) = catalog {
let mutex = Arc::try_unwrap(catalog)
--
2.30.2
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] fix #3323: dry-run for cli backup subcommand
2022-02-17 12:58 [pbs-devel] [PATCH proxmox-backup] fix #3323: dry-run for cli backup subcommand Markus Frank
@ 2022-02-18 7:38 ` Thomas Lamprecht
0 siblings, 0 replies; 2+ messages in thread
From: Thomas Lamprecht @ 2022-02-18 7:38 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Markus Frank
On 17.02.22 13:58, Markus Frank wrote:
> adds a dry-run parameter for "proxmox-backup-client backup".
> With this parameter on, it simply prints out what would be uploaded,
> instead of uploading it.
>
looks ok in general, some comments to code style, or better said, refactor
potential inline.
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> proxmox-backup-client/src/main.rs | 194 +++++++++++++++++-------------
> 1 file changed, 112 insertions(+), 82 deletions(-)
>
> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
> index 081028f2..b992695c 100644
> --- a/proxmox-backup-client/src/main.rs
> +++ b/proxmox-backup-client/src/main.rs
> @@ -613,6 +613,11 @@ fn spawn_catalog_upload(
> description: "Verbose output.",
> optional: true,
> },
> + "dry-run": {
> + type: Boolean,
> + description: "Just show what backup would do, but do not upload anything.",
> + optional: true,
> + },
> }
> }
> )]
> @@ -633,6 +638,8 @@ async fn create_backup(
>
> let verbose = param["verbose"].as_bool().unwrap_or(false);
>
> + let dry_run = param["dry-run"].as_bool().unwrap_or(false);
> +
> let backup_time_opt = param["backup-time"].as_i64();
>
> let chunk_size_opt = param["chunk-size"].as_u64().map(|v| (v*1024) as usize);
> @@ -845,94 +852,117 @@ async fn create_backup(
> let mut catalog_result_rx = None;
>
> for (backup_type, filename, target, size) in upload_list {
> - match backup_type {
did you tried of either matching the tuple (backup_type, dry_run) or using if's in
the match arm, e.g., something like:
match backup_type {
BackupSpecificationType::CONFIG if dry_run => printdry("config file", filename),
BackupSpecificationType::LOGFILE if dry_run => printdry("log file", filename),
...
BackupSpecificationType::CONFIG => {
let upload_options = UploadOptions {
...
},
...,
}
The one with matching the tuple has the slight advantage of ensuring that we'd ensure
to always cover all possibilities, that'd be relevant e.g., if a new type gets added
to avoid it's send by mistake if one forgot to add an explicit dry run here, i.e.,
something that your current solution also avoids.
> - BackupSpecificationType::CONFIG => {
> - let upload_options = UploadOptions {
> - compress: true,
> - encrypt: crypto.mode == CryptMode::Encrypt,
> - ..UploadOptions::default()
> - };
> -
> - println!("Upload config file '{}' to '{}' as {}", filename, repo, target);
> - let stats = client
> - .upload_blob_from_file(&filename, &target, upload_options)
> - .await?;
> - manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
> - }
> - BackupSpecificationType::LOGFILE => { // fixme: remove - not needed anymore ?
> - let upload_options = UploadOptions {
> - compress: true,
> - encrypt: crypto.mode == CryptMode::Encrypt,
> - ..UploadOptions::default()
> - };
> -
> - println!("Upload log file '{}' to '{}' as {}", filename, repo, target);
> - let stats = client
> - .upload_blob_from_file(&filename, &target, upload_options)
> - .await?;
> - manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
> - }
> - BackupSpecificationType::PXAR => {
> - // start catalog upload on first use
> - if catalog.is_none() {
> - let catalog_upload_res = spawn_catalog_upload(client.clone(), crypto.mode == CryptMode::Encrypt)?;
> - catalog = Some(catalog_upload_res.catalog_writer);
> - catalog_result_rx = Some(catalog_upload_res.result);
> + if dry_run {
> + match backup_type {
> + BackupSpecificationType::CONFIG => {
> + println!("Would upload config file'{}' to '{}' as {}", filename, repo, target);
please factor above print out in a local closure/helper-fn, besides reducing repetition
it would also avoid on having inconsistencies like missing a whitespace after "file" ;-)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-02-18 7:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-17 12:58 [pbs-devel] [PATCH proxmox-backup] fix #3323: dry-run for cli backup subcommand Markus Frank
2022-02-18 7:38 ` Thomas Lamprecht
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal