[pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println
Shannon Sterz
s.sterz at proxmox.com
Wed Nov 13 12:41:21 CET 2024
comments in-line:
On Mon Nov 11, 2024 at 2:08 PM CET, Filip Schauer wrote:
> Use log level "info" by default and prevent spamming messages for every
> single chunk uploaded. To re-enable these messages, set the RUST_LOG
> environment variable to "debug".
>
> Signed-off-by: Filip Schauer <f.schauer at proxmox.com>
> ---
> Cargo.toml | 2 ++
> src/main.rs | 28 ++++++++++++++++++++++------
> src/vma2pbs.rs | 38 ++++++++++++++++++++------------------
> 3 files changed, 44 insertions(+), 24 deletions(-)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index ad80304..7951bbc 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -8,7 +8,9 @@ edition = "2021"
> anyhow = "1.0"
> bincode = "1.3"
> chrono = "0.4"
> +env_logger = "0.10"
> hyper = "0.14.5"
> +log = "0.4"
> pico-args = "0.5"
> md5 = "0.7.0"
> regex = "1.7"
> diff --git a/src/main.rs b/src/main.rs
> index d4b36fa..203196b 100644
> --- a/src/main.rs
> +++ b/src/main.rs
> @@ -6,6 +6,7 @@ use std::path::PathBuf;
>
> use anyhow::{bail, Context, Error};
> use chrono::NaiveDateTime;
> +use env_logger::Target;
> use proxmox_sys::linux::tty;
> use proxmox_time::epoch_i64;
> use regex::Regex;
> @@ -128,7 +129,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> {
>
> match (encrypt, keyfile.is_some()) {
> (true, false) => bail!("--encrypt requires a --keyfile!"),
> - (false, true) => println!(
> + (false, true) => log::info!(
> "--keyfile given, but --encrypt not set -> backup will be signed, but not encrypted!"
> ),
> _ => {}
> @@ -190,7 +191,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> {
>
> Some(key_password)
> } else if vma_file_path.is_none() {
> - println!(
> + log::info!(
> "Please use --key-password-file to provide the password when passing the VMA file \
> to stdin, if required."
> );
> @@ -246,13 +247,17 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> {
> let Some((_, [backup_id, timestr, ext])) =
> re.captures(file_name).map(|c| c.extract())
> else {
> - // Skip the file, since it is not a VMA backup
> + log::debug!("Skip \"{file_name}\", since it is not a VMA backup");
> continue;
> };
>
> if let Some(ref vmid) = vmid {
> if backup_id != vmid {
> - // Skip the backup, since it does not match the specified vmid
> + log::debug!(
> + "Skip backup with VMID {}, since it does not match specified VMID {}",
> + backup_id,
> + vmid
nit: you can use format strings here
> + );
> continue;
> }
> }
> @@ -308,14 +313,14 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> {
> bail!("Did not find any backup archives");
> }
>
> - println!(
> + log::info!(
> "Found {} backup archive(s) of {} different VMID(s):",
> total_vma_count,
> grouped_vmas.len()
> );
>
> for (backup_id, vma_group) in &grouped_vmas {
> - println!("- VMID {}: {} backups", backup_id, vma_group.len());
> + log::info!("- VMID {}: {} backups", backup_id, vma_group.len());
> }
nit: if you are already touching this, move this over to format strings
as well
>
> if !yes {
> @@ -358,7 +363,18 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> {
> Ok(options)
> }
>
> +fn init_cli_logger() {
> + env_logger::Builder::from_env(env_logger::Env::new().filter_or("RUST_LOG", "info"))
> + .format_level(false)
> + .format_target(false)
> + .format_timestamp(None)
> + .target(Target::Stdout)
> + .init();
> +}
> +
> fn main() -> Result<(), Error> {
> + init_cli_logger();
> +
> let args = parse_args()?;
> vma2pbs(args)?;
>
> diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs
> index a5b4027..0517251 100644
> --- a/src/vma2pbs.rs
> +++ b/src/vma2pbs.rs
> @@ -82,8 +82,8 @@ fn create_pbs_backup_task(
> pbs_args: &PbsArgs,
> backup_args: &VmaBackupArgs,
> ) -> Result<*mut ProxmoxBackupHandle, Error> {
> - println!(
> - "backup time: {}",
> + log::info!(
> + "\tbackup time: {}",
> epoch_to_rfc3339(backup_args.backup_time)?
> );
>
> @@ -152,7 +152,7 @@ where
> let config_name = config.name;
> let config_data = config.content;
>
> - println!("CFG: size: {} name: {}", config_data.len(), config_name);
> + log::info!("\tCFG: size: {} name: {}", config_data.len(), config_name);
nit: move `config_name` into the format string
>
> let config_name_cstr = CString::new(config_name)?;
>
> @@ -190,9 +190,11 @@ where
> let device_name = vma_reader.get_device_name(device_id.try_into()?)?;
> let device_size = vma_reader.get_device_size(device_id.try_into()?)?;
>
> - println!(
> - "DEV: dev_id={} size: {} devname: {}",
> - device_id, device_size, device_name
> + log::info!(
> + "\tDEV: dev_id={} size: {} devname: {}",
nit: format string
> + device_id,
> + device_size,
> + device_name
> );
>
> let device_name_cstr = CString::new(device_name)?;
> @@ -276,8 +278,8 @@ where
> };
>
> let pbs_upload_chunk = |pbs_chunk_buffer: Option<&[u8]>| {
> - println!(
> - "Uploading dev_id: {} offset: {:#0X} - {:#0X}",
> + log::debug!(
> + "\tUploading dev_id: {} offset: {:#0X} - {:#0X}",
nit: format string, for example:
`\tUploading dev_id: {dev_id} offset: {pbs_chunk_offset:#0X} - {:#0X}`
> dev_id,
> pbs_chunk_offset,
> pbs_chunk_offset + pbs_chunk_size,
> @@ -466,13 +468,13 @@ fn set_notes(
>
> pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> {
> let pbs_args = &args.pbs_args;
> - println!("PBS repository: {}", pbs_args.pbs_repository);
> + log::info!("PBS repository: {}", pbs_args.pbs_repository);
> if let Some(ns) = &pbs_args.namespace {
> - println!("PBS namespace: {}", ns);
> + log::info!("PBS namespace: {}", ns);
nit: format string
> }
> - println!("PBS fingerprint: {}", pbs_args.fingerprint);
> - println!("compress: {}", pbs_args.compress);
> - println!("encrypt: {}", pbs_args.encrypt);
> + log::info!("PBS fingerprint: {}", pbs_args.fingerprint);
> + log::info!("compress: {}", pbs_args.compress);
> + log::info!("encrypt: {}", pbs_args.encrypt);
>
> let start_transfer_time = SystemTime::now();
>
> @@ -486,8 +488,8 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> {
> );
>
> if args.skip_failed {
> - eprintln!("{}", err_msg);
> - println!("Skipping VMID {}", backup_args.backup_id);
> + log::warn!("{}", err_msg);
> + log::info!("Skipping VMID {}", backup_args.backup_id);
> break;
> } else {
> bail!(err_msg);
> @@ -501,15 +503,15 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> {
> let minutes = total_seconds / 60;
> let seconds = total_seconds % 60;
> let milliseconds = transfer_duration.as_millis() % 1000;
> - println!("Backup finished within {minutes} minutes, {seconds} seconds and {milliseconds} ms");
> + log::info!("Backup finished within {minutes} minutes, {seconds} seconds and {milliseconds} ms");
>
> Ok(())
> }
>
> fn upload_vma_file(pbs_args: &PbsArgs, backup_args: &VmaBackupArgs) -> Result<(), Error> {
> match &backup_args.vma_file_path {
> - Some(vma_file_path) => println!("Uploading VMA backup from {:?}", vma_file_path),
> - None => println!("Uploading VMA backup from (stdin)"),
> + Some(vma_file_path) => log::info!("Uploading VMA backup from {:?}", vma_file_path),
nit: format string
> + None => log::info!("Uploading VMA backup from (stdin)"),
> };
>
> let vma_file: Box<dyn BufRead> = match &backup_args.compression {
More information about the pbs-devel
mailing list