all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 2/5] tape: replace '&Path' with 'AsRef<Path>' in function parameters
Date: Thu, 19 May 2022 12:08:17 +0200	[thread overview]
Message-ID: <20220519100820.1829147-3-d.csapak@proxmox.com> (raw)
In-Reply-To: <20220519100820.1829147-1-d.csapak@proxmox.com>

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





  parent reply	other threads:[~2022-05-19 10:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Dominik Csapak [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220519100820.1829147-3-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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