* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal