* [pbs-devel] [PATCH vma-to-pbs v5 0/4] add support for bulk import of a dump directory @ 2024-11-11 13:08 Filip Schauer 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 1/4] " Filip Schauer ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Filip Schauer @ 2024-11-11 13:08 UTC (permalink / raw) To: pbs-devel When a path to a directory is provided in the vma_file argument, try to upload all VMA backups in the directory. This also handles compressed VMA files, notes and logs. If a vmid is specified with --vmid, only the backups of that particular vmid are uploaded. Also improve the readability of the log messages to keep track of all imported backups. Changed since v4: * Switch grouped_vmas from Vec<Vec<>> to HashMap<String, Vec<>> * Remove dependency on itertools * bail when no backups were found * Default to yes on the bulk import confirmation prompt * bail on invalid input to the bulk import confirmation prompt Changed since v3: * Mention in the description of the --vmid argument, that it is required if a single VMA file is provided * Construct grouped_vmas in place * Add debug logs when gathering files for bulk import * Log a summary of the files gathered for bulk import * Remove the "confusing VMA file path" error message in the second commit * Switch chunk_stats from Arc<Mutex<[u64; 256]>> to Arc<[AtomicU64; 256]> and use fetch_add to atomically increment and fetch the chunk stat * Ask for confirmation before bulk import * Add --yes option to skip the confirmation prompt Changed since v2: * Make skipping a VMID on error optional with the --skip-failed option * Switch log output from stderr to stdout * Bump itertools to 0.13 Changed since v1: * Do not recurse through dump directory * Compile regex once before iterating over the files in the dump directory * Use extract on regex capture groups * Do not use deprecated method `chrono::NaiveDateTime::timestamp` * Use proxmox_sys::fs::file_read_optional_string * Group VMA files by VMID and continue with next VMID on error * Move the BackupVmaToPbsArgs split into its own commit * Remove hard coded occurences of 255 * Use level-based logging instead of println Filip Schauer (4): add support for bulk import of a dump directory add option to skip vmids whose backups failed to upload use level-based logging instead of println log device upload progress as a percentage Cargo.toml | 4 + src/main.rs | 193 +++++++++++++++++++++++++++++++++++++++++++++---- src/vma2pbs.rs | 110 ++++++++++++++++++++++------ 3 files changed, 272 insertions(+), 35 deletions(-) -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs v5 1/4] add support for bulk import of a dump directory 2024-11-11 13:08 [pbs-devel] [PATCH vma-to-pbs v5 0/4] add support for bulk import of a dump directory Filip Schauer @ 2024-11-11 13:08 ` Filip Schauer 2024-11-13 11:41 ` Shannon Sterz 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 2/4] add option to skip vmids whose backups failed to upload Filip Schauer ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Filip Schauer @ 2024-11-11 13:08 UTC (permalink / raw) To: pbs-devel When a path to a directory is provided in the vma_file argument, try to upload all VMA backups in the directory. This also handles compressed VMA files, notes and logs. If a vmid is specified with --vmid, only the backups of that particular vmid are uploaded. This is intended for use on a dump directory: PBS_FINGERPRINT='PBS_FINGERPRINT' vma-to-pbs \ --repository 'user@realm!token@server:port:datastore' \ /var/lib/vz/dump Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- Cargo.toml | 2 + src/main.rs | 167 +++++++++++++++++++++++++++++++++++++++++++++---- src/vma2pbs.rs | 64 ++++++++++++++++--- 3 files changed, 214 insertions(+), 19 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cd13426..ad80304 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,9 +7,11 @@ edition = "2021" [dependencies] anyhow = "1.0" bincode = "1.3" +chrono = "0.4" hyper = "0.14.5" pico-args = "0.5" md5 = "0.7.0" +regex = "1.7" scopeguard = "1.1.0" serde = "1.0" serde_json = "1.0" diff --git a/src/main.rs b/src/main.rs index 3e25591..a394078 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,26 +1,35 @@ +use std::collections::HashMap; use std::ffi::OsString; +use std::fs::read_dir; +use std::io::{BufRead, BufReader, Write}; +use std::path::PathBuf; use anyhow::{bail, Context, Error}; +use chrono::NaiveDateTime; use proxmox_sys::linux::tty; use proxmox_time::epoch_i64; +use regex::Regex; mod vma; mod vma2pbs; -use vma2pbs::{vma2pbs, BackupVmaToPbsArgs, PbsArgs, VmaBackupArgs}; +use vma2pbs::{vma2pbs, BackupVmaToPbsArgs, Compression, PbsArgs, VmaBackupArgs}; const CMD_HELP: &str = "\ Usage: vma-to-pbs [OPTIONS] --repository <auth_id@host:port:datastore> --vmid <VMID> [vma_file] Arguments: - [vma_file] + [vma_file | dump_directory] Options: --repository <auth_id@host:port:datastore> Repository URL [--ns <NAMESPACE>] Namespace - --vmid <VMID> + [--vmid <VMID>] Backup ID + This is required if a single VMA file is provided. + If not specified, bulk import all VMA backups in the provided directory. + If specified with a dump directory, only import backups of the specified vmid. [--backup-time <EPOCH>] Backup timestamp --fingerprint <FINGERPRINT> @@ -41,6 +50,8 @@ Options: File containing a comment/notes [--log-file <LOG_FILE>] Log file + -y, --yes + Automatic yes to prompts -h, --help Print help -V, --version @@ -52,7 +63,16 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { args.remove(0); // remove the executable path. let mut first_later_args_index = 0; - let options = ["-h", "--help", "-c", "--compress", "-e", "--encrypt"]; + let options = [ + "-h", + "--help", + "-c", + "--compress", + "-e", + "--encrypt", + "-y", + "--yes", + ]; for (i, arg) in args.iter().enumerate() { if let Some(arg) = arg.to_str() { @@ -87,7 +107,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { let pbs_repository = args.value_from_str("--repository")?; let namespace = args.opt_value_from_str("--ns")?; - let vmid = args.value_from_str("--vmid")?; + let vmid: Option<String> = args.opt_value_from_str("--vmid")?; let backup_time: Option<i64> = args.opt_value_from_str("--backup-time")?; let backup_time = backup_time.unwrap_or_else(epoch_i64); let fingerprint = args.opt_value_from_str("--fingerprint")?; @@ -99,6 +119,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { let key_password_file: Option<OsString> = args.opt_value_from_str("--key-password-file")?; let notes_file: Option<OsString> = args.opt_value_from_str("--notes-file")?; let log_file_path: Option<OsString> = args.opt_value_from_str("--log-file")?; + let yes = args.contains(["-y", "--yes"]); match (encrypt, keyfile.is_some()) { (true, false) => bail!("--encrypt requires a --keyfile!"), @@ -196,15 +217,137 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { encrypt, }; - let vma_args = VmaBackupArgs { - vma_file_path: vma_file_path.cloned(), - backup_id: vmid, - backup_time, - notes, - log_file_path, + let bulk = + vma_file_path + .map(PathBuf::from) + .and_then(|path| if path.is_dir() { Some(path) } else { None }); + + let grouped_vmas = if let Some(dump_dir_path) = bulk { + let re = Regex::new( + r"vzdump-qemu-(\d+)-(\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2}).vma(|.zst|.lzo|.gz)$", + )?; + + let mut vmas = Vec::new(); + + for entry in read_dir(dump_dir_path)? { + let entry = entry?; + let path = entry.path(); + + if !path.is_file() { + continue; + } + + if let Some(file_name) = path.file_name().and_then(|n| n.to_str()) { + 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 + continue; + }; + + if let Some(ref vmid) = vmid { + if backup_id != vmid { + // Skip the backup, since it does not match the specified vmid + continue; + } + } + + let compression = match ext { + "" => None, + ".zst" => Some(Compression::Zstd), + ".lzo" => Some(Compression::Lzo), + ".gz" => Some(Compression::GZip), + _ => bail!("Unexpected file extension: {ext}"), + }; + + let backup_time = NaiveDateTime::parse_from_str(timestr, "%Y_%m_%d-%H_%M_%S")? + .and_utc() + .timestamp(); + + let notes_path = path.with_file_name(format!("{}.notes", file_name)); + let notes = proxmox_sys::fs::file_read_optional_string(notes_path)?; + + let log_path = path.with_file_name(format!("{}.log", file_name)); + let log_file_path = if log_path.exists() { + Some(log_path.to_path_buf().into_os_string()) + } else { + None + }; + + let backup_args = VmaBackupArgs { + vma_file_path: Some(path.clone().into()), + compression, + backup_id: backup_id.to_string(), + backup_time, + notes, + log_file_path, + }; + vmas.push(backup_args); + } + } + + vmas.sort_by_key(|d| d.backup_time); + let total_vma_count = vmas.len(); + let grouped_vmas = vmas.into_iter().fold( + HashMap::new(), + |mut grouped: HashMap<String, Vec<VmaBackupArgs>>, vma_args| { + grouped + .entry(vma_args.backup_id.clone()) + .or_default() + .push(vma_args); + grouped + }, + ); + + if grouped_vmas.is_empty() { + bail!("Did not find any backup archives"); + } + + println!( + "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()); + } + + if !yes { + eprint!("Proceed with the bulk import? (Y/n): "); + std::io::stdout().flush()?; + let mut line = String::new(); + + BufReader::new(std::io::stdin()).read_line(&mut line)?; + let trimmed = line.trim(); + match trimmed { + "y" | "Y" | "" => {} + "n" | "N" => bail!("Bulk import was not confirmed."), + _ => bail!("Unexpected choice '{trimmed}'!"), + } + } + + grouped_vmas + } else if let Some(vmid) = vmid { + HashMap::from([( + vmid.clone(), + vec![VmaBackupArgs { + vma_file_path: vma_file_path.cloned(), + compression: None, + backup_id: vmid, + backup_time, + notes, + log_file_path, + }], + )]) + } else { + bail!("No vmid specified for single backup file"); }; - let options = BackupVmaToPbsArgs { pbs_args, vma_args }; + let options = BackupVmaToPbsArgs { + pbs_args, + grouped_vmas, + }; Ok(options) } diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs index a888a7b..95ede9b 100644 --- a/src/vma2pbs.rs +++ b/src/vma2pbs.rs @@ -4,6 +4,7 @@ use std::collections::HashMap; use std::ffi::{c_char, CStr, CString, OsString}; use std::fs::File; use std::io::{stdin, BufRead, BufReader, Read}; +use std::process::{Command, Stdio}; use std::ptr; use std::time::SystemTime; @@ -30,7 +31,7 @@ const VMA_CLUSTER_SIZE: usize = 65536; pub struct BackupVmaToPbsArgs { pub pbs_args: PbsArgs, - pub vma_args: VmaBackupArgs, + pub grouped_vmas: HashMap<String, Vec<VmaBackupArgs>>, } pub struct PbsArgs { @@ -45,8 +46,15 @@ pub struct PbsArgs { pub encrypt: bool, } +pub enum Compression { + Zstd, + Lzo, + GZip, +} + pub struct VmaBackupArgs { pub vma_file_path: Option<OsString>, + pub compression: Option<Compression>, pub backup_id: String, pub backup_time: i64, pub notes: Option<String>, @@ -467,7 +475,19 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { let start_transfer_time = SystemTime::now(); - upload_vma_file(pbs_args, &args.vma_args)?; + for (_, vma_group) in args.grouped_vmas { + for backup_args in vma_group { + if let Err(e) = upload_vma_file(pbs_args, &backup_args) { + eprintln!( + "Failed to upload vma file at {:?} - {}", + backup_args.vma_file_path.unwrap_or("(stdin)".into()), + e + ); + println!("Skipping VMID {}", backup_args.backup_id); + break; + } + } + } let transfer_duration = SystemTime::now().duration_since(start_transfer_time)?; let total_seconds = transfer_duration.as_secs(); @@ -480,13 +500,43 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { } fn upload_vma_file(pbs_args: &PbsArgs, backup_args: &VmaBackupArgs) -> Result<(), Error> { - let vma_file: Box<dyn BufRead> = match &backup_args.vma_file_path { - Some(vma_file_path) => match File::open(vma_file_path) { - Err(why) => return Err(anyhow!("Couldn't open file: {}", why)), - Ok(file) => Box::new(BufReader::new(file)), + 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)"), + }; + + let vma_file: Box<dyn BufRead> = match &backup_args.compression { + Some(compression) => { + let vma_file_path = backup_args + .vma_file_path + .as_ref() + .expect("No VMA file path provided"); + let mut cmd = match compression { + Compression::Zstd => { + let mut cmd = Command::new("zstd"); + cmd.args(["-q", "-d", "-c"]); + cmd + } + Compression::Lzo => { + let mut cmd = Command::new("lzop"); + cmd.args(["-d", "-c"]); + cmd + } + Compression::GZip => Command::new("zcat"), + }; + let process = cmd.arg(vma_file_path).stdout(Stdio::piped()).spawn()?; + let stdout = process.stdout.expect("Failed to capture stdout"); + Box::new(BufReader::new(stdout)) + } + None => match &backup_args.vma_file_path { + Some(vma_file_path) => match File::open(vma_file_path) { + Err(why) => return Err(anyhow!("Couldn't open file: {}", why)), + Ok(file) => Box::new(BufReader::new(file)), + }, + None => Box::new(BufReader::new(stdin())), }, - None => Box::new(BufReader::new(stdin())), }; + let vma_reader = VmaReader::new(vma_file)?; let pbs = create_pbs_backup_task(pbs_args, backup_args)?; -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs v5 1/4] add support for bulk import of a dump directory 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 1/4] " Filip Schauer @ 2024-11-13 11:41 ` Shannon Sterz 2024-11-13 16:02 ` Filip Schauer 0 siblings, 1 reply; 12+ messages in thread From: Shannon Sterz @ 2024-11-13 11:41 UTC (permalink / raw) To: Proxmox Backup Server development discussion comments in-line: On Mon Nov 11, 2024 at 2:08 PM CET, Filip Schauer wrote: > When a path to a directory is provided in the vma_file argument, try to > upload all VMA backups in the directory. This also handles compressed > VMA files, notes and logs. If a vmid is specified with --vmid, only the > backups of that particular vmid are uploaded. > > This is intended for use on a dump directory: > > PBS_FINGERPRINT='PBS_FINGERPRINT' vma-to-pbs \ > --repository 'user@realm!token@server:port:datastore' \ > /var/lib/vz/dump > > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > Cargo.toml | 2 + > src/main.rs | 167 +++++++++++++++++++++++++++++++++++++++++++++---- > src/vma2pbs.rs | 64 ++++++++++++++++--- > 3 files changed, 214 insertions(+), 19 deletions(-) > > diff --git a/Cargo.toml b/Cargo.toml > index cd13426..ad80304 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -7,9 +7,11 @@ edition = "2021" > [dependencies] > anyhow = "1.0" > bincode = "1.3" > +chrono = "0.4" > hyper = "0.14.5" > pico-args = "0.5" > md5 = "0.7.0" > +regex = "1.7" > scopeguard = "1.1.0" > serde = "1.0" > serde_json = "1.0" > diff --git a/src/main.rs b/src/main.rs > index 3e25591..a394078 100644 > --- a/src/main.rs > +++ b/src/main.rs > @@ -1,26 +1,35 @@ > +use std::collections::HashMap; > use std::ffi::OsString; > +use std::fs::read_dir; > +use std::io::{BufRead, BufReader, Write}; > +use std::path::PathBuf; > > use anyhow::{bail, Context, Error}; > +use chrono::NaiveDateTime; > use proxmox_sys::linux::tty; > use proxmox_time::epoch_i64; > +use regex::Regex; > > mod vma; > mod vma2pbs; > -use vma2pbs::{vma2pbs, BackupVmaToPbsArgs, PbsArgs, VmaBackupArgs}; > +use vma2pbs::{vma2pbs, BackupVmaToPbsArgs, Compression, PbsArgs, VmaBackupArgs}; > > const CMD_HELP: &str = "\ > Usage: vma-to-pbs [OPTIONS] --repository <auth_id@host:port:datastore> --vmid <VMID> [vma_file] > > Arguments: > - [vma_file] > + [vma_file | dump_directory] > > Options: > --repository <auth_id@host:port:datastore> > Repository URL > [--ns <NAMESPACE>] > Namespace > - --vmid <VMID> > + [--vmid <VMID>] nit: this is marked as optional here (and in the code), but the usage line above still make it look like it's required. > Backup ID > + This is required if a single VMA file is provided. > + If not specified, bulk import all VMA backups in the provided directory. > + If specified with a dump directory, only import backups of the specified vmid. > [--backup-time <EPOCH>] > Backup timestamp > --fingerprint <FINGERPRINT> > @@ -41,6 +50,8 @@ Options: > File containing a comment/notes > [--log-file <LOG_FILE>] > Log file > + -y, --yes > + Automatic yes to prompts > -h, --help > Print help > -V, --version > @@ -52,7 +63,16 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { > args.remove(0); // remove the executable path. > > let mut first_later_args_index = 0; > - let options = ["-h", "--help", "-c", "--compress", "-e", "--encrypt"]; > + let options = [ > + "-h", > + "--help", > + "-c", > + "--compress", > + "-e", > + "--encrypt", > + "-y", > + "--yes", > + ]; > > for (i, arg) in args.iter().enumerate() { > if let Some(arg) = arg.to_str() { > @@ -87,7 +107,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { > > let pbs_repository = args.value_from_str("--repository")?; > let namespace = args.opt_value_from_str("--ns")?; > - let vmid = args.value_from_str("--vmid")?; > + let vmid: Option<String> = args.opt_value_from_str("--vmid")?; > let backup_time: Option<i64> = args.opt_value_from_str("--backup-time")?; > let backup_time = backup_time.unwrap_or_else(epoch_i64); > let fingerprint = args.opt_value_from_str("--fingerprint")?; > @@ -99,6 +119,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { > let key_password_file: Option<OsString> = args.opt_value_from_str("--key-password-file")?; > let notes_file: Option<OsString> = args.opt_value_from_str("--notes-file")?; > let log_file_path: Option<OsString> = args.opt_value_from_str("--log-file")?; > + let yes = args.contains(["-y", "--yes"]); > > match (encrypt, keyfile.is_some()) { > (true, false) => bail!("--encrypt requires a --keyfile!"), > @@ -196,15 +217,137 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { > encrypt, > }; > > - let vma_args = VmaBackupArgs { > - vma_file_path: vma_file_path.cloned(), > - backup_id: vmid, > - backup_time, > - notes, > - log_file_path, > + let bulk = > + vma_file_path > + .map(PathBuf::from) > + .and_then(|path| if path.is_dir() { Some(path) } else { None }); > + > + let grouped_vmas = if let Some(dump_dir_path) = bulk { > + let re = Regex::new( > + r"vzdump-qemu-(\d+)-(\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2}).vma(|.zst|.lzo|.gz)$", > + )?; > + > + let mut vmas = Vec::new(); > + > + for entry in read_dir(dump_dir_path)? { > + let entry = entry?; > + let path = entry.path(); > + > + if !path.is_file() { > + continue; > + } > + > + if let Some(file_name) = path.file_name().and_then(|n| n.to_str()) { > + 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 > + continue; > + }; > + > + if let Some(ref vmid) = vmid { > + if backup_id != vmid { > + // Skip the backup, since it does not match the specified vmid > + continue; > + } > + } > + > + let compression = match ext { > + "" => None, > + ".zst" => Some(Compression::Zstd), > + ".lzo" => Some(Compression::Lzo), > + ".gz" => Some(Compression::GZip), > + _ => bail!("Unexpected file extension: {ext}"), > + }; > + > + let backup_time = NaiveDateTime::parse_from_str(timestr, "%Y_%m_%d-%H_%M_%S")? > + .and_utc() > + .timestamp(); > + > + let notes_path = path.with_file_name(format!("{}.notes", file_name)); > + let notes = proxmox_sys::fs::file_read_optional_string(notes_path)?; > + > + let log_path = path.with_file_name(format!("{}.log", file_name)); > + let log_file_path = if log_path.exists() { > + Some(log_path.to_path_buf().into_os_string()) > + } else { > + None > + }; > + > + let backup_args = VmaBackupArgs { > + vma_file_path: Some(path.clone().into()), > + compression, > + backup_id: backup_id.to_string(), > + backup_time, > + notes, > + log_file_path, > + }; > + vmas.push(backup_args); > + } > + } > + > + vmas.sort_by_key(|d| d.backup_time); > + let total_vma_count = vmas.len(); > + let grouped_vmas = vmas.into_iter().fold( > + HashMap::new(), > + |mut grouped: HashMap<String, Vec<VmaBackupArgs>>, vma_args| { > + grouped > + .entry(vma_args.backup_id.clone()) > + .or_default() > + .push(vma_args); > + grouped > + }, > + ); > + > + if grouped_vmas.is_empty() { > + bail!("Did not find any backup archives"); > + } > + > + println!( > + "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()); nit: this should be: ```rs println!("- VMID {backup_id}: {} backups", vma_group.len()); ``` > + } > + > + if !yes { > + eprint!("Proceed with the bulk import? (Y/n): "); > + std::io::stdout().flush()?; > + let mut line = String::new(); > + > + BufReader::new(std::io::stdin()).read_line(&mut line)?; > + let trimmed = line.trim(); > + match trimmed { > + "y" | "Y" | "" => {} > + "n" | "N" => bail!("Bulk import was not confirmed."), > + _ => bail!("Unexpected choice '{trimmed}'!"), > + } > + } > + > + grouped_vmas > + } else if let Some(vmid) = vmid { > + HashMap::from([( > + vmid.clone(), > + vec![VmaBackupArgs { > + vma_file_path: vma_file_path.cloned(), > + compression: None, > + backup_id: vmid, > + backup_time, > + notes, > + log_file_path, > + }], > + )]) > + } else { > + bail!("No vmid specified for single backup file"); > }; > > - let options = BackupVmaToPbsArgs { pbs_args, vma_args }; > + let options = BackupVmaToPbsArgs { > + pbs_args, > + grouped_vmas, > + }; > > Ok(options) > } > diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs > index a888a7b..95ede9b 100644 > --- a/src/vma2pbs.rs > +++ b/src/vma2pbs.rs > @@ -4,6 +4,7 @@ use std::collections::HashMap; > use std::ffi::{c_char, CStr, CString, OsString}; > use std::fs::File; > use std::io::{stdin, BufRead, BufReader, Read}; > +use std::process::{Command, Stdio}; > use std::ptr; > use std::time::SystemTime; > > @@ -30,7 +31,7 @@ const VMA_CLUSTER_SIZE: usize = 65536; > > pub struct BackupVmaToPbsArgs { > pub pbs_args: PbsArgs, > - pub vma_args: VmaBackupArgs, > + pub grouped_vmas: HashMap<String, Vec<VmaBackupArgs>>, > } > > pub struct PbsArgs { > @@ -45,8 +46,15 @@ pub struct PbsArgs { > pub encrypt: bool, > } > > +pub enum Compression { > + Zstd, > + Lzo, > + GZip, > +} > + > pub struct VmaBackupArgs { > pub vma_file_path: Option<OsString>, > + pub compression: Option<Compression>, > pub backup_id: String, > pub backup_time: i64, > pub notes: Option<String>, > @@ -467,7 +475,19 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { > > let start_transfer_time = SystemTime::now(); > > - upload_vma_file(pbs_args, &args.vma_args)?; > + for (_, vma_group) in args.grouped_vmas { > + for backup_args in vma_group { > + if let Err(e) = upload_vma_file(pbs_args, &backup_args) { > + eprintln!( > + "Failed to upload vma file at {:?} - {}", > + backup_args.vma_file_path.unwrap_or("(stdin)".into()), > + e nit: same as above, move `e` into the format string > + ); > + println!("Skipping VMID {}", backup_args.backup_id); > + break; > + } > + } > + } > > let transfer_duration = SystemTime::now().duration_since(start_transfer_time)?; > let total_seconds = transfer_duration.as_secs(); > @@ -480,13 +500,43 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { > } > > fn upload_vma_file(pbs_args: &PbsArgs, backup_args: &VmaBackupArgs) -> Result<(), Error> { > - let vma_file: Box<dyn BufRead> = match &backup_args.vma_file_path { > - Some(vma_file_path) => match File::open(vma_file_path) { > - Err(why) => return Err(anyhow!("Couldn't open file: {}", why)), > - Ok(file) => Box::new(BufReader::new(file)), > + match &backup_args.vma_file_path { > + Some(vma_file_path) => println!("Uploading VMA backup from {:?}", vma_file_path), nit: this could be ```rs Some(vma_file_path) => println!("Uploading VMA backup from {vma_file_path:?}"), ``` > + None => println!("Uploading VMA backup from (stdin)"), > + }; > + > + let vma_file: Box<dyn BufRead> = match &backup_args.compression { > + Some(compression) => { > + let vma_file_path = backup_args > + .vma_file_path > + .as_ref() > + .expect("No VMA file path provided"); > + let mut cmd = match compression { > + Compression::Zstd => { > + let mut cmd = Command::new("zstd"); > + cmd.args(["-q", "-d", "-c"]); > + cmd i think the following would be more elegant here: ```rs Compression::Zstd => Command::new("zstd") .args(["-q", "-d", "-c"]), ``` it's a bit more concise imo > + } > + Compression::Lzo => { > + let mut cmd = Command::new("lzop"); > + cmd.args(["-d", "-c"]); > + cmd same as above > + } > + Compression::GZip => Command::new("zcat"), > + }; > + let process = cmd.arg(vma_file_path).stdout(Stdio::piped()).spawn()?; > + let stdout = process.stdout.expect("Failed to capture stdout"); > + Box::new(BufReader::new(stdout)) > + } > + None => match &backup_args.vma_file_path { > + Some(vma_file_path) => match File::open(vma_file_path) { > + Err(why) => return Err(anyhow!("Couldn't open file: {}", why)), nit: `why` can be moved into the format string here > + Ok(file) => Box::new(BufReader::new(file)), > + }, > + None => Box::new(BufReader::new(stdin())), > }, > - None => Box::new(BufReader::new(stdin())), > }; > + > let vma_reader = VmaReader::new(vma_file)?; > > let pbs = create_pbs_backup_task(pbs_args, backup_args)?; _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs v5 1/4] add support for bulk import of a dump directory 2024-11-13 11:41 ` Shannon Sterz @ 2024-11-13 16:02 ` Filip Schauer 0 siblings, 0 replies; 12+ messages in thread From: Filip Schauer @ 2024-11-13 16:02 UTC (permalink / raw) To: pbs-devel On 13/11/2024 12:41, Shannon Sterz wrote: >> const CMD_HELP: &str = "\ >> Usage: vma-to-pbs [OPTIONS] --repository <auth_id@host:port:datastore> --vmid <VMID> [vma_file] >> >> Arguments: >> - [vma_file] >> + [vma_file | dump_directory] >> >> Options: >> --repository <auth_id@host:port:datastore> >> Repository URL >> [--ns <NAMESPACE>] >> Namespace >> - --vmid <VMID> >> + [--vmid <VMID>] > nit: this is marked as optional here (and in the code), but the usage > line above still make it look like it's required. That usage line describes the command for a single VMA file. In that case `--vmid` actually is required. It is however not required for bulk import. So to make that clearer I made two usage lines in v6. On 13/11/2024 12:41, Shannon Sterz wrote: >> + let vma_file: Box<dyn BufRead> = match &backup_args.compression { >> + Some(compression) => { >> + let vma_file_path = backup_args >> + .vma_file_path >> + .as_ref() >> + .expect("No VMA file path provided"); >> + let mut cmd = match compression { >> + Compression::Zstd => { >> + let mut cmd = Command::new("zstd"); >> + cmd.args(["-q", "-d", "-c"]); >> + cmd > i think the following would be more elegant here: > > > ```rs > Compression::Zstd => Command::new("zstd") > .args(["-q", "-d", "-c"]), > ``` > > it's a bit more concise imo > >> + } >> + Compression::Lzo => { >> + let mut cmd = Command::new("lzop"); >> + cmd.args(["-d", "-c"]); >> + cmd > same as above Yeah I tried that, but unfortunatelly it does not compile: ``` error[E0716]: temporary value dropped while borrowed --> src/vma2pbs.rs:532:38 | 531 | let mut cmd = match compression { | ------- borrow later stored here 532 | Compression::Zstd => Command::new("zstd").args(["-q", "-d", "-c"]), | ^^^^^^^^^^^^^^^^^^^^ - temporary value is freed at the end of this statement | | | creates a temporary value which is freed while still in use | = note: consider using a `let` binding to create a longer lived value ``` _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs v5 2/4] add option to skip vmids whose backups failed to upload 2024-11-11 13:08 [pbs-devel] [PATCH vma-to-pbs v5 0/4] add support for bulk import of a dump directory Filip Schauer 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 1/4] " Filip Schauer @ 2024-11-11 13:08 ` Filip Schauer 2024-11-13 11:41 ` Shannon Sterz 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println Filip Schauer 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 4/4] log device upload progress as a percentage Filip Schauer 3 siblings, 1 reply; 12+ messages in thread From: Filip Schauer @ 2024-11-11 13:08 UTC (permalink / raw) To: pbs-devel Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- src/main.rs | 6 ++++++ src/vma2pbs.rs | 13 ++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index a394078..d4b36fa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -50,6 +50,9 @@ Options: File containing a comment/notes [--log-file <LOG_FILE>] Log file + --skip-failed + Skip VMIDs that failed to be uploaded and continue onto the next VMID if a dump directory + is specified. -y, --yes Automatic yes to prompts -h, --help @@ -70,6 +73,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { "--compress", "-e", "--encrypt", + "--skip-failed", "-y", "--yes", ]; @@ -119,6 +123,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { let key_password_file: Option<OsString> = args.opt_value_from_str("--key-password-file")?; let notes_file: Option<OsString> = args.opt_value_from_str("--notes-file")?; let log_file_path: Option<OsString> = args.opt_value_from_str("--log-file")?; + let skip_failed = args.contains("--skip-failed"); let yes = args.contains(["-y", "--yes"]); match (encrypt, keyfile.is_some()) { @@ -347,6 +352,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { let options = BackupVmaToPbsArgs { pbs_args, grouped_vmas, + skip_failed, }; Ok(options) diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs index 95ede9b..a5b4027 100644 --- a/src/vma2pbs.rs +++ b/src/vma2pbs.rs @@ -32,6 +32,7 @@ const VMA_CLUSTER_SIZE: usize = 65536; pub struct BackupVmaToPbsArgs { pub pbs_args: PbsArgs, pub grouped_vmas: HashMap<String, Vec<VmaBackupArgs>>, + pub skip_failed: bool, } pub struct PbsArgs { @@ -478,13 +479,19 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { for (_, vma_group) in args.grouped_vmas { for backup_args in vma_group { if let Err(e) = upload_vma_file(pbs_args, &backup_args) { - eprintln!( + let err_msg = format!( "Failed to upload vma file at {:?} - {}", backup_args.vma_file_path.unwrap_or("(stdin)".into()), e ); - println!("Skipping VMID {}", backup_args.backup_id); - break; + + if args.skip_failed { + eprintln!("{}", err_msg); + println!("Skipping VMID {}", backup_args.backup_id); + break; + } else { + bail!(err_msg); + } } } } -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs v5 2/4] add option to skip vmids whose backups failed to upload 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 2/4] add option to skip vmids whose backups failed to upload Filip Schauer @ 2024-11-13 11:41 ` Shannon Sterz 0 siblings, 0 replies; 12+ messages in thread From: Shannon Sterz @ 2024-11-13 11:41 UTC (permalink / raw) To: Proxmox Backup Server development discussion comments in-line: On Mon Nov 11, 2024 at 2:08 PM CET, Filip Schauer wrote: > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > src/main.rs | 6 ++++++ > src/vma2pbs.rs | 13 ++++++++++--- > 2 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/src/main.rs b/src/main.rs > index a394078..d4b36fa 100644 > --- a/src/main.rs > +++ b/src/main.rs > @@ -50,6 +50,9 @@ Options: > File containing a comment/notes > [--log-file <LOG_FILE>] > Log file > + --skip-failed > + Skip VMIDs that failed to be uploaded and continue onto the next VMID if a dump directory > + is specified. > -y, --yes > Automatic yes to prompts > -h, --help > @@ -70,6 +73,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { > "--compress", > "-e", > "--encrypt", > + "--skip-failed", > "-y", > "--yes", > ]; > @@ -119,6 +123,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { > let key_password_file: Option<OsString> = args.opt_value_from_str("--key-password-file")?; > let notes_file: Option<OsString> = args.opt_value_from_str("--notes-file")?; > let log_file_path: Option<OsString> = args.opt_value_from_str("--log-file")?; > + let skip_failed = args.contains("--skip-failed"); > let yes = args.contains(["-y", "--yes"]); > > match (encrypt, keyfile.is_some()) { > @@ -347,6 +352,7 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> { > let options = BackupVmaToPbsArgs { > pbs_args, > grouped_vmas, > + skip_failed, > }; > > Ok(options) > diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs > index 95ede9b..a5b4027 100644 > --- a/src/vma2pbs.rs > +++ b/src/vma2pbs.rs > @@ -32,6 +32,7 @@ const VMA_CLUSTER_SIZE: usize = 65536; > pub struct BackupVmaToPbsArgs { > pub pbs_args: PbsArgs, > pub grouped_vmas: HashMap<String, Vec<VmaBackupArgs>>, > + pub skip_failed: bool, > } > > pub struct PbsArgs { > @@ -478,13 +479,19 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { > for (_, vma_group) in args.grouped_vmas { > for backup_args in vma_group { > if let Err(e) = upload_vma_file(pbs_args, &backup_args) { > - eprintln!( > + let err_msg = format!( > "Failed to upload vma file at {:?} - {}", > backup_args.vma_file_path.unwrap_or("(stdin)".into()), > e nit: i'd move `e` into the format string, since you are basically already touching these lines :) > ); > - println!("Skipping VMID {}", backup_args.backup_id); > - break; > + > + if args.skip_failed { > + eprintln!("{}", err_msg); > + println!("Skipping VMID {}", backup_args.backup_id); > + break; > + } else { > + bail!(err_msg); > + } > } > } > } _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println 2024-11-11 13:08 [pbs-devel] [PATCH vma-to-pbs v5 0/4] add support for bulk import of a dump directory Filip Schauer 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 1/4] " Filip Schauer 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 2/4] add option to skip vmids whose backups failed to upload Filip Schauer @ 2024-11-11 13:08 ` Filip Schauer 2024-11-13 11:41 ` Shannon Sterz 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 4/4] log device upload progress as a percentage Filip Schauer 3 siblings, 1 reply; 12+ messages in thread From: Filip Schauer @ 2024-11-11 13:08 UTC (permalink / raw) To: pbs-devel 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@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 + ); 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()); } 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); 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: {}", + 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}", 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); } - 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), + None => log::info!("Uploading VMA backup from (stdin)"), }; let vma_file: Box<dyn BufRead> = match &backup_args.compression { -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println Filip Schauer @ 2024-11-13 11:41 ` Shannon Sterz 2024-11-13 16:02 ` Filip Schauer 0 siblings, 1 reply; 12+ messages in thread From: Shannon Sterz @ 2024-11-13 11:41 UTC (permalink / raw) To: Proxmox Backup Server development discussion 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@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 { _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println 2024-11-13 11:41 ` Shannon Sterz @ 2024-11-13 16:02 ` Filip Schauer 0 siblings, 0 replies; 12+ messages in thread From: Filip Schauer @ 2024-11-13 16:02 UTC (permalink / raw) To: pbs-devel On 13/11/2024 12:41, Shannon Sterz wrote: >> - // 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 I could, but then the line would exceed the 100 character limit and I would rather not split the string over multiple lines. _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs v5 4/4] log device upload progress as a percentage 2024-11-11 13:08 [pbs-devel] [PATCH vma-to-pbs v5 0/4] add support for bulk import of a dump directory Filip Schauer ` (2 preceding siblings ...) 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println Filip Schauer @ 2024-11-11 13:08 ` Filip Schauer 2024-11-13 11:41 ` Shannon Sterz 3 siblings, 1 reply; 12+ messages in thread From: Filip Schauer @ 2024-11-11 13:08 UTC (permalink / raw) To: pbs-devel Log the upload progress of a device as a percentage with log level info every 1000 chunks. Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- src/vma2pbs.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs index 0517251..f469053 100644 --- a/src/vma2pbs.rs +++ b/src/vma2pbs.rs @@ -6,6 +6,8 @@ use std::fs::File; use std::io::{stdin, BufRead, BufReader, Read}; use std::process::{Command, Stdio}; use std::ptr; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Arc; use std::time::SystemTime; use anyhow::{anyhow, bail, Error}; @@ -234,6 +236,8 @@ where non_zero_mask: u64, } + let chunk_stats = Arc::new([const { AtomicU64::new(0) }; VMA_MAX_DEVICES]); + let images_chunks: RefCell<HashMap<u8, HashMap<u64, ImageChunk>>> = RefCell::new(HashMap::new()); @@ -284,6 +288,11 @@ where pbs_chunk_offset, pbs_chunk_offset + pbs_chunk_size, ); + let chunk_stat = chunk_stats[dev_id as usize].fetch_add(1, Ordering::SeqCst); + if (chunk_stat % 1000) == 0 { + let percentage = 100 * PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE * chunk_stat / device_size; + log::info!("\tUploading dev_id: {} ({}%)", dev_id, percentage); + } let mut pbs_err: *mut c_char = ptr::null_mut(); -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs v5 4/4] log device upload progress as a percentage 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 4/4] log device upload progress as a percentage Filip Schauer @ 2024-11-13 11:41 ` Shannon Sterz 2024-11-13 16:07 ` Filip Schauer 0 siblings, 1 reply; 12+ messages in thread From: Shannon Sterz @ 2024-11-13 11:41 UTC (permalink / raw) To: Proxmox Backup Server development discussion comments in-line: On Mon Nov 11, 2024 at 2:08 PM CET, Filip Schauer wrote: > Log the upload progress of a device as a percentage with log level info > every 1000 chunks. > > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > src/vma2pbs.rs | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs > index 0517251..f469053 100644 > --- a/src/vma2pbs.rs > +++ b/src/vma2pbs.rs > @@ -6,6 +6,8 @@ use std::fs::File; > use std::io::{stdin, BufRead, BufReader, Read}; > use std::process::{Command, Stdio}; > use std::ptr; > +use std::sync::atomic::{AtomicU64, Ordering}; > +use std::sync::Arc; > use std::time::SystemTime; > > use anyhow::{anyhow, bail, Error}; > @@ -234,6 +236,8 @@ where > non_zero_mask: u64, > } > > + let chunk_stats = Arc::new([const { AtomicU64::new(0) }; VMA_MAX_DEVICES]); > + > let images_chunks: RefCell<HashMap<u8, HashMap<u64, ImageChunk>>> = > RefCell::new(HashMap::new()); > > @@ -284,6 +288,11 @@ where > pbs_chunk_offset, > pbs_chunk_offset + pbs_chunk_size, > ); > + let chunk_stat = chunk_stats[dev_id as usize].fetch_add(1, Ordering::SeqCst); > + if (chunk_stat % 1000) == 0 { > + let percentage = 100 * PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE * chunk_stat / device_size; > + log::info!("\tUploading dev_id: {} ({}%)", dev_id, percentage); nit: format string > + } > > let mut pbs_err: *mut c_char = ptr::null_mut(); > Other than the nits across the four patches, consider this series: Reviewed-by: Shannon Sterz <s.sterz@proxmox.com> _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs v5 4/4] log device upload progress as a percentage 2024-11-13 11:41 ` Shannon Sterz @ 2024-11-13 16:07 ` Filip Schauer 0 siblings, 0 replies; 12+ messages in thread From: Filip Schauer @ 2024-11-13 16:07 UTC (permalink / raw) To: pbs-devel On 13/11/2024 12:41, Shannon Sterz wrote: > Other than the nits across the four patches, consider this series: > > Reviewed-by: Shannon Sterz<s.sterz@proxmox.com> Thanks for the review! The nits are addressed in v6. Superseded by: https://lists.proxmox.com/pipermail/pbs-devel/2024-November/011465.html _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-11-13 16:07 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-11-11 13:08 [pbs-devel] [PATCH vma-to-pbs v5 0/4] add support for bulk import of a dump directory Filip Schauer 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 1/4] " Filip Schauer 2024-11-13 11:41 ` Shannon Sterz 2024-11-13 16:02 ` Filip Schauer 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 2/4] add option to skip vmids whose backups failed to upload Filip Schauer 2024-11-13 11:41 ` Shannon Sterz 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println Filip Schauer 2024-11-13 11:41 ` Shannon Sterz 2024-11-13 16:02 ` Filip Schauer 2024-11-11 13:08 ` [pbs-devel] [PATCH vma-to-pbs v5 4/4] log device upload progress as a percentage Filip Schauer 2024-11-13 11:41 ` Shannon Sterz 2024-11-13 16:07 ` Filip Schauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox