From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A0E6291384 for ; Thu, 6 Oct 2022 13:36:49 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7D3231C0B1 for ; Thu, 6 Oct 2022 13:36:19 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 6 Oct 2022 13:36:15 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 22B4C43F10 for ; Thu, 6 Oct 2022 13:36:15 +0200 (CEST) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Thu, 6 Oct 2022 13:36:07 +0200 Message-Id: <20221006113610.2764065-2-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221006113610.2764065-1-d.csapak@proxmox.com> References: <20221006113610.2764065-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.068 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [backup.rs, restore.rs, media.rs, drive.rs, inventory.rs, changer.rs, mod.rs] Subject: [pbs-devel] [PATCH proxmox-backup v2 1/4] tape: replace '&Path' with 'AsRef' in function parameters X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Oct 2022 11:36:49 -0000 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 --- 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, 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 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, ) -> 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) -> Result Result, 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 = 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) -> 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) -> Result<(), E pub fn destroy_media(label_text: String, force: Option) -> 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 { - 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 { /// 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) -> 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 { - 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 { - 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>( + state_path: P, changer: Option<&str>, ) -> Result { 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>(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 { + pub fn load>(base_path: P) -> Result { 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 { - let mut path = base_path.to_owned(); +pub fn lock_media_pool>(base_path: P, name: &str) -> Result { + 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 Result { +pub fn lock_unassigned_media_pool>(base_path: P) -> Result { // 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>( + base_path: P, media_set_uuid: &Uuid, timeout: Option, ) -> Result { - 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) -> Vec { - 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) -> Vec< /// List of known media sets pub fn complete_media_set_uuid(_arg: &str, _param: &HashMap) -> Vec { - 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) -> /// List of known media labels (barcodes) pub fn complete_media_label_text(_arg: &str, _param: &HashMap) -> Vec { - 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) 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) }); 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, Error> { + pub fn media_with_catalogs>(base_path: P) -> Result, 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>(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>(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>(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>(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>( + 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>(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>( + base_path: P, media_id: &MediaId, write: bool, create: bool, ) -> Result { 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 { - Self::create_basedir(base_path)?; + pub fn create_temporary_database_file>( + base_path: P, + uuid: &Uuid, + ) -> Result { + 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>( + base_path: P, media_id: &MediaId, log_to_stdout: bool, ) -> Result { 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>( + 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>( + base_path: P, media_id: &MediaId, log_to_stdout: bool, ) -> Result { 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>( + base_path: P, media_id: &MediaId, ) -> Result, 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>( + 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>( name: &str, - state_path: &Path, + state_path: P, media_set_policy: MediaSetPolicy, retention: RetentionPolicy, changer_name: Option, @@ -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>( + state_path: P, config: &MediaPoolConfig, changer_name: Option, 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 { - 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