[pbs-devel] [PATCH vma-to-pbs v3 0/6] add support for bulk import of a dump directory

[pbs-devel] [PATCH vma-to-pbs v3 0/6] add support for bulk import of a dump directory

[Filip Schauer]

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 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 (6):
  split BackupVmaToPbsArgs into PbsArgs and VmaBackupArgs
  add support for bulk import of a dump directory
  add option to skip vmids whose backups failed to upload
  remove hard coded values
  use level-based logging instead of println
  log device upload progress as a percentage

 Cargo.toml     |   5 +
 src/main.rs    | 156 ++++++++++++++++++++++---
 src/vma.rs     |   2 +-
 src/vma2pbs.rs | 312 ++++++++++++++++++++++++++++++++-----------------
 4 files changed, 354 insertions(+), 121 deletions(-)

-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH vma-to-pbs v3 1/6] split BackupVmaToPbsArgs into PbsArgs and VmaBackupArgs

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/main.rs    |  17 +++--
 src/vma2pbs.rs | 199 ++++++++++++++++++++++++++++---------------------
 2 files changed, 126 insertions(+), 90 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index de789c1..3e25591 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -6,7 +6,7 @@ use proxmox_time::epoch_i64;
 
 mod vma;
 mod vma2pbs;
-use vma2pbs::{backup_vma_to_pbs, BackupVmaToPbsArgs};
+use vma2pbs::{vma2pbs, BackupVmaToPbsArgs, PbsArgs, VmaBackupArgs};
 
 const CMD_HELP: &str = "\
 Usage: vma-to-pbs [OPTIONS] --repository <auth_id@host:port:datastore> --vmid <VMID> [vma_file]
@@ -184,12 +184,9 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> {
         None
     };
 
-    let options = BackupVmaToPbsArgs {
-        vma_file_path: vma_file_path.cloned(),
+    let pbs_args = PbsArgs {
         pbs_repository,
         namespace,
-        backup_id: vmid,
-        backup_time,
         pbs_password,
         keyfile,
         key_password,
@@ -197,16 +194,24 @@ fn parse_args() -> Result<BackupVmaToPbsArgs, Error> {
         fingerprint,
         compress,
         encrypt,
+    };
+
+    let vma_args = VmaBackupArgs {
+        vma_file_path: vma_file_path.cloned(),
+        backup_id: vmid,
+        backup_time,
         notes,
         log_file_path,
     };
 
+    let options = BackupVmaToPbsArgs { pbs_args, vma_args };
+
     Ok(options)
 }
 
 fn main() -> Result<(), Error> {
     let args = parse_args()?;
-    backup_vma_to_pbs(args)?;
+    vma2pbs(args)?;
 
     Ok(())
 }
diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs
index d2ce437..37ea308 100644
--- a/src/vma2pbs.rs
+++ b/src/vma2pbs.rs
@@ -29,11 +29,13 @@ use crate::vma::VmaReader;
 const VMA_CLUSTER_SIZE: usize = 65536;
 
 pub struct BackupVmaToPbsArgs {
-    pub vma_file_path: Option<OsString>,
+    pub pbs_args: PbsArgs,
+    pub vma_args: VmaBackupArgs,
+}
+
+pub struct PbsArgs {
     pub pbs_repository: String,
     pub namespace: Option<String>,
-    pub backup_id: String,
-    pub backup_time: i64,
     pub pbs_password: String,
     pub keyfile: Option<String>,
     pub key_password: Option<String>,
@@ -41,6 +43,12 @@ pub struct BackupVmaToPbsArgs {
     pub fingerprint: String,
     pub compress: bool,
     pub encrypt: bool,
+}
+
+pub struct VmaBackupArgs {
+    pub vma_file_path: Option<OsString>,
+    pub backup_id: String,
+    pub backup_time: i64,
     pub notes: Option<String>,
     pub log_file_path: Option<OsString>,
 }
@@ -61,25 +69,23 @@ fn handle_pbs_error(pbs_err: *mut c_char, function_name: &str) -> Result<(), Err
     bail!("{function_name} failed: {pbs_err_str}");
 }
 
-fn create_pbs_backup_task(args: &BackupVmaToPbsArgs) -> Result<*mut ProxmoxBackupHandle, Error> {
-    println!("PBS repository: {}", args.pbs_repository);
-    if let Some(ns) = &args.namespace {
-        println!("PBS namespace: {}", ns);
-    }
-    println!("PBS fingerprint: {}", args.fingerprint);
-    println!("compress: {}", args.compress);
-    println!("encrypt: {}", args.encrypt);
-
-    println!("backup time: {}", epoch_to_rfc3339(args.backup_time)?);
+fn create_pbs_backup_task(
+    pbs_args: &PbsArgs,
+    backup_args: &VmaBackupArgs,
+) -> Result<*mut ProxmoxBackupHandle, Error> {
+    println!(
+        "backup time: {}",
+        epoch_to_rfc3339(backup_args.backup_time)?
+    );
 
     let mut pbs_err: *mut c_char = ptr::null_mut();
 
-    let pbs_repository_cstr = CString::new(args.pbs_repository.as_str())?;
-    let ns_cstr = CString::new(args.namespace.as_deref().unwrap_or(""))?;
-    let backup_id_cstr = CString::new(args.backup_id.as_str())?;
-    let pbs_password_cstr = CString::new(args.pbs_password.as_str())?;
-    let fingerprint_cstr = CString::new(args.fingerprint.as_str())?;
-    let keyfile_cstr = args
+    let pbs_repository_cstr = CString::new(pbs_args.pbs_repository.as_str())?;
+    let ns_cstr = CString::new(pbs_args.namespace.as_deref().unwrap_or(""))?;
+    let backup_id_cstr = CString::new(backup_args.backup_id.as_str())?;
+    let pbs_password_cstr = CString::new(pbs_args.pbs_password.as_str())?;
+    let fingerprint_cstr = CString::new(pbs_args.fingerprint.as_str())?;
+    let keyfile_cstr = pbs_args
         .keyfile
         .as_ref()
         .map(|v| CString::new(v.as_str()).unwrap());
@@ -87,7 +93,7 @@ fn create_pbs_backup_task(args: &BackupVmaToPbsArgs) -> Result<*mut ProxmoxBacku
         .as_ref()
         .map(|v| v.as_ptr())
         .unwrap_or(ptr::null());
-    let key_password_cstr = args
+    let key_password_cstr = pbs_args
         .key_password
         .as_ref()
         .map(|v| CString::new(v.as_str()).unwrap());
@@ -95,7 +101,7 @@ fn create_pbs_backup_task(args: &BackupVmaToPbsArgs) -> Result<*mut ProxmoxBacku
         .as_ref()
         .map(|v| v.as_ptr())
         .unwrap_or(ptr::null());
-    let master_keyfile_cstr = args
+    let master_keyfile_cstr = pbs_args
         .master_keyfile
         .as_ref()
         .map(|v| CString::new(v.as_str()).unwrap());
@@ -108,14 +114,14 @@ fn create_pbs_backup_task(args: &BackupVmaToPbsArgs) -> Result<*mut ProxmoxBacku
         pbs_repository_cstr.as_ptr(),
         ns_cstr.as_ptr(),
         backup_id_cstr.as_ptr(),
-        args.backup_time as u64,
+        backup_args.backup_time as u64,
         PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE,
         pbs_password_cstr.as_ptr(),
         keyfile_ptr,
         key_password_ptr,
         master_keyfile_ptr,
-        args.compress,
-        args.encrypt,
+        pbs_args.compress,
+        pbs_args.encrypt,
         fingerprint_cstr.as_ptr(),
         &mut pbs_err,
     );
@@ -361,17 +367,24 @@ where
     Ok(())
 }
 
-fn pbs_client_setup(args: &BackupVmaToPbsArgs) -> Result<(HttpClient, String, Value), Error> {
-    let repo: BackupRepository = args.pbs_repository.parse()?;
+fn pbs_client_setup(
+    pbs_args: &PbsArgs,
+    backup_args: &VmaBackupArgs,
+) -> Result<(HttpClient, String, Value), Error> {
+    let repo: BackupRepository = pbs_args.pbs_repository.parse()?;
     let options = HttpClientOptions::new_interactive(
-        Some(args.pbs_password.clone()),
-        Some(args.fingerprint.clone()),
+        Some(pbs_args.pbs_password.clone()),
+        Some(pbs_args.fingerprint.clone()),
     );
     let client = HttpClient::new(repo.host(), repo.port(), repo.auth_id(), options)?;
 
-    let backup_dir = BackupDir::from((BackupType::Vm, args.backup_id.clone(), args.backup_time));
+    let backup_dir = BackupDir::from((
+        BackupType::Vm,
+        backup_args.backup_id.clone(),
+        backup_args.backup_time,
+    ));
 
-    let namespace = match &args.namespace {
+    let namespace = match &pbs_args.namespace {
         Some(namespace) => BackupNamespace::new(namespace)?,
         None => BackupNamespace::root(),
     };
@@ -386,45 +399,44 @@ fn pbs_client_setup(args: &BackupVmaToPbsArgs) -> Result<(HttpClient, String, Va
 
 fn upload_log(
     client: &HttpClient,
-    args: &BackupVmaToPbsArgs,
+    log_file_path: &OsString,
+    pbs_args: &PbsArgs,
     store: &str,
     request_args: Value,
 ) -> Result<(), Error> {
-    if let Some(log_file_path) = &args.log_file_path {
-        let path = format!("api2/json/admin/datastore/{}/upload-backup-log", store);
-        let data = std::fs::read(log_file_path)?;
-
-        let blob = if args.encrypt {
-            let crypt_config = match &args.keyfile {
-                None => None,
-                Some(keyfile) => {
-                    let key = std::fs::read(keyfile)?;
-                    let (key, _created, _) = decrypt_key(&key, &|| -> Result<Vec<u8>, Error> {
-                        match &args.key_password {
-                            Some(key_password) => Ok(key_password.clone().into_bytes()),
-                            None => bail!("no key password provided"),
-                        }
-                    })?;
-                    let crypt_config = CryptConfig::new(key)?;
-                    Some(crypt_config)
-                }
-            };
-
-            DataBlob::encode(&data, crypt_config.as_ref(), args.compress)?
-        } else {
-            // fixme: howto sign log?
-            DataBlob::encode(&data, None, args.compress)?
+    let path = format!("api2/json/admin/datastore/{}/upload-backup-log", store);
+    let data = std::fs::read(log_file_path)?;
+
+    let blob = if pbs_args.encrypt {
+        let crypt_config = match &pbs_args.keyfile {
+            None => None,
+            Some(keyfile) => {
+                let key = std::fs::read(keyfile)?;
+                let (key, _created, _) = decrypt_key(&key, &|| -> Result<Vec<u8>, Error> {
+                    match &pbs_args.key_password {
+                        Some(key_password) => Ok(key_password.clone().into_bytes()),
+                        None => bail!("no key password provided"),
+                    }
+                })?;
+                let crypt_config = CryptConfig::new(key)?;
+                Some(crypt_config)
+            }
         };
 
-        let body = hyper::Body::from(blob.into_inner());
+        DataBlob::encode(&data, crypt_config.as_ref(), pbs_args.compress)?
+    } else {
+        // fixme: howto sign log?
+        DataBlob::encode(&data, None, pbs_args.compress)?
+    };
 
-        block_on(async {
-            client
-                .upload("application/octet-stream", body, &path, Some(request_args))
-                .await
-                .unwrap();
-        });
-    }
+    let body = hyper::Body::from(blob.into_inner());
+
+    block_on(async {
+        client
+            .upload("application/octet-stream", body, &path, Some(request_args))
+            .await
+            .unwrap();
+    });
 
     Ok(())
 }
@@ -444,8 +456,32 @@ fn set_notes(
     Ok(())
 }
 
-pub fn backup_vma_to_pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> {
-    let vma_file: Box<dyn BufRead> = match &args.vma_file_path {
+pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> {
+    let pbs_args = &args.pbs_args;
+    println!("PBS repository: {}", pbs_args.pbs_repository);
+    if let Some(ns) = &pbs_args.namespace {
+        println!("PBS namespace: {}", ns);
+    }
+    println!("PBS fingerprint: {}", pbs_args.fingerprint);
+    println!("compress: {}", pbs_args.compress);
+    println!("encrypt: {}", pbs_args.encrypt);
+
+    let start_transfer_time = SystemTime::now();
+
+    upload_vma_file(pbs_args, &args.vma_args)?;
+
+    let transfer_duration = SystemTime::now().duration_since(start_transfer_time)?;
+    let total_seconds = transfer_duration.as_secs();
+    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");
+
+    Ok(())
+}
+
+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)),
@@ -454,7 +490,7 @@ pub fn backup_vma_to_pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> {
     };
     let vma_reader = VmaReader::new(vma_file)?;
 
-    let pbs = create_pbs_backup_task(&args)?;
+    let pbs = create_pbs_backup_task(pbs_args, backup_args)?;
 
     defer! {
         proxmox_backup_disconnect(pbs);
@@ -467,10 +503,6 @@ pub fn backup_vma_to_pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> {
         handle_pbs_error(pbs_err, "proxmox_backup_connect")?;
     }
 
-    println!("Connected to Proxmox Backup Server");
-
-    let start_transfer_time = SystemTime::now();
-
     upload_configs(&vma_reader, pbs)?;
     upload_block_devices(vma_reader, pbs)?;
 
@@ -478,24 +510,23 @@ pub fn backup_vma_to_pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> {
         handle_pbs_error(pbs_err, "proxmox_backup_finish")?;
     }
 
-    if args.notes.is_some() || args.log_file_path.is_some() {
-        let (client, store, request_args) = pbs_client_setup(&args)?;
-
-        if args.log_file_path.is_some() {
-            upload_log(&client, &args, &store, request_args.clone())?;
+    if backup_args.notes.is_some() || backup_args.log_file_path.is_some() {
+        let (client, store, request_args) = pbs_client_setup(pbs_args, backup_args)?;
+
+        if let Some(log_file_path) = &backup_args.log_file_path {
+            upload_log(
+                &client,
+                log_file_path,
+                pbs_args,
+                &store,
+                request_args.clone(),
+            )?;
         }
 
-        if let Some(notes) = args.notes {
-            set_notes(&client, &notes, &store, request_args)?;
+        if let Some(notes) = &backup_args.notes {
+            set_notes(&client, notes, &store, request_args)?;
         }
     }
 
-    let transfer_duration = SystemTime::now().duration_since(start_transfer_time)?;
-    let total_seconds = transfer_duration.as_secs();
-    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");
-
     Ok(())
 }
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH vma-to-pbs v3 2/6] add support for bulk import of a dump directory

[Filip Schauer]

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     |   3 ++
 src/main.rs    | 120 ++++++++++++++++++++++++++++++++++++++++++++-----
 src/vma2pbs.rs |  64 +++++++++++++++++++++++---
 3 files changed, 169 insertions(+), 18 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index cd13426..5c6a175 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -7,9 +7,12 @@ edition = "2021"
 [dependencies]
 anyhow = "1.0"
 bincode = "1.3"
+chrono = "0.4"
 hyper = "0.14.5"
+itertools = "0.13"
 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..83c59f6 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,26 +1,33 @@
 use std::ffi::OsString;
+use std::fs::read_dir;
+use std::path::PathBuf;
 
 use anyhow::{bail, Context, Error};
+use chrono::NaiveDateTime;
+use itertools::Itertools;
 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
+          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>
@@ -87,7 +94,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 = 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")?;
@@ -196,15 +203,106 @@ 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 mut grouped_vmas = Vec::new();
+
+    let bulk = if let Some(vma_file_path) = vma_file_path {
+        PathBuf::from(vma_file_path).is_dir()
+    } else {
+        false
     };
 
-    let options = BackupVmaToPbsArgs { pbs_args, vma_args };
+    if bulk {
+        let dump_dir_path =
+            PathBuf::from(vma_file_path.expect("no directory specified for bulk import"));
+
+        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),
+                    _ => continue,
+                };
+
+                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);
+        grouped_vmas = vmas
+            .into_iter()
+            .into_group_map_by(|d| d.backup_id.clone())
+            .into_values()
+            .collect();
+        grouped_vmas.sort_by_key(|d| d[0].backup_id.clone());
+    } else if let Some(vmid) = vmid {
+        let vmas = vec![VmaBackupArgs {
+            vma_file_path: vma_file_path.cloned(),
+            compression: None,
+            backup_id: vmid,
+            backup_time,
+            notes,
+            log_file_path,
+        }];
+        grouped_vmas.push(vmas);
+    } else {
+        bail!("No vmid specified for single backup file");
+    }
+
+    let options = BackupVmaToPbsArgs {
+        pbs_args,
+        grouped_vmas,
+    };
 
     Ok(options)
 }
diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs
index 37ea308..05294f3 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: Vec<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>,
@@ -468,7 +476,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.expect("missing VMA file path"),
+                    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();
@@ -481,13 +501,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

[pbs-devel] [PATCH vma-to-pbs v3 3/6] add option to skip vmids whose backups failed to upload

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/main.rs    | 15 ++++++++++++++-
 src/vma2pbs.rs | 15 +++++++++++----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index 83c59f6..2ebab16 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -48,6 +48,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.
   -h, --help
           Print help
   -V, --version
@@ -59,7 +62,15 @@ 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",
+        "--skip-failed",
+    ];
 
     for (i, arg) in args.iter().enumerate() {
         if let Some(arg) = arg.to_str() {
@@ -106,6 +117,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");
 
     match (encrypt, keyfile.is_some()) {
         (true, false) => bail!("--encrypt requires a --keyfile!"),
@@ -302,6 +314,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 05294f3..ef9d770 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: Vec<Vec<VmaBackupArgs>>,
+    pub skip_failed: bool,
 }
 
 pub struct PbsArgs {
@@ -479,13 +480,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.expect("missing VMA file path"),
+                    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

[pbs-devel] [PATCH vma-to-pbs v3 4/6] remove hard coded values

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/vma.rs     |  2 +-
 src/vma2pbs.rs | 23 +++++++++++------------
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/src/vma.rs b/src/vma.rs
index 518de8a..63ee3b5 100644
--- a/src/vma.rs
+++ b/src/vma.rs
@@ -22,7 +22,7 @@ const VMA_MAX_CONFIGS: usize = 256;
 
 /// Maximum number of block devices
 /// See VMA Header in pve-qemu.git/vma_spec.txt
-const VMA_MAX_DEVICES: usize = 256;
+pub const VMA_MAX_DEVICES: usize = 256;
 
 /// VMA magic string
 /// See VMA Header in pve-qemu.git/vma_spec.txt
diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs
index ef9d770..497f3ae 100644
--- a/src/vma2pbs.rs
+++ b/src/vma2pbs.rs
@@ -25,7 +25,7 @@ use proxmox_time::epoch_to_rfc3339;
 use scopeguard::defer;
 use serde_json::Value;
 
-use crate::vma::VmaReader;
+use crate::vma::{VmaReader, VMA_MAX_DEVICES};
 
 const VMA_CLUSTER_SIZE: usize = 65536;
 
@@ -174,20 +174,21 @@ where
 fn register_block_devices<T>(
     vma_reader: &VmaReader<T>,
     pbs: *mut ProxmoxBackupHandle,
-) -> Result<[Option<BlockDeviceInfo>; 256], Error>
+) -> Result<[Option<BlockDeviceInfo>; VMA_MAX_DEVICES], Error>
 where
     T: Read,
 {
-    let mut block_device_infos: [Option<BlockDeviceInfo>; 256] = [None; 256];
+    let mut block_device_infos: [Option<BlockDeviceInfo>; VMA_MAX_DEVICES] =
+        [None; VMA_MAX_DEVICES];
     let mut pbs_err: *mut c_char = ptr::null_mut();
 
-    for device_id in 0..255 {
-        if !vma_reader.contains_device(device_id) {
+    for (device_id, block_device_info) in block_device_infos.iter_mut().enumerate() {
+        if !vma_reader.contains_device(device_id.try_into()?) {
             continue;
         }
 
-        let device_name = vma_reader.get_device_name(device_id)?;
-        let device_size = vma_reader.get_device_size(device_id)?;
+        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: {}",
@@ -207,12 +208,10 @@ where
             handle_pbs_error(pbs_err, "proxmox_backup_register_image")?;
         }
 
-        let block_device_info = BlockDeviceInfo {
+        *block_device_info = Some(BlockDeviceInfo {
             pbs_device_id: pbs_device_id as u8,
             device_size,
-        };
-
-        block_device_infos[device_id as usize] = Some(block_device_info);
+        });
     }
 
     Ok(block_device_infos)
@@ -360,7 +359,7 @@ where
 
     let mut pbs_err: *mut c_char = ptr::null_mut();
 
-    for block_device_info in block_device_infos.iter().take(255) {
+    for block_device_info in block_device_infos {
         let block_device_info = match block_device_info {
             Some(block_device_info) => block_device_info,
             None => continue,
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH vma-to-pbs v3 5/6] use level-based logging instead of println

[Filip Schauer]

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    | 16 ++++++++++++++--
 src/vma2pbs.rs | 38 ++++++++++++++++++++------------------
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 5c6a175..0f4b2a6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -8,8 +8,10 @@ edition = "2021"
 anyhow = "1.0"
 bincode = "1.3"
 chrono = "0.4"
+env_logger = "0.10"
 hyper = "0.14.5"
 itertools = "0.13"
+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 2ebab16..07ada83 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -4,6 +4,7 @@ use std::path::PathBuf;
 
 use anyhow::{bail, Context, Error};
 use chrono::NaiveDateTime;
+use env_logger::Target;
 use itertools::Itertools;
 use proxmox_sys::linux::tty;
 use proxmox_time::epoch_i64;
@@ -121,7 +122,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!"
         ),
         _ => {}
@@ -183,7 +184,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."
             );
@@ -320,7 +321,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 497f3ae..a5907ed 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

[pbs-devel] [PATCH vma-to-pbs v3 6/6] log device upload progress as a percentage

[Filip Schauer]

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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs
index a5907ed..d8bdccf 100644
--- a/src/vma2pbs.rs
+++ b/src/vma2pbs.rs
@@ -6,6 +6,7 @@ use std::fs::File;
 use std::io::{stdin, BufRead, BufReader, Read};
 use std::process::{Command, Stdio};
 use std::ptr;
+use std::sync::{Arc, Mutex};
 use std::time::SystemTime;
 
 use anyhow::{anyhow, bail, Error};
@@ -234,6 +235,8 @@ where
         non_zero_mask: u64,
     }
 
+    let chunk_stats = Arc::new(Mutex::new([0u64; VMA_MAX_DEVICES]));
+
     let images_chunks: RefCell<HashMap<u8, HashMap<u64, ImageChunk>>> =
         RefCell::new(HashMap::new());
 
@@ -284,6 +287,14 @@ where
                 pbs_chunk_offset,
                 pbs_chunk_offset + pbs_chunk_size,
             );
+            let mut chunk_stats_locked = chunk_stats.lock().unwrap();
+            chunk_stats_locked[dev_id as usize] += 1;
+            if (chunk_stats_locked[dev_id as usize] % 1000) == 0 {
+                let percentage =
+                    100 * PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE * chunk_stats_locked[dev_id as usize]
+                        / 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

Re: [pbs-devel] [PATCH vma-to-pbs v3 6/6] log device upload progress as a percentage

[Fabian Grünbichler]

Quoting Filip Schauer (2024-10-22 16:28:43)
> 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 | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs
> index a5907ed..d8bdccf 100644
> --- a/src/vma2pbs.rs
> +++ b/src/vma2pbs.rs
> @@ -6,6 +6,7 @@ use std::fs::File;
>  use std::io::{stdin, BufRead, BufReader, Read};
>  use std::process::{Command, Stdio};
>  use std::ptr;
> +use std::sync::{Arc, Mutex};
>  use std::time::SystemTime;
>  
>  use anyhow::{anyhow, bail, Error};
> @@ -234,6 +235,8 @@ where
>          non_zero_mask: u64,
>      }
>  
> +    let chunk_stats = Arc::new(Mutex::new([0u64; VMA_MAX_DEVICES]));

Arc::new([AtomicUsize::new(0); VMA_MAX_DEVICES])

> +
>      let images_chunks: RefCell<HashMap<u8, HashMap<u64, ImageChunk>>> =
>          RefCell::new(HashMap::new());
>  
> @@ -284,6 +287,14 @@ where
>                  pbs_chunk_offset,
>                  pbs_chunk_offset + pbs_chunk_size,
>              );
> +            let mut chunk_stats_locked = chunk_stats.lock().unwrap();
> +            chunk_stats_locked[dev_id as usize] += 1;

then instead of this, just do a fetch_add, which returns the previous value

> +            if (chunk_stats_locked[dev_id as usize] % 1000) == 0 {
> +                let percentage =
> +                    100 * PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE * chunk_stats_locked[dev_id as usize]

which can then be used here, since without the Mutex, we don't have a guarantee
that the current value will not be incremented from current % 1000 == 999 to
current % 1000 == 1 before someone has a chance to hit this printing code ;)

> +                        / 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
> 
>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH vma-to-pbs v3 2/6] add support for bulk import of a dump directory

[Fabian Grünbichler]

Quoting Filip Schauer (2024-10-22 16:28:39)
> 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     |   3 ++
>  src/main.rs    | 120 ++++++++++++++++++++++++++++++++++++++++++++-----
>  src/vma2pbs.rs |  64 +++++++++++++++++++++++---
>  3 files changed, 169 insertions(+), 18 deletions(-)
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index cd13426..5c6a175 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -7,9 +7,12 @@ edition = "2021"
>  [dependencies]
>  anyhow = "1.0"
>  bincode = "1.3"
> +chrono = "0.4"
>  hyper = "0.14.5"
> +itertools = "0.13"
>  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..83c59f6 100644
> --- a/src/main.rs
> +++ b/src/main.rs
> @@ -1,26 +1,33 @@
>  use std::ffi::OsString;
> +use std::fs::read_dir;
> +use std::path::PathBuf;
>  
>  use anyhow::{bail, Context, Error};
> +use chrono::NaiveDateTime;
> +use itertools::Itertools;
>  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
> +          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.

this is missing the previous case that is still there - if a vma_file is given, then vmid is required.

>        [--backup-time <EPOCH>]
>            Backup timestamp
>        --fingerprint <FINGERPRINT>
> @@ -87,7 +94,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 = 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")?;
> @@ -196,15 +203,106 @@ 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 mut grouped_vmas = Vec::new();

I think this would be better off as a map? that would also allow constructing
it in place below..

> +
> +    let bulk = if let Some(vma_file_path) = vma_file_path {
> +        PathBuf::from(vma_file_path).is_dir()
> +    } else {
> +        false
>      };

this is vma_file_path.is_some_and(|path| PathBuf::from(path).is_dir()) , but see below

>  
> -    let options = BackupVmaToPbsArgs { pbs_args, vma_args };
> +    if bulk {

since bulk is only used here,

> +        let dump_dir_path =
> +            PathBuf::from(vma_file_path.expect("no directory specified for bulk import"));

and this does another conversion to a PathBuf, this could just be

    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(

although I am not sure whether the possible cases here
- optional vmid, vma_file_path is set and a dir (the new bulk import)
- vmid is set, optional vma_file_path (the old single file or stdin import)
- no vmid set, vma_file_path not set or not a dir (invalid combination of options)

are not an argument for splitting single file and bulk import into separate
commands, or adding an explicit --bulk bool argument that would force a user to
opt into bulk importing?

> +
> +        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()) {

if we find a file that is not valid UTF-8, should we warn about it?

> +                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;

should we log these as well?

> +                    }
> +                }
> +
> +                let compression = match ext {
> +                    "" => None,
> +                    ".zst" => Some(Compression::Zstd),
> +                    ".lzo" => Some(Compression::Lzo),
> +                    ".gz" => Some(Compression::GZip),
> +                    _ => continue,

and these?

> +                };
> +
> +                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
> +                };

and finally here all that are found and will be processed? ;)

> +
> +                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);
> +        grouped_vmas = vmas
> +            .into_iter()
> +            .into_group_map_by(|d| d.backup_id.clone())
> +            .into_values()
> +            .collect();
> +        grouped_vmas.sort_by_key(|d| d[0].backup_id.clone());

or here, if we just want to print a summary for the bulk imports like:

Found backup archives: {total_count}
Summary:
- VMID XX: {count}
- VMID XY: {count}
- ..

could even ask for confirmation before starting the bulk import?

> +    } else if let Some(vmid) = vmid {
> +        let vmas = vec![VmaBackupArgs {
> +            vma_file_path: vma_file_path.cloned(),
> +            compression: None,
> +            backup_id: vmid,
> +            backup_time,
> +            notes,
> +            log_file_path,
> +        }];
> +        grouped_vmas.push(vmas);
> +    } else {
> +        bail!("No vmid specified for single backup file");
> +    }
> +
> +    let options = BackupVmaToPbsArgs {
> +        pbs_args,
> +        grouped_vmas,
> +    };
>  
>      Ok(options)
>  }
> diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs
> index 37ea308..05294f3 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: Vec<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>,
> @@ -468,7 +476,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.expect("missing VMA file path"),

if this is None it's a backup from stdin, and shouldn't print this confusing
error message? (ah, that happens in the next patch, but should be folded in
here ;))

> +                    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();
> @@ -481,13 +501,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))

did you test whether with_capacity and a higher buffer size than 8K improves performance?

> +        }
> +        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)),

same here

> +            },
> +            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
> 
>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH vma-to-pbs v3 0/6] add support for bulk import of a dump directory

[Fabian Grünbichler]

this is shaping up nicely, some comments on individual patches.

I just bumped proxmox-backup-qemu and vma-to-pbs to use PBS 3.2.8-1, please
rebase on top of that for subsequent versions.

Quoting Filip Schauer (2024-10-22 16:28:37)
> 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 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 (6):
>   split BackupVmaToPbsArgs into PbsArgs and VmaBackupArgs
>   add support for bulk import of a dump directory
>   add option to skip vmids whose backups failed to upload
>   remove hard coded values
>   use level-based logging instead of println
>   log device upload progress as a percentage
> 
>  Cargo.toml     |   5 +
>  src/main.rs    | 156 ++++++++++++++++++++++---
>  src/vma.rs     |   2 +-
>  src/vma2pbs.rs | 312 ++++++++++++++++++++++++++++++++-----------------
>  4 files changed, 354 insertions(+), 121 deletions(-)
> 
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH vma-to-pbs v3 2/6] add support for bulk import of a dump directory

[Filip Schauer]

On 29/10/2024 11:41, Fabian Grünbichler wrote:
>> +        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()) {
> if we find a file that is not valid UTF-8, should we warn about it?


Any file name that contains invalid UTF-8 will not get matched by the
regex anyway and v4 of this patch adds a debug message logging any file
that does not match the regex.


On 29/10/2024 11:41, Fabian Grünbichler wrote:
>> +            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))
> did you test whether with_capacity and a higher buffer size than 8K improves performance?
>
>> +        }
>> +        None => match &backup_args.vma_file_path {
>> +            Some(vma_file_path) => matchFile::open(vma_file_path)  {
>> +                Err(why) => return Err(anyhow!("Couldn't open file: {}", why)),
>> +                Ok(file) => Box::new(BufReader::new(file)),
> same here


Tested buffer sizes from 1024 up to 1048576 and it made no measurable
performance difference.



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH vma-to-pbs v3 0/6] add support for bulk import of a dump directory

[Filip Schauer]

Superseded by:
https://lists.proxmox.com/pipermail/pbs-devel/2024-October/011247.html

On 29/10/2024 11:51, Fabian Grünbichler wrote:
> this is shaping up nicely, some comments on individual patches.
>
> I just bumped proxmox-backup-qemu and vma-to-pbs to use PBS 3.2.8-1, please
> rebase on top of that for subsequent versions.
>
> Quoting Filip Schauer (2024-10-22 16:28:37)
>> 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 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 (6):
>>    split BackupVmaToPbsArgs into PbsArgs and VmaBackupArgs
>>    add support for bulk import of a dump directory
>>    add option to skip vmids whose backups failed to upload
>>    remove hard coded values
>>    use level-based logging instead of println
>>    log device upload progress as a percentage
>>
>>   Cargo.toml     |   5 +
>>   src/main.rs    | 156 ++++++++++++++++++++++---
>>   src/vma.rs     |   2 +-
>>   src/vma2pbs.rs | 312 ++++++++++++++++++++++++++++++++-----------------
>>   4 files changed, 354 insertions(+), 121 deletions(-)
>>
>> -- 
>> 2.39.5
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH vma-to-pbs v3 2/6] add support for bulk import of a dump directory

[Fabian Grünbichler]

> Filip Schauer <f.schauer@proxmox.com> hat am 30.10.2024 14:59 CET geschrieben:
> 
>  
> On 29/10/2024 11:41, Fabian Grünbichler wrote:
> >> +        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()) {
> > if we find a file that is not valid UTF-8, should we warn about it?
> 
> 
> Any file name that contains invalid UTF-8 will not get matched by the
> regex anyway and v4 of this patch adds a debug message logging any file
> that does not match the regex.
> 
> 
> On 29/10/2024 11:41, Fabian Grünbichler wrote:
> >> +            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))
> > did you test whether with_capacity and a higher buffer size than 8K improves performance?

just realized that there is a pipe before it anyhow which likely has a 64k buffer..

> >
> >> +        }
> >> +        None => match &backup_args.vma_file_path {
> >> +            Some(vma_file_path) => matchFile::open(vma_file_path)  {
> >> +                Err(why) => return Err(anyhow!("Couldn't open file: {}", why)),
> >> +                Ok(file) => Box::new(BufReader::new(file)),
> > same here
> 
> 
> Tested buffer sizes from 1024 up to 1048576 and it made no measurable
> performance difference.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel