[pbs-devel] [PATCH proxmox-backup v3 1/2] api2/tape/backup: include a summary on notification e-mails

[pbs-devel] [PATCH proxmox-backup v3 1/2] api2/tape/backup: include a summary on notification e-mails

[Dominik Csapak]

for now only contains the list of included snapshots (if any),
as well as the backup duration

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v2:
* move TapeBackupJobSummary to email_notifications module
  (where we need it)
* rebase on master
 src/api2/tape/backup.rs           | 32 +++++++++++++++++++++++++------
 src/server/email_notifications.rs | 21 ++++++++++++++++++++
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index e4fba59c..4da22b81 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -33,6 +33,7 @@ use crate::{
     },
     server::{
         lookup_user_email,
+        TapeBackupJobSummary,
         jobstate::{
             Job,
             JobState,
@@ -198,13 +199,16 @@ pub fn do_tape_backup_job(
             let notify_user = setup.notify_user.as_ref().unwrap_or_else(|| &Userid::root_userid());
             let email = lookup_user_email(notify_user);
 
-            let job_result = backup_worker(
+            let (job_result, summary) = match backup_worker(
                 &worker,
                 datastore,
                 &pool_config,
                 &setup,
                 email.clone(),
-            );
+            ) {
+                Ok(summary) => (Ok(()), summary),
+                Err(err) => (Err(err), Default::default()),
+            };
 
             let status = worker.create_state(&job_result);
 
@@ -214,6 +218,7 @@ pub fn do_tape_backup_job(
                     Some(job.jobname()),
                     &setup,
                     &job_result,
+                    summary,
                 ) {
                     eprintln!("send tape backup notification failed: {}", err);
                 }
@@ -340,13 +345,17 @@ pub fn backup(
         move |worker| {
             let _drive_lock = drive_lock; // keep lock guard
             set_tape_device_state(&setup.drive, &worker.upid().to_string())?;
-            let job_result = backup_worker(
+
+            let (job_result, summary) = match backup_worker(
                 &worker,
                 datastore,
                 &pool_config,
                 &setup,
                 email.clone(),
-            );
+            ) {
+                Ok(summary) => (Ok(()), summary),
+                Err(err) => (Err(err), Default::default()),
+            };
 
             if let Some(email) = email {
                 if let Err(err) = crate::server::send_tape_backup_status(
@@ -354,6 +363,7 @@ pub fn backup(
                     None,
                     &setup,
                     &job_result,
+                    summary,
                 ) {
                     eprintln!("send tape backup notification failed: {}", err);
                 }
@@ -374,9 +384,11 @@ fn backup_worker(
     pool_config: &MediaPoolConfig,
     setup: &TapeBackupJobSetup,
     email: Option<String>,
-) -> Result<(), Error> {
+) -> Result<TapeBackupJobSummary, Error> {
 
     let status_path = Path::new(TAPE_STATUS_DIR);
+    let start = std::time::Instant::now();
+    let mut summary: TapeBackupJobSummary = Default::default();
 
     let _lock = MediaPool::lock(status_path, &pool_config.name)?;
 
@@ -422,8 +434,11 @@ fn backup_worker(
                     task_log!(worker, "skip snapshot {}", info.backup_dir);
                     continue;
                 }
+                let snapshot_name = info.backup_dir.to_string();
                 if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
                     errors = true;
+                } else {
+                    summary.snapshot_list.push(snapshot_name);
                 }
                 progress.done_snapshots = 1;
                 task_log!(
@@ -439,8 +454,11 @@ fn backup_worker(
                     task_log!(worker, "skip snapshot {}", info.backup_dir);
                     continue;
                 }
+                let snapshot_name = info.backup_dir.to_string();
                 if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
                     errors = true;
+                } else {
+                    summary.snapshot_list.push(snapshot_name);
                 }
                 progress.done_snapshots = snapshot_number as u64 + 1;
                 task_log!(
@@ -478,7 +496,9 @@ fn backup_worker(
         bail!("Tape backup finished with some errors. Please check the task log.");
     }
 
-    Ok(())
+    summary.duration = start.elapsed();
+
+    Ok(summary)
 }
 
 // Try to update the the media online status
diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs
index e92c8091..4e07ed13 100644
--- a/src/server/email_notifications.rs
+++ b/src/server/email_notifications.rs
@@ -149,6 +149,14 @@ Datastore:  {{job.store}}
 Tape Pool:  {{job.pool}}
 Tape Drive: {{job.drive}}
 
+{{#if snapshot-list ~}}
+Snapshots included:
+
+{{#each snapshot-list~}}
+{{this}}
+{{/each~}}
+{{/if}}
+Duration: {{duration}}
 
 Tape Backup successful.
 
@@ -215,6 +223,15 @@ lazy_static::lazy_static!{
     };
 }
 
+/// Summary of a successful Tape Job
+#[derive(Default)]
+pub struct TapeBackupJobSummary {
+    /// The list of snaphots backed up
+    pub snapshot_list: Vec<String>,
+    /// The total time of the backup job
+    pub duration: std::time::Duration,
+}
+
 fn send_job_status_mail(
     email: &str,
     subject: &str,
@@ -412,14 +429,18 @@ pub fn send_tape_backup_status(
     id: Option<&str>,
     job: &TapeBackupJobSetup,
     result: &Result<(), Error>,
+    summary: TapeBackupJobSummary,
 ) -> Result<(), Error> {
 
     let (fqdn, port) = get_server_url();
+    let duration: crate::tools::systemd::time::TimeSpan = summary.duration.into();
     let mut data = json!({
         "job": job,
         "fqdn": fqdn,
         "port": port,
         "id": id,
+        "snapshot-list": summary.snapshot_list,
+        "duration": duration.to_string(),
     });
 
     let text = match result {
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup v3 2/2] api2/tape/backup: wait indefinitely for lock in scheduled backup jobs

[Dominik Csapak]

so that a user can schedule multiple backup jobs onto a single
media pool without having to consider timing them apart

this makes sense since we can backup multiple datastores onto
the same media-set but can only specify one datastore per backup job

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v2:
* rebase on master

 src/api2/tape/backup.rs | 57 +++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 4da22b81..27659298 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -5,6 +5,7 @@ use anyhow::{bail, format_err, Error};
 use serde_json::Value;
 
 use proxmox::{
+    try_block,
     api::{
         api,
         RpcEnvironment,
@@ -177,8 +178,15 @@ pub fn do_tape_backup_job(
 
     let (drive_config, _digest) = config::drive::config()?;
 
-    // early check/lock before starting worker
-    let drive_lock = lock_tape_device(&drive_config, &setup.drive)?;
+    // for scheduled jobs we acquire the lock later in the worker
+    let drive_lock = if schedule.is_some() {
+        None
+    } else {
+        Some(lock_tape_device(&drive_config, &setup.drive)?)
+    };
+
+    let notify_user = setup.notify_user.as_ref().unwrap_or_else(|| &Userid::root_userid());
+    let email = lookup_user_email(notify_user);
 
     let upid_str = WorkerTask::new_thread(
         &worker_type,
@@ -186,26 +194,37 @@ pub fn do_tape_backup_job(
         auth_id.clone(),
         false,
         move |worker| {
-            let _drive_lock = drive_lock; // keep lock guard
-
-            set_tape_device_state(&setup.drive, &worker.upid().to_string())?;
             job.start(&worker.upid().to_string())?;
+            let mut drive_lock = drive_lock;
+
+            let (job_result, summary) = match try_block!({
+                if schedule.is_some() {
+                    // for scheduled tape backup jobs, we wait indefinitely for the lock
+                    task_log!(worker, "waiting for drive lock...");
+                    loop {
+                        if let Ok(lock) = lock_tape_device(&drive_config, &setup.drive) {
+                            drive_lock = Some(lock);
+                            break;
+                        } // ignore errors
+
+                        worker.check_abort()?;
+                    }
+                }
+                set_tape_device_state(&setup.drive, &worker.upid().to_string())?;
 
-            task_log!(worker,"Starting tape backup job '{}'", job_id);
-            if let Some(event_str) = schedule {
-                task_log!(worker,"task triggered by schedule '{}'", event_str);
-            }
-
-            let notify_user = setup.notify_user.as_ref().unwrap_or_else(|| &Userid::root_userid());
-            let email = lookup_user_email(notify_user);
+                task_log!(worker,"Starting tape backup job '{}'", job_id);
+                if let Some(event_str) = schedule {
+                    task_log!(worker,"task triggered by schedule '{}'", event_str);
+                }
 
-            let (job_result, summary) = match backup_worker(
-                &worker,
-                datastore,
-                &pool_config,
-                &setup,
-                email.clone(),
-            ) {
+                backup_worker(
+                    &worker,
+                    datastore,
+                    &pool_config,
+                    &setup,
+                    email.clone(),
+                )
+            }) {
                 Ok(summary) => (Ok(()), summary),
                 Err(err) => (Err(err), Default::default()),
             };
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox-backup v3 1/2] api2/tape/backup: include a summary on notification e-mails

[Dietmar Maurer]

applied both patches