all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup 0/5] allow restoring catalogs during inventory
@ 2022-05-19 10:08 Dominik Csapak
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 1/5] tape/inventory: make 'load_media_db' a method Dominik Csapak
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Dominik Csapak @ 2022-05-19 10:08 UTC (permalink / raw)
  To: pbs-devel

This series allows automatically restoring the catalog from tape during
inventory, by adding a new 'catalog' parameter.

This is useful after a disaster, since it allows the admin to re-import
all tapes in an automatic way.

The first two patches are just cleanups, so they could be applied
independently.

Dominik Csapak (5):
  tape/inventory: make 'load_media_db' a method
  tape: replace '&Path' with 'AsRef<Path>' in function parameters
  api/tape/inventory: optionally try to restore catalogs
  proxmox-tape: add 'catalog' option to 'proxmox-tape inventory'
  ui: tape/ChangerStatus: adding parameter selection to inventory

 src/api2/tape/backup.rs               |  11 +--
 src/api2/tape/changer.rs              |   4 +-
 src/api2/tape/drive.rs                | 121 +++++++++++++++-----------
 src/api2/tape/media.rs                |  34 +++-----
 src/api2/tape/restore.rs              |  27 +++---
 src/bin/proxmox-tape.rs               |  16 +++-
 src/tape/changer/online_status_map.rs |   4 +-
 src/tape/inventory.rs                 |  47 +++++-----
 src/tape/media_catalog.rs             |  52 ++++++-----
 src/tape/media_catalog_cache.rs       |  12 +--
 src/tape/media_pool.rs                |  16 ++--
 src/tape/pool_writer/mod.rs           |  16 ++--
 www/tape/ChangerStatus.js             |  66 +++++++-------
 13 files changed, 214 insertions(+), 212 deletions(-)

-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 1/5] tape/inventory: make 'load_media_db' a method
  2022-05-19 10:08 [pbs-devel] [PATCH proxmox-backup 0/5] allow restoring catalogs during inventory Dominik Csapak
@ 2022-05-19 10:08 ` Dominik Csapak
  2022-10-05 17:32   ` [pbs-devel] applied: " Thomas Lamprecht
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 2/5] tape: replace '&Path' with 'AsRef<Path>' in function parameters Dominik Csapak
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Dominik Csapak @ 2022-05-19 10:08 UTC (permalink / raw)
  To: pbs-devel

