* [pbs-devel] [PATCH proxmox-backup 1/2] tape: refactor uuid of empty media set into constant
@ 2022-11-29 10:51 Dominik Csapak
2022-11-29 10:51 ` [pbs-devel] [PATCH proxmox-backup 2/2] tape: inventory: skip empty tapes Dominik Csapak
2022-11-29 12:03 ` [pbs-devel] [PATCH proxmox-backup 1/2] tape: refactor uuid of empty media set into constant Thomas Lamprecht
0 siblings, 2 replies; 3+ messages in thread
From: Dominik Csapak @ 2022-11-29 10:51 UTC (permalink / raw)
To: pbs-devel
so that we have a easily recognizable name for it and can see
instantly what it does
make the 'let is_empty' unnecessary as it's clear now what the check is
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/tape/drive.rs | 8 ++++----
src/api2/tape/media.rs | 5 ++---
src/tape/inventory.rs | 16 +++++++++-------
src/tape/media_pool.rs | 4 ++--
4 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 107bcfd8..4bb9ade9 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -42,7 +42,7 @@ use crate::{
},
file_formats::{MediaLabel, MediaSetLabel},
lock_media_pool, lock_media_set, lock_unassigned_media_pool, Inventory, MediaCatalog,
- MediaId, TAPE_STATUS_DIR,
+ MediaId, EMPTY_MEDIA_SET_UUID, TAPE_STATUS_DIR,
},
};
@@ -528,7 +528,7 @@ fn write_media_label(
label.label_text,
pool
);
- let set = MediaSetLabel::with_data(pool, [0u8; 16].into(), 0, label.ctime, None);
+ let set = MediaSetLabel::with_data(pool, EMPTY_MEDIA_SET_UUID.into(), 0, label.ctime, None);
drive.write_media_set_label(&set, None)?;
@@ -575,7 +575,7 @@ fn write_media_label(
if let Some(ref pool) = pool {
match info.media_set_label {
Some(set) => {
- if set.uuid != [0u8; 16].into() {
+ if set.uuid != EMPTY_MEDIA_SET_UUID.into() {
bail!("verify media set label failed - got wrong set uuid");
}
if &set.pool != pool {
@@ -1301,7 +1301,7 @@ pub fn catalog_media(
return Ok(());
}
Some(ref set) => {
- if set.uuid.as_ref() == [0u8; 16] {
+ if set.uuid.as_ref() == EMPTY_MEDIA_SET_UUID {
// media is empty
task_log!(worker, "media is empty");
let _lock = lock_unassigned_media_pool(TAPE_STATUS_DIR)?;
diff --git a/src/api2/tape/media.rs b/src/api2/tape/media.rs
index ed0105b0..08c60615 100644
--- a/src/api2/tape/media.rs
+++ b/src/api2/tape/media.rs
@@ -15,7 +15,7 @@ use pbs_config::CachedUserInfo;
use crate::tape::{
changer::update_online_status, media_catalog_snapshot_list, Inventory, MediaCatalog, MediaPool,
- TAPE_STATUS_DIR,
+ EMPTY_MEDIA_SET_UUID, TAPE_STATUS_DIR,
};
#[api(
@@ -336,8 +336,7 @@ pub fn destroy_media(label_text: String, force: Option<bool>) -> Result<(), Erro
if !force {
if let Some(ref set) = media_id.media_set_label {
- let is_empty = set.uuid.as_ref() == [0u8; 16];
- if !is_empty {
+ if set.uuid.as_ref() != EMPTY_MEDIA_SET_UUID {
bail!(
"media '{}' contains data (please use 'force' flag to remove.",
label_text
diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs
index 9c5887d7..6a6056b9 100644
--- a/src/tape/inventory.rs
+++ b/src/tape/inventory.rs
@@ -54,6 +54,8 @@ use crate::tape::{
MediaCatalog, MediaSet, TAPE_STATUS_DIR,
};
+pub const EMPTY_MEDIA_SET_UUID: [u8; 16] = [0u8; 16];
+
/// Unique Media Identifier
///
/// This combines the label and media set label.
@@ -185,7 +187,7 @@ impl Inventory {
// do not overwrite unsaved pool assignments
if media_id.media_set_label.is_none() {
if let Some(ref set) = previous.id.media_set_label {
- if set.uuid.as_ref() == [0u8; 16] {
+ if set.uuid.as_ref() == EMPTY_MEDIA_SET_UUID {
media_id.media_set_label = Some(set.clone());
}
}
@@ -255,7 +257,7 @@ impl Inventory {
match entry.id.media_set_label {
None => None, // not assigned to any pool
Some(ref set) => {
- let is_empty = set.uuid.as_ref() == [0u8; 16];
+ let is_empty = set.uuid.as_ref() == EMPTY_MEDIA_SET_UUID;
Some((&set.pool, is_empty))
}
}
@@ -275,7 +277,7 @@ impl Inventory {
continue; // belong to another pool
}
- if set.uuid.as_ref() == [0u8; 16] {
+ if set.uuid.as_ref() == EMPTY_MEDIA_SET_UUID {
list.push(MediaId {
label: entry.id.label.clone(),
media_set_label: None,
@@ -298,7 +300,7 @@ impl Inventory {
match entry.id.media_set_label {
None => continue, // not assigned to any pool
Some(ref set) => {
- if set.uuid.as_ref() != [0u8; 16] {
+ if set.uuid.as_ref() != EMPTY_MEDIA_SET_UUID {
list.push(entry.id.clone());
}
}
@@ -410,7 +412,7 @@ impl Inventory {
.map
.values()
.filter_map(|entry| entry.id.media_set_label.as_ref())
- .filter(|set| set.pool == pool && set.uuid.as_ref() != [0u8; 16]);
+ .filter(|set| set.pool == pool && set.uuid.as_ref() != EMPTY_MEDIA_SET_UUID);
for set in set_list {
match last_set {
@@ -435,7 +437,7 @@ impl Inventory {
.map
.values()
.filter_map(|entry| entry.id.media_set_label.as_ref())
- .filter(|set| set.pool == pool && set.uuid.as_ref() != [0u8; 16]);
+ .filter(|set| set.pool == pool && set.uuid.as_ref() != EMPTY_MEDIA_SET_UUID);
for set in set_list {
if set.uuid != uuid && set.ctime >= ctime {
@@ -600,7 +602,7 @@ impl Inventory {
let uuid = label.uuid.clone();
- let set = MediaSetLabel::with_data(pool, [0u8; 16].into(), 0, ctime, None);
+ let set = MediaSetLabel::with_data(pool, EMPTY_MEDIA_SET_UUID.into(), 0, ctime, None);
self.store(
MediaId {
diff --git a/src/tape/media_pool.rs b/src/tape/media_pool.rs
index eb477885..e37fa47c 100644
--- a/src/tape/media_pool.rs
+++ b/src/tape/media_pool.rs
@@ -22,7 +22,7 @@ use pbs_config::BackupLockGuard;
use crate::tape::{
file_formats::{MediaLabel, MediaSetLabel},
lock_media_pool, lock_media_set, lock_unassigned_media_pool, Inventory, MediaCatalog, MediaId,
- MediaSet,
+ MediaSet, EMPTY_MEDIA_SET_UUID,
};
/// Media Pool
@@ -181,7 +181,7 @@ impl MediaPool {
// should never trigger
return (MediaStatus::Unknown, location); // belong to another pool
}
- if set.uuid.as_ref() == [0u8; 16] {
+ if set.uuid.as_ref() == EMPTY_MEDIA_SET_UUID {
// not assigned to any pool
return (MediaStatus::Writable, location);
}
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread* [pbs-devel] [PATCH proxmox-backup 2/2] tape: inventory: skip empty tapes
2022-11-29 10:51 [pbs-devel] [PATCH proxmox-backup 1/2] tape: refactor uuid of empty media set into constant Dominik Csapak
@ 2022-11-29 10:51 ` Dominik Csapak
2022-11-29 12:03 ` [pbs-devel] [PATCH proxmox-backup 1/2] tape: refactor uuid of empty media set into constant Thomas Lamprecht
1 sibling, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2022-11-29 10:51 UTC (permalink / raw)
To: pbs-devel
tapes that are labeled into a pool but are empty, belong to the special
empty media-set. these will never have a catalog on them, so skip them
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/tape/drive.rs | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 4bb9ade9..b75fb265 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -956,6 +956,9 @@ pub fn update_inventory(
ref pool, ref uuid, ..
}) = media_id.media_set_label
{
+ if uuid.as_ref() == EMPTY_MEDIA_SET_UUID {
+ continue;
+ }
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)?;
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 1/2] tape: refactor uuid of empty media set into constant
2022-11-29 10:51 [pbs-devel] [PATCH proxmox-backup 1/2] tape: refactor uuid of empty media set into constant Dominik Csapak
2022-11-29 10:51 ` [pbs-devel] [PATCH proxmox-backup 2/2] tape: inventory: skip empty tapes Dominik Csapak
@ 2022-11-29 12:03 ` Thomas Lamprecht
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2022-11-29 12:03 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
Am 29/11/2022 um 11:51 schrieb Dominik Csapak:
> so that we have a easily recognizable name for it and can see
> instantly what it does
>
> make the 'let is_empty' unnecessary as it's clear now what the check is
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> src/api2/tape/drive.rs | 8 ++++----
> src/api2/tape/media.rs | 5 ++---
> src/tape/inventory.rs | 16 +++++++++-------
> src/tape/media_pool.rs | 4 ++--
> 4 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
> index 107bcfd8..4bb9ade9 100644
> --- a/src/api2/tape/drive.rs
> +++ b/src/api2/tape/drive.rs
> @@ -42,7 +42,7 @@ use crate::{
> },
> file_formats::{MediaLabel, MediaSetLabel},
> lock_media_pool, lock_media_set, lock_unassigned_media_pool, Inventory, MediaCatalog,
> - MediaId, TAPE_STATUS_DIR,
> + MediaId, EMPTY_MEDIA_SET_UUID, TAPE_STATUS_DIR,
> },
> };
>
> @@ -528,7 +528,7 @@ fn write_media_label(
> label.label_text,
> pool
> );
> - let set = MediaSetLabel::with_data(pool, [0u8; 16].into(), 0, label.ctime, None);
> + let set = MediaSetLabel::with_data(pool, EMPTY_MEDIA_SET_UUID.into(), 0, label.ctime, None);
>
> drive.write_media_set_label(&set, None)?;
>
> @@ -575,7 +575,7 @@ fn write_media_label(
> if let Some(ref pool) = pool {
> match info.media_set_label {
> Some(set) => {
> - if set.uuid != [0u8; 16].into() {
> + if set.uuid != EMPTY_MEDIA_SET_UUID.into() {
Did not looked to closely, but why not impl a method on MediaSetLabel instead,
avoiding leaking out of that internal detail in the first place?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-29 12:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-29 10:51 [pbs-devel] [PATCH proxmox-backup 1/2] tape: refactor uuid of empty media set into constant Dominik Csapak
2022-11-29 10:51 ` [pbs-devel] [PATCH proxmox-backup 2/2] tape: inventory: skip empty tapes Dominik Csapak
2022-11-29 12:03 ` [pbs-devel] [PATCH proxmox-backup 1/2] tape: refactor uuid of empty media set into constant Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox