[pbs-devel] [PATCH proxmox-backup 2/3] tape: initial WORM support
Dietmar Maurer
dietmar at proxmox.com
Tue Feb 28 10:56:08 CET 2023
comments inline
On 2/23/23 12:59, Dominik Csapak wrote:
> 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 <d.csapak at proxmox.com>
> ---
> src/api2/tape/drive.rs | 101 +++++++++++++++++------------------
> 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, 109 insertions(+), 94 deletions(-)
>
> diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
> index a66c999c..cf4d6190 100644
> --- a/src/api2/tape/drive.rs
> +++ b/src/api2/tape/drive.rs
> @@ -346,19 +346,21 @@ 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 _pool_lock = if let Some(pool) = media_id.pool() {
> + lock_media_pool(TAPE_STATUS_DIR, &pool)?
> + } else {
> + lock_unassigned_media_pool(TAPE_STATUS_DIR)?
> + };
> +
> + if let Some(MediaSetLabel { ref uuid, .. }) = media_id.media_set_label {
> 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(TAPE_STATUS_DIR)?;
> MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
> inventory.remove_media(&media_id.label.uuid)?;
> };
> + inventory.remove_media(&media_id.label.uuid)?;
above refactoring looks wrong to me! I.e. you call remove_media twice...
> + drop(_pool_lock);
>
> handle.format_media(fast.unwrap_or(true))?;
> }
> @@ -504,6 +506,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 +523,31 @@ fn write_media_label(
> pool: Option<String>,
> ) -> 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 +557,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 +566,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 +675,7 @@ pub async fn read_label(drive: String, inventorize: Option<bool>) -> Result<Medi
> label_text: media_id.label.label_text.clone(),
> media_set_ctime: None,
> media_set_uuid: None,
> - pool: None,
> + pool: media_id.label.pool.clone(),
> seq_nr: None,
> uuid: media_id.label.uuid.clone(),
> }
> @@ -695,19 +684,20 @@ pub async fn read_label(drive: String, inventorize: Option<bool>) -> Result<Medi
> if let Some(true) = inventorize {
> 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 _pool_lock = if let Some(pool) = media_id.pool() {
> + lock_media_pool(TAPE_STATUS_DIR, &pool)?
> + } else {
> + lock_unassigned_media_pool(TAPE_STATUS_DIR)?
> + };
> +
> + if let Some(MediaSetLabel { ref uuid, .. }) = media_id.media_set_label {
> 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(TAPE_STATUS_DIR)?;
> MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
> - inventory.store(media_id, false)?;
> };
> +
> + inventory.store(media_id, false)?;
> }
>
> Ok(label)
> @@ -952,8 +942,13 @@ pub fn update_inventory(
> media_id.label.uuid
> );
>
> + 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)?
> + };
> +
> if let Some(ref set) = media_id.media_set_label {
> - let _pool_lock = lock_media_pool(TAPE_STATUS_DIR, &set.pool)?;
> let _lock = lock_media_set(TAPE_STATUS_DIR, &set.uuid, None)?;
> MediaCatalog::destroy_unrelated_catalog(TAPE_STATUS_DIR, &media_id)?;
> inventory.store(media_id.clone(), false)?;
> @@ -977,7 +972,6 @@ pub fn update_inventory(
> }
> }
> } else {
> - let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?;
> MediaCatalog::destroy(TAPE_STATUS_DIR, &media_id.label.uuid)?;
> inventory.store(media_id, false)?;
> };
> @@ -1113,6 +1107,7 @@ fn barcode_label_media_worker(
> label_text: label_text.to_string(),
> uuid: Uuid::generate(),
> ctime,
> + pool: pool.clone(),
> };catzalog
>
> write_media_label(worker.clone(), &mut drive, label, pool.clone())?
> @@ -1296,7 +1291,11 @@ pub fn catalog_media(
> 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(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<String>,
> }
>
> #[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<MediaSetLabel>,
> }
>
> +impl MediaId {
> + pub fn pool(&self) -> Option<String> {
> + 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 {
What if media_set.pool != label.pool??
> + 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<MediaId> {
> 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);
> }
> }
>
More information about the pbs-devel
mailing list