* [pbs-devel] [PATCH proxmox-backup 1/3] tape: inventory: don't skip unassigned tapes completely
@ 2023-02-23 11:59 Dominik Csapak
2023-02-23 11:59 ` [pbs-devel] [PATCH proxmox-backup 2/3] tape: initial WORM support Dominik Csapak
2023-02-23 11:59 ` [pbs-devel] [PATCH proxmox-backup 3/3] docs: add WORM tape documentation Dominik Csapak
0 siblings, 2 replies; 4+ messages in thread
From: Dominik Csapak @ 2023-02-23 11:59 UTC (permalink / raw)
To: pbs-devel
since commit 139acf37 ("tape: inventory: skip unassigned tapes")
we skip unassigned tapes (special all-zero media-set uuid) when we look
for a catalog. We accidentally skipped storing it in the inventory
completely, which means we never inventoried completely empty tapes.
to fix that, simply move the check below the inserting in the inventory
Fixes: 139acf37 ("tape: inventory: skip unassigned tapes")
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/tape/drive.rs | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 16ea5f98..a66c999c 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -953,14 +953,15 @@ pub fn update_inventory(
);
if let Some(ref set) = media_id.media_set_label {
- if set.unassigned() {
- continue;
- }
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)?;
+ if set.unassigned() {
+ continue;
+ }
+
if catalog {
let media_set = inventory.compute_media_set_members(&set.uuid)?;
if let Err(err) = fast_catalog_restore(
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/3] tape: initial WORM support
2023-02-23 11:59 [pbs-devel] [PATCH proxmox-backup 1/3] tape: inventory: don't skip unassigned tapes completely Dominik Csapak
@ 2023-02-23 11:59 ` Dominik Csapak
2023-02-28 9:56 ` Dietmar Maurer
2023-02-23 11:59 ` [pbs-devel] [PATCH proxmox-backup 3/3] docs: add WORM tape documentation Dominik Csapak
1 sibling, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2023-02-23 11:59 UTC (permalink / raw)
To: pbs-devel
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@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)?;
+ 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(),
};
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 {
+ 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);
}
}
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 3/3] docs: add WORM tape documentation
2023-02-23 11:59 [pbs-devel] [PATCH proxmox-backup 1/3] tape: inventory: don't skip unassigned tapes completely Dominik Csapak
2023-02-23 11:59 ` [pbs-devel] [PATCH proxmox-backup 2/3] tape: initial WORM support Dominik Csapak
@ 2023-02-23 11:59 ` Dominik Csapak
1 sibling, 0 replies; 4+ messages in thread
From: Dominik Csapak @ 2023-02-23 11:59 UTC (permalink / raw)
To: pbs-devel
a short section about how to use WORM tapes (since we currently don't
handle them differently than normal tapes)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
docs/tape-backup.rst | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/docs/tape-backup.rst b/docs/tape-backup.rst
index fbc72b67..a3a62818 100644
--- a/docs/tape-backup.rst
+++ b/docs/tape-backup.rst
@@ -1050,6 +1050,21 @@ This command does the following:
- unload the cleaning tape (to slot 3)
+WORM Tapes
+----------
+
+WORM (write once, read many) tapes are special cartridges that cannot be deleted
+or overwritten, which may be useful for legal or protection purposes.
+
+Since they cannot be overwritten or deleted, if you want to use them, you must
+use a media pool with a retention policy of `keep`, otherwise a backup job
+potentially fails when trying to erase or overwrite it.
+
+Proxmox Backup Server does not handle WORM tapes differently, so it is
+recommended to use a different naming scheme, use only seperate media pools
+and not mix WORM and non-WORM tapes in a media pool together (since it may
+lead to confusion about which tapes are not overwritable).
+
Example Setups
--------------
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/3] tape: initial WORM support
2023-02-23 11:59 ` [pbs-devel] [PATCH proxmox-backup 2/3] tape: initial WORM support Dominik Csapak
@ 2023-02-28 9:56 ` Dietmar Maurer
0 siblings, 0 replies; 4+ messages in thread
From: Dietmar Maurer @ 2023-02-28 9:56 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
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@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);
> }
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-28 9:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-23 11:59 [pbs-devel] [PATCH proxmox-backup 1/3] tape: inventory: don't skip unassigned tapes completely Dominik Csapak
2023-02-23 11:59 ` [pbs-devel] [PATCH proxmox-backup 2/3] tape: initial WORM support Dominik Csapak
2023-02-28 9:56 ` Dietmar Maurer
2023-02-23 11:59 ` [pbs-devel] [PATCH proxmox-backup 3/3] docs: add WORM tape documentation Dominik Csapak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox