From cdf39e62b348e66a9d07179d2c4f619fe45b995f Mon Sep 17 00:00:00 2001 From: Dietmar Maurer Date: Thu, 4 Feb 2021 10:15:18 +0100 Subject: [PATCH] tape: MediaPool - replace use_offline_media with changer_name This way, we can improve location_is_available, because we only consider media from that changer as available. --- src/api2/tape/backup.rs | 20 ++++++++---------- src/api2/tape/media.rs | 4 ++-- src/tape/media_pool.rs | 32 ++++++++++++++++++++++------- src/tape/test/current_set_usable.rs | 14 ++++++------- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs index b512ef98..0a169ff6 100644 --- a/src/api2/tape/backup.rs +++ b/src/api2/tape/backup.rs @@ -131,11 +131,9 @@ fn backup_worker( let _lock = MediaPool::lock(status_path, &pool_config.name)?; task_log!(worker, "update media online status"); - let has_changer = update_media_online_status(drive)?; + let changer_name = update_media_online_status(drive)?; - let use_offline_media = !has_changer; - - let pool = MediaPool::with_config(status_path, &pool_config, use_offline_media)?; + let pool = MediaPool::with_config(status_path, &pool_config, changer_name)?; let mut pool_writer = PoolWriter::new(pool, drive)?; @@ -168,17 +166,13 @@ fn backup_worker( } // Try to update the the media online status -fn update_media_online_status(drive: &str) -> Result { +fn update_media_online_status(drive: &str) -> Result, Error> { let (config, _digest) = config::drive::config()?; - let mut has_changer = false; - if let Ok(Some((mut changer, changer_name))) = media_changer(&config, drive) { - has_changer = true; - - let label_text_list = changer.online_media_label_texts()?; + let label_text_list = changer.online_media_label_texts()?; let status_path = Path::new(TAPE_STATUS_DIR); let mut inventory = Inventory::load(status_path)?; @@ -189,9 +183,11 @@ fn update_media_online_status(drive: &str) -> Result { &changer_name, &label_text_list, )?; - } - Ok(has_changer) + Ok(Some(changer_name)) + } else { + Ok(None) + } } pub fn backup_snapshot( diff --git a/src/api2/tape/media.rs b/src/api2/tape/media.rs index fd80a933..27ed37c9 100644 --- a/src/api2/tape/media.rs +++ b/src/api2/tape/media.rs @@ -86,8 +86,8 @@ pub async fn list_media(pool: Option) -> Result, Err let config: MediaPoolConfig = config.lookup("pool", pool_name)?; - let use_offline_media = true; // does not matter here - let pool = MediaPool::with_config(status_path, &config, use_offline_media)?; + let changer_name = None; // does not matter here + let pool = MediaPool::with_config(status_path, &config, changer_name)?; let current_time = proxmox::tools::time::epoch_i64(); diff --git a/src/tape/media_pool.rs b/src/tape/media_pool.rs index c4eabcfc..fc4410b0 100644 --- a/src/tape/media_pool.rs +++ b/src/tape/media_pool.rs @@ -44,7 +44,9 @@ pub struct MediaPool { media_set_policy: MediaSetPolicy, retention: RetentionPolicy, - use_offline_media: bool, + + changer_name: Option, + encrypt_fingerprint: Option, inventory: Inventory, @@ -55,12 +57,18 @@ pub struct MediaPool { impl MediaPool { /// Creates a new instance + /// + /// If you specify a `changer_name`, only media accessible via + /// that changer is considered available. If you pass `None` for + /// `changer`, all offline media is considered available (backups + /// to standalone drives may not use media from inside a tape + /// library). pub fn new( name: &str, state_path: &Path, media_set_policy: MediaSetPolicy, retention: RetentionPolicy, - use_offline_media: bool, + changer_name: Option, encrypt_fingerprint: Option, ) -> Result { @@ -75,7 +83,7 @@ impl MediaPool { name: String::from(name), media_set_policy, retention, - use_offline_media, + changer_name, inventory, current_media_set, encrypt_fingerprint, @@ -86,7 +94,7 @@ impl MediaPool { pub fn with_config( state_path: &Path, config: &MediaPoolConfig, - use_offline_media: bool, + changer_name: Option, ) -> Result { let allocation = config.allocation.clone().unwrap_or_else(|| String::from("continue")).parse()?; @@ -103,7 +111,7 @@ impl MediaPool { state_path, allocation, retention, - use_offline_media, + changer_name, encrypt_fingerprint, ) } @@ -272,8 +280,18 @@ impl MediaPool { // check if a location is considered on site pub fn location_is_available(&self, location: &MediaLocation) -> bool { match location { - MediaLocation::Online(_) => true, - MediaLocation::Offline => self.use_offline_media, + MediaLocation::Online(name) => { + if let Some(ref changer_name) = self.changer_name { + name == changer_name + } else { + // a standalone drive cannot use media currently inside a library + false + } + } + MediaLocation::Offline => { + // consider available for standalone drives + self.changer_name.is_none() + } MediaLocation::Vault(_) => false, } } diff --git a/src/tape/test/current_set_usable.rs b/src/tape/test/current_set_usable.rs index d11ce8ab..a2f39240 100644 --- a/src/tape/test/current_set_usable.rs +++ b/src/tape/test/current_set_usable.rs @@ -49,7 +49,7 @@ fn test_current_set_usable_1() -> Result<(), Error> { &testdir, MediaSetPolicy::AlwaysCreate, RetentionPolicy::KeepForever, - true, + None, None, )?; @@ -75,7 +75,7 @@ fn test_current_set_usable_2() -> Result<(), Error> { &testdir, MediaSetPolicy::AlwaysCreate, RetentionPolicy::KeepForever, - true, + None, None, )?; @@ -103,7 +103,7 @@ fn test_current_set_usable_3() -> Result<(), Error> { &testdir, MediaSetPolicy::AlwaysCreate, RetentionPolicy::KeepForever, - false, + Some(String::from("changer1")), None, )?; @@ -131,7 +131,7 @@ fn test_current_set_usable_4() -> Result<(), Error> { &testdir, MediaSetPolicy::AlwaysCreate, RetentionPolicy::KeepForever, - true, + None, None, )?; @@ -161,7 +161,7 @@ fn test_current_set_usable_5() -> Result<(), Error> { &testdir, MediaSetPolicy::AlwaysCreate, RetentionPolicy::KeepForever, - true, + None, None, )?; @@ -189,7 +189,7 @@ fn test_current_set_usable_6() -> Result<(), Error> { &testdir, MediaSetPolicy::AlwaysCreate, RetentionPolicy::KeepForever, - true, + None, None, )?; @@ -223,7 +223,7 @@ fn test_current_set_usable_7() -> Result<(), Error> { &testdir, MediaSetPolicy::AlwaysCreate, RetentionPolicy::KeepForever, - true, + None, None, )?;