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 20E3A95159 for ; Tue, 28 Feb 2023 14:17:27 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 09F9B720F for ; Tue, 28 Feb 2023 14:17:27 +0100 (CET) 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 ; Tue, 28 Feb 2023 14:17:25 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 18B3C48B62 for ; Tue, 28 Feb 2023 14:17:25 +0100 (CET) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Tue, 28 Feb 2023 14:17:21 +0100 Message-Id: <20230228131722.955886-2-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230228131722.955886-1-d.csapak@proxmox.com> References: <20230228131722.955886-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.061 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. [drive.rs, self.map, media.rs, pool.id, mod.rs, self.name, inventory.rs] Subject: [pbs-devel] [PATCH proxmox-backup v2 2/3] fix #4412: tape: initial WORM support 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: Tue, 28 Feb 2023 13:17:27 -0000 the only thing preventing us from using WORM tapes was that we relied on being able to rewrite the media set label when first using a tape that was pre-allocated in a media-pool. so instead of needing to write a meida set label with a special uuid, just save the pool in the media label itself. This has currently no downsides, as we're not able to move tapes from one pool to another anyway. this makes some checks a bit trickier, as we now have to get the pool out of the media set label and as a fallback look into the media label. such new tapes can still be read and restored by older proxmox-bacukp-server versions. The only thing missing is when a tape labeled with the new format that has an assigned pool, that pool won't show up when the tape is inventoried in an old version (but can still be used otherwise). Signed-off-by: Dominik Csapak --- changes from v1: * improved refactor of the locks in format_media * added 'fix #4412' to commit message dietmars remarks about different pools don't apply, since the 'pool()' method already returns the pool from the media set label src/api2/tape/drive.rs | 111 ++++++++++++++++++----------------- src/api2/tape/media.rs | 40 ++++++++----- src/tape/file_formats/mod.rs | 3 + src/tape/inventory.rs | 49 +++++++++------- src/tape/media_pool.rs | 10 +--- 5 files changed, 115 insertions(+), 98 deletions(-) diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs index a66c999c..2d465537 100644 --- a/src/api2/tape/drive.rs +++ b/src/api2/tape/drive.rs @@ -346,20 +346,24 @@ pub fn format_media( 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(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)?; + let _pool_lock = if let Some(pool) = media_id.pool() { + lock_media_pool(TAPE_STATUS_DIR, &pool)? } else { - 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)?; + lock_unassigned_media_pool(TAPE_STATUS_DIR)? + }; + + let _media_set_lock = match media_id.media_set_label { + Some(MediaSetLabel { ref uuid, .. }) => { + Some(lock_media_set(TAPE_STATUS_DIR, uuid, None)?) + } + None => None, }; + MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?; + inventory.remove_media(&media_id.label.uuid)?; + drop(_media_set_lock); + drop(_pool_lock); + handle.format_media(fast.unwrap_or(true))?; } } @@ -504,6 +508,7 @@ pub fn label_media( label_text: label_text.to_string(), uuid: Uuid::generate(), ctime, + pool: pool.clone(), }; write_media_label(worker, &mut drive, label, pool) @@ -520,50 +525,31 @@ fn write_media_label( pool: Option, ) -> Result<(), Error> { drive.label_tape(&label)?; - let media_id = if let Some(ref pool) = pool { - // assign media to pool by writing special media set label + if let Some(ref pool) = pool { task_log!( worker, "Label media '{}' for pool '{}'", label.label_text, pool ); - let set = MediaSetLabel::new_unassigned(pool, label.ctime); - - drive.write_media_set_label(&set, None)?; - - let media_id = MediaId { - label, - media_set_label: Some(set), - }; - - // Create the media catalog - MediaCatalog::overwrite(TAPE_STATUS_DIR, &media_id, false)?; - - let mut inventory = Inventory::new(TAPE_STATUS_DIR); - inventory.store(media_id.clone(), false)?; - - media_id } else { task_log!( worker, "Label media '{}' (no pool assignment)", label.label_text ); + } - let media_id = MediaId { - label, - media_set_label: None, - }; - - // Create the media catalog - MediaCatalog::overwrite(TAPE_STATUS_DIR, &media_id, false)?; + let media_id = MediaId { + label, + media_set_label: None, + }; - let mut inventory = Inventory::new(TAPE_STATUS_DIR); - inventory.store(media_id.clone(), false)?; + // Create the media catalog + MediaCatalog::overwrite(TAPE_STATUS_DIR, &media_id, false)?; - media_id - }; + let mut inventory = Inventory::new(TAPE_STATUS_DIR); + inventory.store(media_id.clone(), false)?; drive.rewind()?; @@ -573,8 +559,8 @@ fn write_media_label( bail!("verify label failed - got wrong label uuid"); } if let Some(ref pool) = pool { - match info.media_set_label { - Some(set) => { + match (info.label.pool, info.media_set_label) { + (None, Some(set)) => { if !set.unassigned() { bail!("verify media set label failed - got wrong set uuid"); } @@ -582,7 +568,12 @@ fn write_media_label( bail!("verify media set label failed - got wrong pool"); } } - None => { + (Some(initial_pool), _) => { + if initial_pool != *pool { + bail!("verify media label failed - got wrong pool"); + } + } + (None, None) => { bail!("verify media set label failed (missing set label)"); } } @@ -686,7 +677,7 @@ pub async fn read_label(drive: String, inventorize: Option) -> Result) -> Result { task_log!(worker, "media is empty"); - let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?; + let _pool_lock = if let Some(pool) = media_id.pool() { + lock_media_pool(TAPE_STATUS_DIR, &pool)? + } else { + lock_unassigned_media_pool(TAPE_STATUS_DIR)? + }; MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?; inventory.store(media_id.clone(), false)?; return Ok(()); diff --git a/src/api2/tape/media.rs b/src/api2/tape/media.rs index cdeffd5b..3e8e1a9d 100644 --- a/src/api2/tape/media.rs +++ b/src/api2/tape/media.rs @@ -240,37 +240,45 @@ pub async fn list_media( // set status to MediaStatus::Unknown for uuid in inventory.media_list() { let media_id = inventory.lookup_media(uuid).unwrap(); - let media_set_label = match media_id.media_set_label { - Some(ref set) => set, - None => continue, - }; - if config.sections.get(&media_set_label.pool).is_some() { - continue; - } + if let Some(pool) = media_id.pool() { + if config.sections.get(&pool).is_some() { + continue; + } - let privs = user_info.lookup_privs(&auth_id, &["tape", "pool", &media_set_label.pool]); - if (privs & PRIV_TAPE_AUDIT) == 0 { + let privs = user_info.lookup_privs(&auth_id, &["tape", "pool", &pool]); + if (privs & PRIV_TAPE_AUDIT) == 0 { + continue; + } + } else { continue; } let (_status, location) = inventory.status_and_location(uuid); - - let media_set_name = inventory.generate_media_set_name(&media_set_label.uuid, None)?; + let (media_set_name, media_set_ctime, media_set_uuid, seq_nr) = + match media_id.media_set_label { + Some(ref set) => ( + Some(inventory.generate_media_set_name(&set.uuid, None)?), + Some(set.ctime), + Some(set.uuid.clone()), + Some(set.seq_nr), + ), + None => (None, None, None, None), + }; list.push(MediaListEntry { uuid: media_id.label.uuid.clone(), label_text: media_id.label.label_text.clone(), ctime: media_id.label.ctime, - pool: Some(media_set_label.pool.clone()), + pool: media_id.pool(), location, status: MediaStatus::Unknown, catalog: catalogs.contains(uuid), expired: false, - media_set_ctime: Some(media_set_label.ctime), - media_set_uuid: Some(media_set_label.uuid.clone()), - media_set_name: Some(media_set_name), - seq_nr: Some(media_set_label.seq_nr), + media_set_ctime, + media_set_uuid, + media_set_name, + seq_nr, }); } diff --git a/src/tape/file_formats/mod.rs b/src/tape/file_formats/mod.rs index f80e2d90..0f983059 100644 --- a/src/tape/file_formats/mod.rs +++ b/src/tape/file_formats/mod.rs @@ -131,6 +131,9 @@ pub struct MediaLabel { pub label_text: String, /// Creation time stamp pub ctime: i64, + /// The initial pool the media is reserved for + #[serde(skip_serializing_if = "Option::is_none")] + pub pool: Option, } #[derive(Serialize, Deserialize, Clone, Debug)] diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs index e01a7f91..b84bd346 100644 --- a/src/tape/inventory.rs +++ b/src/tape/inventory.rs @@ -64,6 +64,15 @@ pub struct MediaId { pub media_set_label: Option, } +impl MediaId { + pub fn pool(&self) -> Option { + if let Some(set) = &self.media_set_label { + return Some(set.pool.to_owned()); + } + self.label.pool.to_owned() + } +} + #[derive(Serialize, Deserialize)] struct MediaStateEntry { id: MediaId, @@ -249,11 +258,12 @@ impl Inventory { /// /// Returns (pool, is_empty) pub fn lookup_media_pool(&self, uuid: &Uuid) -> Option<(&str, bool)> { - let pool = self.map.get(uuid)?; - pool.id - .media_set_label - .as_ref() - .map(|media_set| (media_set.pool.as_str(), media_set.unassigned())) + let media_id = &self.map.get(uuid)?.id; + match (&media_id.label.pool, &media_id.media_set_label) { + (_, Some(media_set)) => Some((media_set.pool.as_str(), media_set.unassigned())), + (Some(pool), None) => Some((pool.as_str(), true)), + (None, None) => None, + } } /// List all media assigned to the pool @@ -261,18 +271,16 @@ impl Inventory { let mut list = Vec::new(); for entry in self.map.values() { - match entry.id.media_set_label { - Some(ref set) if set.pool == pool => { - let id = match set.unassigned() { - true => MediaId { - label: entry.id.label.clone(), - media_set_label: None, - }, - false => entry.id.clone(), - }; - list.push(id); + if entry.id.pool().as_deref() == Some(pool) { + match entry.id.media_set_label { + Some(ref set) if set.unassigned() => list.push(MediaId { + label: entry.id.label.clone(), + media_set_label: None, + }), + _ => { + list.push(entry.id.clone()); + } } - _ => continue, // not assigned to any pool or belongs to another pool } } @@ -294,7 +302,7 @@ impl Inventory { pub fn list_unassigned_media(&self) -> Vec { self.map .values() - .filter_map(|entry| match entry.id.media_set_label { + .filter_map(|entry| match entry.id.pool() { None => Some(entry.id.clone()), _ => None, }) @@ -553,6 +561,7 @@ impl Inventory { label_text: label_text.to_string(), uuid: Uuid::generate(), ctime, + pool: None, }; let uuid = label.uuid.clone(); @@ -575,16 +584,15 @@ impl Inventory { label_text: label_text.to_string(), uuid: Uuid::generate(), ctime, + pool: Some(pool.to_string()), }; let uuid = label.uuid.clone(); - let set = MediaSetLabel::new_unassigned(pool, ctime); - self.store( MediaId { label, - media_set_label: Some(set), + media_set_label: None, }, false, ) @@ -599,6 +607,7 @@ impl Inventory { label_text: label_text.to_string(), uuid: Uuid::generate(), ctime, + pool: Some(set.pool.clone()), }; let uuid = label.uuid.clone(); diff --git a/src/tape/media_pool.rs b/src/tape/media_pool.rs index e1bba9ab..afb27cab 100644 --- a/src/tape/media_pool.rs +++ b/src/tape/media_pool.rs @@ -205,13 +205,9 @@ impl MediaPool { Some(media_id) => media_id.clone(), }; - if let Some(ref set) = media_id.media_set_label { - if set.pool != self.name { - bail!( - "media does not belong to pool ({} != {})", - set.pool, - self.name - ); + if let Some(pool) = media_id.pool() { + if pool != self.name { + bail!("media does not belong to pool ({} != {})", pool, self.name); } } -- 2.30.2