[pbs-devel] [PATCH proxmox-backup v2 1/4] tape: replace '&Path' with 'AsRef<Path>' in function parameters

Dominik Csapak d.csapak at proxmox.com
Thu Oct 6 13:36:07 CEST 2022


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 at 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              | 26 ++++-----
 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(+), 157 deletions(-)

diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 472fd5c3..735aea15 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};
@@ -88,7 +87,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 {
@@ -111,7 +109,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);
@@ -391,7 +390,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");
@@ -400,7 +398,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)?;
@@ -584,8 +582,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 ad7c4354..ed0105b0 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 && pool.is_none() {
@@ -295,8 +292,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)
@@ -332,8 +328,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)
@@ -389,8 +384,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();
 
@@ -435,7 +429,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 {
@@ -478,8 +472,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);
 
@@ -504,8 +497,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 2872f7c4..6e13bd89 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -362,11 +362,10 @@ pub fn restore(
     user_info.check_privs(&auth_id, &["tape", "drive", &drive], PRIV_TAPE_READ, false)?;
 
     let media_set_uuid = media_set.parse()?;
-    let status_path = Path::new(TAPE_STATUS_DIR);
 
-    let _lock = lock_media_set(status_path, &media_set_uuid, None)?;
+    let _lock = lock_media_set(TAPE_STATUS_DIR, &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)?;
     user_info.check_privs(&auth_id, &["tape", "pool", &pool], PRIV_TAPE_READ, false)?;
@@ -945,8 +944,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();
@@ -958,7 +955,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)?;
             }
         }
@@ -1295,8 +1292,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()?;
@@ -1333,7 +1329,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(())
 }
@@ -1820,8 +1816,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");
@@ -1902,7 +1896,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",
@@ -1914,7 +1908,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)?;
 
@@ -1939,7 +1933,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 5bfa3b3c..d8dac669 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 d0f101bd..9c5887d7 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 b3aa5998..4a76bba0 100644
--- a/src/tape/media_catalog.rs
+++ b/src/tape/media_catalog.rs
@@ -66,10 +66,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") {
@@ -83,27 +83,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) {
@@ -114,7 +114,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);
@@ -165,7 +168,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()
@@ -179,15 +182,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)?;
@@ -239,8 +242,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);
 
@@ -270,14 +276,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)?;
@@ -322,8 +328,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> {
@@ -397,14 +403,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 07413f1f..eb477885 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 ddd6bd8c..ce6e3605 100644
--- a/src/tape/pool_writer/mod.rs
+++ b/src/tape/pool_writer/mod.rs
@@ -6,7 +6,7 @@ pub use new_chunks_iterator::*;
 
 use std::collections::HashSet;
 use std::fs::File;
-use std::path::Path;
+use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
 use std::time::SystemTime;
 
@@ -77,8 +77,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)?;
         }
 
@@ -297,8 +296,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");
 
@@ -634,13 +632,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) => {
@@ -656,7 +652,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?
 
@@ -670,7 +666,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






More information about the pbs-devel mailing list