and use self.inventory_path. This is only used internally (not pub) so there
is no need to have it as a static function.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/inventory.rs | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs
index c3bd4606..c270ffcf 100644
--- a/src/tape/inventory.rs
+++ b/src/tape/inventory.rs
@@ -112,7 +112,7 @@ impl Inventory {
 
     /// Reload the database
     pub fn reload(&mut self) -> Result<(), Error> {
-        self.map = Self::load_media_db(&self.inventory_path)?;
+        self.map = self.load_media_db()?;
         self.update_helpers();
         Ok(())
     }
@@ -140,8 +140,8 @@ impl Inventory {
         open_backup_lockfile(&self.lockfile_path, None, true)
     }
 
-    fn load_media_db(path: &Path) -> Result<BTreeMap<Uuid, MediaStateEntry>, Error> {
-        let data = file_get_json(path, Some(json!([])))?;
+    fn load_media_db(&self) -> Result<BTreeMap<Uuid, MediaStateEntry>, Error> {
+        let data = file_get_json(&self.inventory_path, Some(json!([])))?;
         let media_list: Vec<MediaStateEntry> = serde_json::from_value(data)?;
 
         let mut map = BTreeMap::new();
@@ -177,7 +177,7 @@ impl Inventory {
     /// Stores a single MediaID persistently
     pub fn store(&mut self, mut media_id: MediaId, clear_media_status: bool) -> Result<(), Error> {
         let _lock = self.lock()?;
-        self.map = Self::load_media_db(&self.inventory_path)?;
+        self.map = self.load_media_db()?;
 
         let uuid = media_id.label.uuid.clone();
 
@@ -217,7 +217,7 @@ impl Inventory {
     /// Remove a single media persistently
     pub fn remove_media(&mut self, uuid: &Uuid) -> Result<(), Error> {
         let _lock = self.lock()?;
-        self.map = Self::load_media_db(&self.inventory_path)?;
+        self.map = self.load_media_db()?;
         self.map.remove(uuid);
         self.update_helpers();
         self.replace_file()?;
@@ -659,7 +659,7 @@ impl Inventory {
     // Lock database, reload database, set status, store database
     fn set_media_status(&mut self, uuid: &Uuid, status: Option<MediaStatus>) -> Result<(), Error> {
         let _lock = self.lock()?;
-        self.map = Self::load_media_db(&self.inventory_path)?;
+        self.map = self.load_media_db()?;
         if let Some(entry) = self.map.get_mut(uuid) {
             entry.status = status;
             self.update_helpers();
@@ -697,7 +697,7 @@ impl Inventory {
         location: Option<MediaLocation>,
     ) -> Result<(), Error> {
         let _lock = self.lock()?;
-        self.map = Self::load_media_db(&self.inventory_path)?;
+        self.map = self.load_media_db()?;
         if let Some(entry) = self.map.get_mut(uuid) {
             entry.location = location;
             self.update_helpers();
@@ -721,7 +721,7 @@ impl Inventory {
     /// Update online status
     pub fn update_online_status(&mut self, online_map: &OnlineStatusMap) -> Result<(), Error> {
         let _lock = self.lock()?;
-        self.map = Self::load_media_db(&self.inventory_path)?;
+        self.map = self.load_media_db()?;
 
         for (uuid, entry) in self.map.iter_mut() {
             if let Some(changer_name) = online_map.lookup_changer(uuid) {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 2/5] tape: replace '&Path' with 'AsRef<Path>' in function parameters
  2022-05-19 10:08 [pbs-devel] [PATCH proxmox-backup 0/5] allow restoring catalogs during inventory Dominik Csapak
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 1/5] tape/inventory: make 'load_media_db' a method Dominik Csapak
@ 2022-05-19 10:08 ` Dominik Csapak
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 3/5] api/tape/inventory: optionally try to restore catalogs Dominik Csapak
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2022-05-19 10:08 UTC (permalink / raw)
  To: pbs-devel

this way we can omit the pattern
```
let status_path = Path::new(TAPE_STATUS_DIR);
some_function(status_path);
```
and give the TAPE_STATUS_DIR directly. In some instances we now have to
give TAPE_STATUS_DIR more often, but most often we save a few
intermediary Paths.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/backup.rs               | 11 ++--
 src/api2/tape/changer.rs              |  4 +-
 src/api2/tape/drive.rs                | 80 +++++++++++----------------
 src/api2/tape/media.rs                | 34 +++++-------
 src/api2/tape/restore.rs              | 27 ++++-----
 src/tape/changer/online_status_map.rs |  4 +-
 src/tape/inventory.rs                 | 31 +++++------
 src/tape/media_catalog.rs             | 52 +++++++++--------
 src/tape/media_catalog_cache.rs       | 12 ++--
 src/tape/media_pool.rs                | 16 +++---
 src/tape/pool_writer/mod.rs           | 16 ++----
 11 files changed, 129 insertions(+), 158 deletions(-)

diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index c7d042fb..3c5d2af0 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -1,4 +1,3 @@
-use std::path::Path;
 use std::sync::{Arc, Mutex};
 
 use anyhow::{bail, format_err, Error};
@@ -97,7 +96,6 @@ pub fn list_tape_backup_jobs(
         });
 
     let mut list = Vec::new();
-    let status_path = Path::new(TAPE_STATUS_DIR);
     let current_time = proxmox_time::epoch_i64();
 
     for job in job_list_iter {
@@ -120,7 +118,8 @@ pub fn list_tape_backup_jobs(
             if let Ok(Some((_, name))) = media_changer(&drive_config, &job.setup.drive) {
                 changer_name = Some(name);
             }
-            if let Ok(mut pool) = MediaPool::with_config(status_path, &pool, changer_name, true) {
+            if let Ok(mut pool) = MediaPool::with_config(TAPE_STATUS_DIR, &pool, changer_name, true)
+            {
                 if pool.start_write_session(next_run, false).is_ok() {
                     if let Ok(media_id) = pool.guess_next_writable_media(next_run) {
                         next_media_label = Some(media_id.label.label_text);
@@ -398,7 +397,6 @@ fn backup_worker(
     summary: &mut TapeBackupJobSummary,
     force_media_set: bool,
 ) -> Result<(), Error> {
-    let status_path = Path::new(TAPE_STATUS_DIR);
     let start = std::time::Instant::now();
 
     task_log!(worker, "update media online status");
@@ -407,7 +405,7 @@ fn backup_worker(
     let root_namespace = setup.ns.clone().unwrap_or_default();
     let ns_magic = !root_namespace.is_root() || setup.max_depth != Some(0);
 
-    let pool = MediaPool::with_config(status_path, pool_config, changer_name, false)?;
+    let pool = MediaPool::with_config(TAPE_STATUS_DIR, pool_config, changer_name, false)?;
 
     let mut pool_writer =
         PoolWriter::new(pool, &setup.drive, worker, email, force_media_set, ns_magic)?;
@@ -578,8 +576,7 @@ fn update_media_online_status(drive: &str) -> Result<Option<String>, Error> {
     if let Ok(Some((mut changer, changer_name))) = media_changer(&config, drive) {
         let label_text_list = changer.online_media_label_texts()?;
 
-        let status_path = Path::new(TAPE_STATUS_DIR);
-        let mut inventory = Inventory::load(status_path)?;
+        let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
         update_changer_online_status(&config, &mut inventory, &changer_name, &label_text_list)?;
 
diff --git a/src/api2/tape/changer.rs b/src/api2/tape/changer.rs
index 54fab87f..7ecf7bff 100644
--- a/src/api2/tape/changer.rs
+++ b/src/api2/tape/changer.rs
@@ -1,5 +1,4 @@
 use std::collections::HashMap;
-use std::path::Path;
 
 use anyhow::Error;
 use serde_json::Value;
@@ -55,8 +54,7 @@ pub async fn get_status(name: String, cache: bool) -> Result<Vec<MtxStatusEntry>
 
     let status = tokio::task::spawn_blocking(move || changer_config.status(cache)).await??;
 
-    let state_path = Path::new(TAPE_STATUS_DIR);
-    let mut inventory = Inventory::load(state_path)?;
+    let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     let mut map = OnlineStatusMap::new(&config)?;
     let online_set = mtx_status_to_online_set(&status, &inventory);
diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index b4fe60d0..28c0f0f5 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -1,6 +1,5 @@
 use std::collections::HashMap;
 use std::panic::UnwindSafe;
-use std::path::Path;
 use std::sync::Arc;
 
 use anyhow::{bail, format_err, Error};
@@ -345,20 +344,19 @@ pub fn format_media(
                         media_id.label.uuid,
                     );
 
-                    let status_path = Path::new(TAPE_STATUS_DIR);
-                    let mut inventory = Inventory::new(status_path);
+                    let mut inventory = Inventory::new(TAPE_STATUS_DIR);
 
                     if let Some(MediaSetLabel {
                         ref pool, ref uuid, ..
                     }) = media_id.media_set_label
                     {
-                        let _pool_lock = lock_media_pool(status_path, pool)?;
-                        let _media_set_lock = lock_media_set(status_path, uuid, None)?;
-                        MediaCatalog::destroy(status_path, &media_id.label.uuid)?;
+                        let _pool_lock = lock_media_pool(TAPE_STATUS_DIR, pool)?;
+                        let _media_set_lock = lock_media_set(TAPE_STATUS_DIR, uuid, None)?;
+                        MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
                         inventory.remove_media(&media_id.label.uuid)?;
                     } else {
-                        let _lock = lock_unassigned_media_pool(status_path)?;
-                        MediaCatalog::destroy(status_path, &media_id.label.uuid)?;
+                        let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?;
+                        MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
                         inventory.remove_media(&media_id.label.uuid)?;
                     };
 
@@ -522,9 +520,6 @@ fn write_media_label(
     pool: Option<String>,
 ) -> Result<(), Error> {
     drive.label_tape(&label)?;
-
-    let status_path = Path::new(TAPE_STATUS_DIR);
-
     let media_id = if let Some(ref pool) = pool {
         // assign media to pool by writing special media set label
         task_log!(
@@ -543,9 +538,9 @@ fn write_media_label(
         };
 
         // Create the media catalog
-        MediaCatalog::overwrite(status_path, &media_id, false)?;
+        MediaCatalog::overwrite(TAPE_STATUS_DIR, &media_id, false)?;
 
-        let mut inventory = Inventory::new(status_path);
+        let mut inventory = Inventory::new(TAPE_STATUS_DIR);
         inventory.store(media_id.clone(), false)?;
 
         media_id
@@ -562,9 +557,9 @@ fn write_media_label(
         };
 
         // Create the media catalog
-        MediaCatalog::overwrite(status_path, &media_id, false)?;
+        MediaCatalog::overwrite(TAPE_STATUS_DIR, &media_id, false)?;
 
-        let mut inventory = Inventory::new(status_path);
+        let mut inventory = Inventory::new(TAPE_STATUS_DIR);
         inventory.store(media_id.clone(), false)?;
 
         media_id
@@ -698,20 +693,19 @@ pub async fn read_label(drive: String, inventorize: Option<bool>) -> Result<Medi
         };
 
         if let Some(true) = inventorize {
-            let state_path = Path::new(TAPE_STATUS_DIR);
-            let mut inventory = Inventory::new(state_path);
+            let mut inventory = Inventory::new(TAPE_STATUS_DIR);
 
             if let Some(MediaSetLabel {
                 ref pool, ref uuid, ..
             }) = media_id.media_set_label
             {
-                let _pool_lock = lock_media_pool(state_path, pool)?;
-                let _lock = lock_media_set(state_path, uuid, None)?;
-                MediaCatalog::destroy_unrelated_catalog(state_path, &media_id)?;
+                let _pool_lock = lock_media_pool(TAPE_STATUS_DIR, pool)?;
+                let _lock = lock_media_set(TAPE_STATUS_DIR, uuid, None)?;
+                MediaCatalog::destroy_unrelated_catalog(TAPE_STATUS_DIR, &media_id)?;
                 inventory.store(media_id, false)?;
             } else {
-                let _lock = lock_unassigned_media_pool(state_path)?;
-                MediaCatalog::destroy(state_path, &media_id.label.uuid)?;
+                let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?;
+                MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
                 inventory.store(media_id, false)?;
             };
         }
@@ -813,9 +807,7 @@ pub async fn inventory(drive: String) -> Result<Vec<LabelUuidMap>, Error> {
 
         let label_text_list = changer.online_media_label_texts()?;
 
-        let state_path = Path::new(TAPE_STATUS_DIR);
-
-        let mut inventory = Inventory::load(state_path)?;
+        let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
         update_changer_online_status(&config, &mut inventory, &changer_name, &label_text_list)?;
 
@@ -894,9 +886,7 @@ pub fn update_inventory(
                 task_log!(worker, "changer device does not list any media labels");
             }
 
-            let state_path = Path::new(TAPE_STATUS_DIR);
-
-            let mut inventory = Inventory::load(state_path)?;
+            let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
             update_changer_online_status(&config, &mut inventory, &changer_name, &label_text_list)?;
 
@@ -954,13 +944,13 @@ pub fn update_inventory(
                             ref pool, ref uuid, ..
                         }) = media_id.media_set_label
                         {
-                            let _pool_lock = lock_media_pool(state_path, pool)?;
-                            let _lock = lock_media_set(state_path, uuid, None)?;
-                            MediaCatalog::destroy_unrelated_catalog(state_path, &media_id)?;
+                            let _pool_lock = lock_media_pool(TAPE_STATUS_DIR, pool)?;
+                            let _lock = lock_media_set(TAPE_STATUS_DIR, uuid, None)?;
+                            MediaCatalog::destroy_unrelated_catalog(TAPE_STATUS_DIR, &media_id)?;
                             inventory.store(media_id, false)?;
                         } else {
-                            let _lock = lock_unassigned_media_pool(state_path)?;
-                            MediaCatalog::destroy(state_path, &media_id.label.uuid)?;
+                            let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?;
+                            MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
                             inventory.store(media_id, false)?;
                         };
                     }
@@ -1031,9 +1021,7 @@ fn barcode_label_media_worker(
     // make sure we label them in the right order
     label_text_list.sort();
 
-    let state_path = Path::new(TAPE_STATUS_DIR);
-
-    let mut inventory = Inventory::load(state_path)?;
+    let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     update_changer_online_status(
         drive_config,
@@ -1275,15 +1263,13 @@ pub fn catalog_media(
                 (None, _) => bail!("media is empty (no media label found)"),
             };
 
-            let status_path = Path::new(TAPE_STATUS_DIR);
-
-            let mut inventory = Inventory::new(status_path);
+            let mut inventory = Inventory::new(TAPE_STATUS_DIR);
 
             let (_media_set_lock, media_set_uuid) = match media_id.media_set_label {
                 None => {
                     task_log!(worker, "media is empty");
-                    let _lock = lock_unassigned_media_pool(status_path)?;
-                    MediaCatalog::destroy(status_path, &media_id.label.uuid)?;
+                    let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?;
+                    MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
                     inventory.store(media_id.clone(), false)?;
                     return Ok(());
                 }
@@ -1291,8 +1277,8 @@ pub fn catalog_media(
                     if set.uuid.as_ref() == [0u8; 16] {
                         // media is empty
                         task_log!(worker, "media is empty");
-                        let _lock = lock_unassigned_media_pool(status_path)?;
-                        MediaCatalog::destroy(status_path, &media_id.label.uuid)?;
+                        let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?;
+                        MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
                         inventory.store(media_id.clone(), false)?;
                         return Ok(());
                     }
@@ -1303,10 +1289,10 @@ pub fn catalog_media(
 
                     drive.set_encryption(encrypt_fingerprint)?;
 
-                    let _pool_lock = lock_media_pool(status_path, &set.pool)?;
-                    let media_set_lock = lock_media_set(status_path, &set.uuid, None)?;
+                    let _pool_lock = lock_media_pool(TAPE_STATUS_DIR, &set.pool)?;
+                    let media_set_lock = lock_media_set(TAPE_STATUS_DIR, &set.uuid, None)?;
 
-                    MediaCatalog::destroy_unrelated_catalog(status_path, &media_id)?;
+                    MediaCatalog::destroy_unrelated_catalog(TAPE_STATUS_DIR, &media_id)?;
 
                     inventory.store(media_id.clone(), false)?;
 
@@ -1314,7 +1300,7 @@ pub fn catalog_media(
                 }
             };
 
-            if MediaCatalog::exists(status_path, &media_id.label.uuid) && !force {
+            if MediaCatalog::exists(TAPE_STATUS_DIR, &media_id.label.uuid) && !force {
                 bail!("media catalog exists (please use --force to overwrite)");
             }
 
diff --git a/src/api2/tape/media.rs b/src/api2/tape/media.rs
index c6f6b52e..82fb47d6 100644
--- a/src/api2/tape/media.rs
+++ b/src/api2/tape/media.rs
@@ -1,5 +1,4 @@
 use std::collections::HashSet;
-use std::path::Path;
 
 use anyhow::{bail, format_err, Error};
 
@@ -41,8 +40,6 @@ pub async fn list_media_sets(
 
     let (config, _digest) = pbs_config::media_pool::config()?;
 
-    let status_path = Path::new(TAPE_STATUS_DIR);
-
     let mut media_sets: HashSet<Uuid> = HashSet::new();
     let mut list = Vec::new();
 
@@ -60,7 +57,7 @@ pub async fn list_media_sets(
         let config: MediaPoolConfig = config.lookup("pool", pool_name)?;
 
         let changer_name = None; // assume standalone drive
-        let pool = MediaPool::with_config(status_path, &config, changer_name, true)?;
+        let pool = MediaPool::with_config(TAPE_STATUS_DIR, &config, changer_name, true)?;
 
         for media in pool.list_media() {
             if let Some(label) = media.media_set_label() {
@@ -130,18 +127,18 @@ pub async fn list_media(
 
     let (config, _digest) = pbs_config::media_pool::config()?;
 
-    let status_path = Path::new(TAPE_STATUS_DIR);
-
     let catalogs = tokio::task::spawn_blocking(move || {
         if update_status {
             // update online media status
-            if let Err(err) = update_online_status(status_path, update_status_changer.as_deref()) {
+            if let Err(err) =
+                update_online_status(TAPE_STATUS_DIR, update_status_changer.as_deref())
+            {
                 eprintln!("{}", err);
                 eprintln!("update online media status failed - using old state");
             }
         }
         // test what catalog files we have
-        MediaCatalog::media_with_catalogs(status_path)
+        MediaCatalog::media_with_catalogs(TAPE_STATUS_DIR)
     })
     .await??;
 
@@ -166,7 +163,7 @@ pub async fn list_media(
         let config: MediaPoolConfig = config.lookup("pool", pool_name)?;
 
         let changer_name = None; // assume standalone drive
-        let mut pool = MediaPool::with_config(status_path, &config, changer_name, true)?;
+        let mut pool = MediaPool::with_config(TAPE_STATUS_DIR, &config, changer_name, true)?;
 
         let current_time = proxmox_time::epoch_i64();
 
@@ -211,7 +208,7 @@ pub async fn list_media(
         }
     }
 
-    let inventory = Inventory::load(status_path)?;
+    let inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     let privs = user_info.lookup_privs(&auth_id, &["tape", "pool"]);
     if (privs & PRIV_TAPE_AUDIT) != 0 {
@@ -297,8 +294,7 @@ pub async fn list_media(
 )]
 /// Change Tape location to vault (if given), or offline.
 pub fn move_tape(label_text: String, vault_name: Option<String>) -> Result<(), Error> {
-    let status_path = Path::new(TAPE_STATUS_DIR);
-    let mut inventory = Inventory::load(status_path)?;
+    let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     let uuid = inventory
         .find_media_by_label_text(&label_text)
@@ -334,8 +330,7 @@ pub fn move_tape(label_text: String, vault_name: Option<String>) -> Result<(), E
 pub fn destroy_media(label_text: String, force: Option<bool>) -> Result<(), Error> {
     let force = force.unwrap_or(false);
 
-    let status_path = Path::new(TAPE_STATUS_DIR);
-    let mut inventory = Inventory::load(status_path)?;
+    let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     let media_id = inventory
         .find_media_by_label_text(&label_text)
@@ -391,8 +386,7 @@ pub fn list_content(
 
     let (config, _digest) = pbs_config::media_pool::config()?;
 
-    let status_path = Path::new(TAPE_STATUS_DIR);
-    let inventory = Inventory::load(status_path)?;
+    let inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     let mut list = Vec::new();
 
@@ -437,7 +431,7 @@ pub fn list_content(
             .generate_media_set_name(&set.uuid, template)
             .unwrap_or_else(|_| set.uuid.to_string());
 
-        for (store, snapshot) in media_catalog_snapshot_list(status_path, &media_id)? {
+        for (store, snapshot) in media_catalog_snapshot_list(TAPE_STATUS_DIR, &media_id)? {
             let (_, backup_dir) = pbs_api_types::parse_ns_and_snapshot(&snapshot)?;
 
             if let Some(backup_type) = filter.backup_type {
@@ -480,8 +474,7 @@ pub fn list_content(
 )]
 /// Get current media status
 pub fn get_media_status(uuid: Uuid) -> Result<MediaStatus, Error> {
-    let status_path = Path::new(TAPE_STATUS_DIR);
-    let inventory = Inventory::load(status_path)?;
+    let inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     let (status, _location) = inventory.status_and_location(&uuid);
 
@@ -506,8 +499,7 @@ pub fn get_media_status(uuid: Uuid) -> Result<MediaStatus, Error> {
 /// It is not allowed to set status to 'writable' or 'unknown' (those
 /// are internally managed states).
 pub fn update_media_status(uuid: Uuid, status: Option<MediaStatus>) -> Result<(), Error> {
-    let status_path = Path::new(TAPE_STATUS_DIR);
-    let mut inventory = Inventory::load(status_path)?;
+    let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     match status {
         None => inventory.clear_media_status(&uuid)?,
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 2b1bfc24..2624915b 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -370,11 +370,9 @@ pub fn restore(
 
     let media_set_uuid = media_set.parse()?;
 
-    let status_path = Path::new(TAPE_STATUS_DIR);
+    let _lock = lock_media_set(TAPE_STATUS_DIR, &media_set_uuid, None)?;
 
-    let _lock = lock_media_set(status_path, &media_set_uuid, None)?;
-
-    let inventory = Inventory::load(status_path)?;
+    let inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
     let pool = inventory.lookup_media_set_pool(&media_set_uuid)?;
 
@@ -995,8 +993,6 @@ fn get_media_set_catalog(
     inventory: &Inventory,
     media_set_uuid: &Uuid,
 ) -> Result<MediaSetCatalog, Error> {
-    let status_path = Path::new(TAPE_STATUS_DIR);
-
     let members = inventory.compute_media_set_members(media_set_uuid)?;
     let media_list = members.media_list();
     let mut catalog = MediaSetCatalog::new();
@@ -1012,7 +1008,7 @@ fn get_media_set_catalog(
             }
             Some(media_uuid) => {
                 let media_id = inventory.lookup_media(media_uuid).unwrap();
-                let media_catalog = MediaCatalog::open(status_path, media_id, false, false)?;
+                let media_catalog = MediaCatalog::open(TAPE_STATUS_DIR, media_id, false, false)?;
                 catalog.append_catalog(media_catalog)?;
             }
         }
@@ -1373,8 +1369,7 @@ pub fn restore_media(
     verbose: bool,
     auth_id: &Authid,
 ) -> Result<(), Error> {
-    let status_path = Path::new(TAPE_STATUS_DIR);
-    let mut catalog = MediaCatalog::create_temporary_database(status_path, media_id, false)?;
+    let mut catalog = MediaCatalog::create_temporary_database(TAPE_STATUS_DIR, media_id, false)?;
 
     loop {
         let current_file_number = drive.current_file_number()?;
@@ -1411,7 +1406,7 @@ pub fn restore_media(
 
     catalog.commit()?;
 
-    MediaCatalog::finish_temporary_database(status_path, &media_id.label.uuid, true)?;
+    MediaCatalog::finish_temporary_database(TAPE_STATUS_DIR, &media_id.label.uuid, true)?;
 
     Ok(())
 }
@@ -1899,8 +1894,6 @@ pub fn fast_catalog_restore(
     media_set: &MediaSet,
     uuid: &Uuid, // current media Uuid
 ) -> Result<bool, Error> {
-    let status_path = Path::new(TAPE_STATUS_DIR);
-
     let current_file_number = drive.current_file_number()?;
     if current_file_number != 2 {
         bail!("fast_catalog_restore: wrong media position - internal error");
@@ -1986,7 +1979,7 @@ pub fn fast_catalog_restore(
                     // always restore and overwrite catalog
                 } else {
                     // only restore if catalog does not exist
-                    if MediaCatalog::exists(status_path, catalog_uuid) {
+                    if MediaCatalog::exists(TAPE_STATUS_DIR, catalog_uuid) {
                         task_log!(
                             worker,
                             "catalog for media '{}' already exists",
@@ -1998,7 +1991,7 @@ pub fn fast_catalog_restore(
                 }
 
                 let mut file =
-                    MediaCatalog::create_temporary_database_file(status_path, catalog_uuid)?;
+                    MediaCatalog::create_temporary_database_file(TAPE_STATUS_DIR, catalog_uuid)?;
 
                 std::io::copy(&mut reader, &mut file)?;
 
@@ -2023,7 +2016,11 @@ pub fn fast_catalog_restore(
                             continue;
                         }
 
-                        MediaCatalog::finish_temporary_database(status_path, &media_uuid, true)?;
+                        MediaCatalog::finish_temporary_database(
+                            TAPE_STATUS_DIR,
+                            &media_uuid,
+                            true,
+                        )?;
 
                         if catalog_uuid == uuid {
                             task_log!(worker, "successfully restored catalog");
diff --git a/src/tape/changer/online_status_map.rs b/src/tape/changer/online_status_map.rs
index a8c2e5e6..de0233e2 100644
--- a/src/tape/changer/online_status_map.rs
+++ b/src/tape/changer/online_status_map.rs
@@ -119,8 +119,8 @@ pub fn mtx_status_to_online_set(status: &MtxStatus, inventory: &Inventory) -> Ha
 /// Update online media status
 ///
 /// For a single 'changer', or else simply ask all changer devices.
-pub fn update_online_status(
-    state_path: &Path,
+pub fn update_online_status<P: AsRef<Path>>(
+    state_path: P,
     changer: Option<&str>,
 ) -> Result<OnlineStatusMap, Error> {
     let (config, _digest) = pbs_config::drive::config()?;
diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs
index c270ffcf..82e2e93f 100644
--- a/src/tape/inventory.rs
+++ b/src/tape/inventory.rs
@@ -89,11 +89,11 @@ impl Inventory {
     pub const MEDIA_INVENTORY_LOCKFILE: &'static str = ".inventory.lck";
 
     /// Create empty instance, no data loaded
-    pub fn new(base_path: &Path) -> Self {
-        let mut inventory_path = base_path.to_owned();
+    pub fn new<P: AsRef<Path>>(base_path: P) -> Self {
+        let mut inventory_path = base_path.as_ref().to_owned();
         inventory_path.push(Self::MEDIA_INVENTORY_FILENAME);
 
-        let mut lockfile_path = base_path.to_owned();
+        let mut lockfile_path = base_path.as_ref().to_owned();
         lockfile_path.push(Self::MEDIA_INVENTORY_LOCKFILE);
 
         Self {
@@ -104,7 +104,7 @@ impl Inventory {
         }
     }
 
-    pub fn load(base_path: &Path) -> Result<Self, Error> {
+    pub fn load<P: AsRef<Path>>(base_path: P) -> Result<Self, Error> {
         let mut me = Self::new(base_path);
         me.reload()?;
         Ok(me)
@@ -751,8 +751,8 @@ impl Inventory {
 }
 
 /// Lock a media pool
-pub fn lock_media_pool(base_path: &Path, name: &str) -> Result<BackupLockGuard, Error> {
-    let mut path = base_path.to_owned();
+pub fn lock_media_pool<P: AsRef<Path>>(base_path: P, name: &str) -> Result<BackupLockGuard, Error> {
+    let mut path = base_path.as_ref().to_owned();
     path.push(format!(".pool-{}", name));
     path.set_extension("lck");
 
@@ -760,7 +760,7 @@ pub fn lock_media_pool(base_path: &Path, name: &str) -> Result<BackupLockGuard,
 }
 
 /// Lock for media not assigned to any pool
-pub fn lock_unassigned_media_pool(base_path: &Path) -> Result<BackupLockGuard, Error> {
+pub fn lock_unassigned_media_pool<P: AsRef<Path>>(base_path: P) -> Result<BackupLockGuard, Error> {
     // lock artificial "__UNASSIGNED__" pool to avoid races
     lock_media_pool(base_path, "__UNASSIGNED__")
 }
@@ -768,12 +768,12 @@ pub fn lock_unassigned_media_pool(base_path: &Path) -> Result<BackupLockGuard, E
 /// Lock a media set
 ///
 /// Timeout is 10 seconds by default
-pub fn lock_media_set(
-    base_path: &Path,
+pub fn lock_media_set<P: AsRef<Path>>(
+    base_path: P,
     media_set_uuid: &Uuid,
     timeout: Option<Duration>,
 ) -> Result<BackupLockGuard, Error> {
-    let mut path = base_path.to_owned();
+    let mut path = base_path.as_ref().to_owned();
     path.push(format!(".media-set-{}", media_set_uuid));
     path.set_extension("lck");
 
@@ -784,7 +784,7 @@ pub fn lock_media_set(
 
 /// List of known media uuids
 pub fn complete_media_uuid(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
-    let inventory = match Inventory::load(Path::new(TAPE_STATUS_DIR)) {
+    let inventory = match Inventory::load(TAPE_STATUS_DIR) {
         Ok(inventory) => inventory,
         Err(_) => return Vec::new(),
     };
@@ -794,7 +794,7 @@ pub fn complete_media_uuid(_arg: &str, _param: &HashMap<String, String>) -> Vec<
 
 /// List of known media sets
 pub fn complete_media_set_uuid(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
-    let inventory = match Inventory::load(Path::new(TAPE_STATUS_DIR)) {
+    let inventory = match Inventory::load(TAPE_STATUS_DIR) {
         Ok(inventory) => inventory,
         Err(_) => return Vec::new(),
     };
@@ -809,7 +809,7 @@ pub fn complete_media_set_uuid(_arg: &str, _param: &HashMap<String, String>) ->
 
 /// List of known media labels (barcodes)
 pub fn complete_media_label_text(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
-    let inventory = match Inventory::load(Path::new(TAPE_STATUS_DIR)) {
+    let inventory = match Inventory::load(TAPE_STATUS_DIR) {
         Ok(inventory) => inventory,
         Err(_) => return Vec::new(),
     };
@@ -826,8 +826,7 @@ pub fn complete_media_set_snapshots(_arg: &str, param: &HashMap<String, String>)
         Some(uuid) => uuid,
         None => return Vec::new(),
     };
-    let status_path = Path::new(TAPE_STATUS_DIR);
-    let inventory = match Inventory::load(status_path) {
+    let inventory = match Inventory::load(TAPE_STATUS_DIR) {
         Ok(inventory) => inventory,
         Err(_) => return Vec::new(),
     };
@@ -843,7 +842,7 @@ pub fn complete_media_set_snapshots(_arg: &str, param: &HashMap<String, String>)
             });
 
     for media_id in media_ids {
-        let catalog = match MediaCatalog::open(status_path, &media_id, false, false) {
+        let catalog = match MediaCatalog::open(TAPE_STATUS_DIR, &media_id, false, false) {
             Ok(catalog) => catalog,
             Err(_) => continue,
         };
diff --git a/src/tape/media_catalog.rs b/src/tape/media_catalog.rs
index 85128f80..1ec5da37 100644
--- a/src/tape/media_catalog.rs
+++ b/src/tape/media_catalog.rs
@@ -65,10 +65,10 @@ impl MediaCatalog {
         [76, 142, 232, 193, 32, 168, 137, 113];
 
     /// List media with catalogs
-    pub fn media_with_catalogs(base_path: &Path) -> Result<HashSet<Uuid>, Error> {
+    pub fn media_with_catalogs<P: AsRef<Path>>(base_path: P) -> Result<HashSet<Uuid>, Error> {
         let mut catalogs = HashSet::new();
 
-        for entry in read_subdir(libc::AT_FDCWD, base_path)? {
+        for entry in read_subdir(libc::AT_FDCWD, base_path.as_ref())? {
             let entry = entry?;
             let name = unsafe { entry.file_name_utf8_unchecked() };
             if !name.ends_with(".log") {
@@ -82,27 +82,27 @@ impl MediaCatalog {
         Ok(catalogs)
     }
 
-    pub fn catalog_path(base_path: &Path, uuid: &Uuid) -> PathBuf {
-        let mut path = base_path.to_owned();
+    fn catalog_path<P: AsRef<Path>>(base_path: P, uuid: &Uuid) -> PathBuf {
+        let mut path = base_path.as_ref().to_owned();
         path.push(uuid.to_string());
         path.set_extension("log");
         path
     }
 
-    fn tmp_catalog_path(base_path: &Path, uuid: &Uuid) -> PathBuf {
-        let mut path = base_path.to_owned();
+    fn tmp_catalog_path<P: AsRef<Path>>(base_path: P, uuid: &Uuid) -> PathBuf {
+        let mut path = base_path.as_ref().to_owned();
         path.push(uuid.to_string());
         path.set_extension("tmp");
         path
     }
 
     /// Test if a catalog exists
-    pub fn exists(base_path: &Path, uuid: &Uuid) -> bool {
+    pub fn exists<P: AsRef<Path>>(base_path: P, uuid: &Uuid) -> bool {
         Self::catalog_path(base_path, uuid).exists()
     }
 
     /// Destroy the media catalog (remove all files)
-    pub fn destroy(base_path: &Path, uuid: &Uuid) -> Result<(), Error> {
+    pub fn destroy<P: AsRef<Path>>(base_path: P, uuid: &Uuid) -> Result<(), Error> {
         let path = Self::catalog_path(base_path, uuid);
 
         match std::fs::remove_file(path) {
@@ -113,7 +113,10 @@ impl MediaCatalog {
     }
 
     /// Destroy the media catalog if media_set uuid does not match
-    pub fn destroy_unrelated_catalog(base_path: &Path, media_id: &MediaId) -> Result<(), Error> {
+    pub fn destroy_unrelated_catalog<P: AsRef<Path>>(
+        base_path: P,
+        media_id: &MediaId,
+    ) -> Result<(), Error> {
         let uuid = &media_id.label.uuid;
 
         let path = Self::catalog_path(base_path, uuid);
@@ -164,7 +167,7 @@ impl MediaCatalog {
         self.log_to_stdout = enable;
     }
 
-    fn create_basedir(base_path: &Path) -> Result<(), Error> {
+    fn create_basedir<P: AsRef<Path>>(base_path: P) -> Result<(), Error> {
         let backup_user = pbs_config::backup_user()?;
         let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
         let opts = CreateOptions::new()
@@ -178,15 +181,15 @@ impl MediaCatalog {
     }
 
     /// Open a catalog database, load into memory
-    pub fn open(
-        base_path: &Path,
+    pub fn open<P: AsRef<Path>>(
+        base_path: P,
         media_id: &MediaId,
         write: bool,
         create: bool,
     ) -> Result<Self, Error> {
         let uuid = &media_id.label.uuid;
 
-        let path = Self::catalog_path(base_path, uuid);
+        let path = Self::catalog_path(&base_path, uuid);
 
         let me = proxmox_lang::try_block!({
             Self::create_basedir(base_path)?;
@@ -238,8 +241,11 @@ impl MediaCatalog {
     }
 
     /// Creates a temporary empty catalog file
-    pub fn create_temporary_database_file(base_path: &Path, uuid: &Uuid) -> Result<File, Error> {
-        Self::create_basedir(base_path)?;
+    pub fn create_temporary_database_file<P: AsRef<Path>>(
+        base_path: P,
+        uuid: &Uuid,
+    ) -> Result<File, Error> {
+        Self::create_basedir(&base_path)?;
 
         let tmp_path = Self::tmp_catalog_path(base_path, uuid);
 
@@ -269,14 +275,14 @@ impl MediaCatalog {
     /// Creates a temporary, empty catalog database
     ///
     /// Creates a new catalog file using a ".tmp" file extension.
-    pub fn create_temporary_database(
-        base_path: &Path,
+    pub fn create_temporary_database<P: AsRef<Path>>(
+        base_path: P,
         media_id: &MediaId,
         log_to_stdout: bool,
     ) -> Result<Self, Error> {
         let uuid = &media_id.label.uuid;
 
-        let tmp_path = Self::tmp_catalog_path(base_path, uuid);
+        let tmp_path = Self::tmp_catalog_path(&base_path, uuid);
 
         let me = proxmox_lang::try_block!({
             let file = Self::create_temporary_database_file(base_path, uuid)?;
@@ -321,8 +327,8 @@ impl MediaCatalog {
     ///
     /// With commit set, we rename the ".tmp" file extension to
     /// ".log". When commit is false, we remove the ".tmp" file.
-    pub fn finish_temporary_database(
-        base_path: &Path,
+    pub fn finish_temporary_database<P: AsRef<Path>>(
+        base_path: P,
         uuid: &Uuid,
         commit: bool,
     ) -> Result<(), Error> {
@@ -396,14 +402,14 @@ impl MediaCatalog {
     }
 
     /// Destroy existing catalog, opens a new one
-    pub fn overwrite(
-        base_path: &Path,
+    pub fn overwrite<P: AsRef<Path>>(
+        base_path: P,
         media_id: &MediaId,
         log_to_stdout: bool,
     ) -> Result<Self, Error> {
         let uuid = &media_id.label.uuid;
 
-        let me = Self::create_temporary_database(base_path, media_id, log_to_stdout)?;
+        let me = Self::create_temporary_database(&base_path, media_id, log_to_stdout)?;
 
         Self::finish_temporary_database(base_path, uuid, true)?;
 
diff --git a/src/tape/media_catalog_cache.rs b/src/tape/media_catalog_cache.rs
index 5d20101c..f3466271 100644
--- a/src/tape/media_catalog_cache.rs
+++ b/src/tape/media_catalog_cache.rs
@@ -11,13 +11,13 @@ use crate::tape::{MediaCatalog, MediaId};
 ///
 /// To speedup things for large catalogs, we cache the list of
 /// snapshots into a separate file.
-pub fn media_catalog_snapshot_list(
-    base_path: &Path,
+pub fn media_catalog_snapshot_list<P: AsRef<Path>>(
+    base_path: P,
     media_id: &MediaId,
 ) -> Result<Vec<(String, String)>, Error> {
     let uuid = &media_id.label.uuid;
 
-    let mut cache_path = base_path.to_owned();
+    let mut cache_path = base_path.as_ref().to_owned();
     cache_path.push(uuid.to_string());
     let mut catalog_path = cache_path.clone();
     cache_path.set_extension("index");
@@ -42,7 +42,7 @@ pub fn media_catalog_snapshot_list(
                 Some(Ok(id)) => {
                     if id != cache_id {
                         // cache is outdated - rewrite
-                        return write_snapshot_cache(base_path, media_id, &cache_path, &cache_id);
+                        return write_snapshot_cache(&base_path, media_id, &cache_path, &cache_id);
                     }
                 }
                 _ => bail!("unable to read catalog cache firstline {:?}", cache_path),
@@ -69,8 +69,8 @@ pub fn media_catalog_snapshot_list(
     }
 }
 
-fn write_snapshot_cache(
-    base_path: &Path,
+fn write_snapshot_cache<P: AsRef<Path>>(
+    base_path: P,
     media_id: &MediaId,
     cache_path: &Path,
     cache_id: &str,
diff --git a/src/tape/media_pool.rs b/src/tape/media_pool.rs
index c688693f..ec0cd857 100644
--- a/src/tape/media_pool.rs
+++ b/src/tape/media_pool.rs
@@ -56,9 +56,9 @@ impl MediaPool {
     /// `changer`, all offline media is considered available (backups
     /// to standalone drives may not use media from inside a tape
     /// library).
-    pub fn new(
+    pub fn new<P: AsRef<Path>>(
         name: &str,
-        state_path: &Path,
+        state_path: P,
         media_set_policy: MediaSetPolicy,
         retention: RetentionPolicy,
         changer_name: Option<String>,
@@ -68,10 +68,10 @@ impl MediaPool {
         let _pool_lock = if no_media_set_locking {
             None
         } else {
-            Some(lock_media_pool(state_path, name)?)
+            Some(lock_media_pool(&state_path, name)?)
         };
 
-        let inventory = Inventory::load(state_path)?;
+        let inventory = Inventory::load(&state_path)?;
 
         let current_media_set = match inventory.latest_media_set(name) {
             Some(set_uuid) => inventory.compute_media_set_members(&set_uuid)?,
@@ -81,12 +81,12 @@ impl MediaPool {
         let current_media_set_lock = if no_media_set_locking {
             None
         } else {
-            Some(lock_media_set(state_path, current_media_set.uuid(), None)?)
+            Some(lock_media_set(&state_path, current_media_set.uuid(), None)?)
         };
 
         Ok(MediaPool {
             name: String::from(name),
-            state_path: state_path.to_owned(),
+            state_path: state_path.as_ref().to_owned(),
             media_set_policy,
             retention,
             changer_name,
@@ -112,8 +112,8 @@ impl MediaPool {
     }
 
     /// Creates a new instance using the media pool configuration
-    pub fn with_config(
-        state_path: &Path,
+    pub fn with_config<P: AsRef<Path>>(
+        state_path: P,
         config: &MediaPoolConfig,
         changer_name: Option<String>,
         no_media_set_locking: bool, // for list_media()
diff --git a/src/tape/pool_writer/mod.rs b/src/tape/pool_writer/mod.rs
index d00c16e6..96b06896 100644
--- a/src/tape/pool_writer/mod.rs
+++ b/src/tape/pool_writer/mod.rs
@@ -5,7 +5,7 @@ mod new_chunks_iterator;
 pub use new_chunks_iterator::*;
 
 use std::fs::File;
-use std::path::Path;
+use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
 use std::time::SystemTime;
 
@@ -75,8 +75,7 @@ impl PoolWriter {
         // load all catalogs read-only at start
         for media_uuid in pool.current_media_list()? {
             let media_info = pool.lookup_media(media_uuid).unwrap();
-            let media_catalog =
-                MediaCatalog::open(Path::new(TAPE_STATUS_DIR), media_info.id(), false, false)?;
+            let media_catalog = MediaCatalog::open(TAPE_STATUS_DIR, media_info.id(), false, false)?;
             catalog_set.append_read_only_catalog(media_catalog)?;
         }
 
@@ -277,8 +276,7 @@ impl PoolWriter {
     }
 
     fn open_catalog_file(uuid: &Uuid) -> Result<File, Error> {
-        let status_path = Path::new(TAPE_STATUS_DIR);
-        let mut path = status_path.to_owned();
+        let mut path = PathBuf::from(TAPE_STATUS_DIR);
         path.push(uuid.to_string());
         path.set_extension("log");
 
@@ -613,13 +611,11 @@ fn update_media_set_label(
         None
     };
 
-    let status_path = Path::new(TAPE_STATUS_DIR);
-
     let new_media = match old_set {
         None => {
             task_log!(worker, "writing new media set label");
             drive.write_media_set_label(new_set, key_config.as_ref())?;
-            media_catalog = MediaCatalog::overwrite(status_path, media_id, false)?;
+            media_catalog = MediaCatalog::overwrite(TAPE_STATUS_DIR, media_id, false)?;
             true
         }
         Some(media_set_label) => {
@@ -635,7 +631,7 @@ fn update_media_set_label(
                 {
                     bail!("detected changed encryption fingerprint - internal error");
                 }
-                media_catalog = MediaCatalog::open(status_path, media_id, true, false)?;
+                media_catalog = MediaCatalog::open(TAPE_STATUS_DIR, media_id, true, false)?;
 
                 // todo: verify last content/media_catalog somehow?
 
@@ -649,7 +645,7 @@ fn update_media_set_label(
                 );
 
                 drive.write_media_set_label(new_set, key_config.as_ref())?;
-                media_catalog = MediaCatalog::overwrite(status_path, media_id, false)?;
+                media_catalog = MediaCatalog::overwrite(TAPE_STATUS_DIR, media_id, false)?;
                 true
             }
         }
-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 3/5] api/tape/inventory: optionally try to restore catalogs
  2022-05-19 10:08 [pbs-devel] [PATCH proxmox-backup 0/5] allow restoring catalogs during inventory Dominik Csapak
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 1/5] tape/inventory: make 'load_media_db' a method Dominik Csapak
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 2/5] tape: replace '&Path' with 'AsRef<Path>' in function parameters Dominik Csapak
@ 2022-05-19 10:08 ` Dominik Csapak
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 4/5] proxmox-tape: add 'catalog' option to 'proxmox-tape inventory' Dominik Csapak
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 5/5] ui: tape/ChangerStatus: adding parameter selection to inventory Dominik Csapak
  4 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2022-05-19 10:08 UTC (permalink / raw)
  To: pbs-devel

in a disaster recovery case, it is useful to not only re-invetorize
the labels + media-sets, but also to try to recover the catalogs
from the tape (to know whats on there). This adds an option to
the inventory api call that tries to do a fast catalog restore
from each tape to be inventorized.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/drive.rs | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 28c0f0f5..3394afe9 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -848,6 +848,13 @@ pub async fn inventory(drive: String) -> Result<Vec<LabelUuidMap>, Error> {
             "read-all-labels": {
                 description: "Load all tapes and try read labels (even if already inventoried)",
                 type: bool,
+                default: false,
+                optional: true,
+            },
+            "catalog": {
+                description: "Restore the catalog from tape.",
+                type: bool,
+                default: false,
                 optional: true,
             },
         },
@@ -867,10 +874,13 @@ pub async fn inventory(drive: String) -> Result<Vec<LabelUuidMap>, Error> {
 /// then loads any unknown media into the drive, reads the label, and
 /// store the result to the media database.
 ///
+/// If `catalog` is true, also tries to restore the catalog from tape.
+///
 /// Note: This updates the media online status.
 pub fn update_inventory(
     drive: String,
     read_all_labels: Option<bool>,
+    catalog: Option<bool>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
     let upid_str = run_drive_worker(
@@ -889,6 +899,8 @@ pub fn update_inventory(
             let mut inventory = Inventory::load(TAPE_STATUS_DIR)?;
 
             update_changer_online_status(&config, &mut inventory, &changer_name, &label_text_list)?;
+            let catalog = catalog.unwrap_or(false);
+            let read_all_labels = read_all_labels.unwrap_or(false);
 
             for label_text in label_text_list.iter() {
                 if label_text.starts_with("CLN") {
@@ -898,11 +910,13 @@ pub fn update_inventory(
 
                 let label_text = label_text.to_string();
 
-                if !read_all_labels.unwrap_or(false)
-                    && inventory.find_media_by_label_text(&label_text).is_some()
-                {
-                    task_log!(worker, "media '{}' already inventoried", label_text);
-                    continue;
+                if !read_all_labels {
+                    if let Some(media_id) = inventory.find_media_by_label_text(&label_text) {
+                        if !catalog || MediaCatalog::exists(TAPE_STATUS_DIR, &media_id.label.uuid) {
+                            task_log!(worker, "media '{}' already inventoried", label_text);
+                            continue;
+                        }
+                    }
                 }
 
                 if let Err(err) = changer.load_media(&label_text) {
@@ -947,7 +961,22 @@ pub fn update_inventory(
                             let _pool_lock = lock_media_pool(TAPE_STATUS_DIR, pool)?;
                             let _lock = lock_media_set(TAPE_STATUS_DIR, uuid, None)?;
                             MediaCatalog::destroy_unrelated_catalog(TAPE_STATUS_DIR, &media_id)?;
-                            inventory.store(media_id, false)?;
+                            inventory.store(media_id.clone(), false)?;
+
+                            if catalog {
+                                let media_set = inventory.compute_media_set_members(uuid)?;
+                                if let Err(err) = fast_catalog_restore(
+                                    &worker,
+                                    &mut drive,
+                                    &media_set,
+                                    &media_id.label.uuid,
+                                ) {
+                                    task_warn!(
+                                        worker,
+                                        "could not restore catalog for {label_text}: {err}"
+                                    );
+                                }
+                            }
                         } else {
                             let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?;
                             MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 4/5] proxmox-tape: add 'catalog' option to 'proxmox-tape inventory'
  2022-05-19 10:08 [pbs-devel] [PATCH proxmox-backup 0/5] allow restoring catalogs during inventory Dominik Csapak
                   ` (2 preceding siblings ...)
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 3/5] api/tape/inventory: optionally try to restore catalogs Dominik Csapak
@ 2022-05-19 10:08 ` Dominik Csapak
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 5/5] ui: tape/ChangerStatus: adding parameter selection to inventory Dominik Csapak
  4 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2022-05-19 10:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/proxmox-tape.rs | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/bin/proxmox-tape.rs b/src/bin/proxmox-tape.rs
index af39556e..56d864a2 100644
--- a/src/bin/proxmox-tape.rs
+++ b/src/bin/proxmox-tape.rs
@@ -444,6 +444,12 @@ async fn read_label(mut param: Value) -> Result<(), Error> {
                 type: bool,
                 optional: true,
             },
+            "catalog": {
+                description: "Try to restore catalogs from tapes.",
+                type: bool,
+                default: false,
+                optional: true,
+            }
         },
     },
 )]
@@ -451,6 +457,7 @@ async fn read_label(mut param: Value) -> Result<(), Error> {
 async fn inventory(
     read_labels: Option<bool>,
     read_all_labels: Option<bool>,
+    catalog: Option<bool>,
     mut param: Value,
 ) -> Result<(), Error> {
     let output_format = extract_output_format(&mut param);
@@ -458,7 +465,9 @@ async fn inventory(
     let (config, _digest) = pbs_config::drive::config()?;
     let drive = extract_drive_name(&mut param, &config)?;
 
-    let do_read = read_labels.unwrap_or(false) || read_all_labels.unwrap_or(false);
+    let read_all_labels = read_all_labels.unwrap_or(false);
+    let catalog = catalog.unwrap_or(false);
+    let do_read = read_labels.unwrap_or(false) || read_all_labels || catalog;
 
     let client = connect_to_localhost()?;
 
@@ -466,9 +475,8 @@ async fn inventory(
 
     if do_read {
         let mut param = json!({});
-        if let Some(true) = read_all_labels {
-            param["read-all-labels"] = true.into();
-        }
+        param["read-all-labels"] = read_all_labels.into();
+        param["catalog"] = catalog.into();
 
         let result = client.put(&path, Some(param)).await?; // update inventory
         view_task_result(&client, result, &output_format).await?;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 5/5] ui: tape/ChangerStatus: adding parameter selection to inventory
  2022-05-19 10:08 [pbs-devel] [PATCH proxmox-backup 0/5] allow restoring catalogs during inventory Dominik Csapak
                   ` (3 preceding siblings ...)
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 4/5] proxmox-tape: add 'catalog' option to 'proxmox-tape inventory' Dominik Csapak
@ 2022-05-19 10:08 ` Dominik Csapak
  4 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2022-05-19 10:08 UTC (permalink / raw)
  To: pbs-devel

namely 'catalog' and 'read-all-labels', by always opening a
window (with a drive now autoselected) and the two checkboxes

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/tape/ChangerStatus.js | 66 ++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 36 deletions(-)

diff --git a/www/tape/ChangerStatus.js b/www/tape/ChangerStatus.js
index a99b6ba6..27c7605c 100644
--- a/www/tape/ChangerStatus.js
+++ b/www/tape/ChangerStatus.js
@@ -375,46 +375,40 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 		return;
 	    }
 
-	    let singleDrive = me.drives.length === 1 ? me.drives[0] : undefined;
+	    Ext.create('Proxmox.window.Edit', {
+		title: gettext('Inventory'),
+		showTaskViewer: true,
+		method: 'PUT',
+		url: '/api2/extjs/tape/drive',
+		submitUrl: function(url, values) {
+		    let drive = values.drive;
+		    delete values.drive;
+		    return `${url}/${encodeURIComponent(drive)}/inventory`;
+		},
 
-	    if (singleDrive !== undefined) {
-		Proxmox.Utils.API2Request({
-		    method: 'PUT',
-		    url: `/api2/extjs/tape/drive/${singleDrive}/inventory`,
-		    success: function(response, opt) {
-			Ext.create('Proxmox.window.TaskViewer', {
-			    upid: response.result.data,
-			    taskDone: function(success) {
-				me.reload();
-			    },
-			}).show();
+		items: [
+		    {
+			xtype: 'pbsDriveSelector',
+			labelWidth: 120,
+			fieldLabel: gettext('Drive'),
+			name: 'drive',
+			changer: changer,
+			autoSelect: true,
 		    },
-		    failure: function(response, opt) {
-			Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		    {
+			xtype: 'proxmoxcheckbox',
+			labelWidth: 120,
+			fieldLabel: gettext('Restore Catalogs'),
+			name: 'catalog',
 		    },
-		});
-	    } else {
-		Ext.create('Proxmox.window.Edit', {
-		    title: gettext('Inventory'),
-		    showTaskViewer: true,
-		    method: 'PUT',
-		    url: '/api2/extjs/tape/drive',
-		    submitUrl: function(url, values) {
-			let drive = values.drive;
-			delete values.drive;
-			return `${url}/${encodeURIComponent(drive)}/inventory`;
+		    {
+			xtype: 'proxmoxcheckbox',
+			labelWidth: 120,
+			fieldLabel: gettext('Force all Tapes'),
+			name: 'read-all-labels',
 		    },
-
-		    items: [
-			{
-			    xtype: 'pbsDriveSelector',
-			    fieldLabel: gettext('Drive'),
-			    name: 'drive',
-			    changer: changer,
-			},
-		    ],
-		}).show();
-	    }
+		],
+	    }).show();
 	},
 
 	scheduleReload: function(time) {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] applied: [PATCH proxmox-backup 1/5] tape/inventory: make 'load_media_db' a method
  2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 1/5] tape/inventory: make 'load_media_db' a method Dominik Csapak
@ 2022-10-05 17:32   ` Thomas Lamprecht
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2022-10-05 17:32 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

Am 19/05/2022 um 12:08 schrieb Dominik Csapak:
> and use self.inventory_path. This is only used internally (not pub) so there
> is no need to have it as a static function.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/tape/inventory.rs | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
>

applied this one, thanks! The rest of the series would need rebasing, I
skimmed it only quickly though so no in-depth review available from my
side; sorry.




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-10-05 17:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19 10:08 [pbs-devel] [PATCH proxmox-backup 0/5] allow restoring catalogs during inventory Dominik Csapak
2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 1/5] tape/inventory: make 'load_media_db' a method Dominik Csapak
2022-10-05 17:32   ` [pbs-devel] applied: " Thomas Lamprecht
2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 2/5] tape: replace '&Path' with 'AsRef<Path>' in function parameters Dominik Csapak
2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 3/5] api/tape/inventory: optionally try to restore catalogs Dominik Csapak
2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 4/5] proxmox-tape: add 'catalog' option to 'proxmox-tape inventory' Dominik Csapak
2022-05-19 10:08 ` [pbs-devel] [PATCH proxmox-backup 5/5] ui: tape/ChangerStatus: adding parameter selection to inventory Dominik Csapak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal