* [pbs-devel] [PATCH proxmox-backup 2/2] fix #4904: tape changer: add option to eject before unload
2023-12-07 12:51 [pbs-devel] [PATCH proxmox-backup 1/2] tape: changer: save whole LtoTapeDrive config in MtxMediaChanger Dominik Csapak
@ 2023-12-07 12:51 ` Dominik Csapak
2023-12-12 13:38 ` [pbs-devel] applied: " Dietmar Maurer
2023-12-12 13:38 ` [pbs-devel] applied: [PATCH proxmox-backup 1/2] tape: changer: save whole LtoTapeDrive config in MtxMediaChanger Dietmar Maurer
1 sibling, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2023-12-07 12:51 UTC (permalink / raw)
To: pbs-devel
some tape libraries need the tape being ejected from the drive before
doing an unload. Since we cannot easily detect if that's the case,
introduce an 'eject_before_unload' option.
Instead of just adding a bool flag to the config, add a new 'options'
property string where we can put such niche options similar to how we
handle the datastore tuning options.
Extend the LtoTapeHandle with 'medium_present' which just uses a
TEST UNIT READY command to check for present medium, so we don't
try to eject an already ejected tape.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
pbs-api-types/src/tape/changer.rs | 28 +++++++++++++++++++++++++++-
src/api2/config/changer.rs | 9 +++++++++
src/bin/proxmox_tape/changer.rs | 1 +
src/tape/changer/mod.rs | 21 +++++++++++++++++++--
src/tape/drive/lto/mod.rs | 5 +++++
5 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/pbs-api-types/src/tape/changer.rs b/pbs-api-types/src/tape/changer.rs
index c9c7fcaa..e3cf27c1 100644
--- a/pbs-api-types/src/tape/changer.rs
+++ b/pbs-api-types/src/tape/changer.rs
@@ -3,7 +3,7 @@
use serde::{Deserialize, Serialize};
use proxmox_schema::{
- api, ApiStringFormat, ArraySchema, IntegerSchema, Schema, StringSchema, Updater,
+ api, ApiStringFormat, ApiType, ArraySchema, IntegerSchema, Schema, StringSchema, Updater,
};
use crate::{OptionalDeviceIdentification, PROXMOX_SAFE_ID_FORMAT};
@@ -39,6 +39,26 @@ Import/Export, i.e. any media in those slots are considered to be
.format(&ApiStringFormat::PropertyString(&SLOT_ARRAY_SCHEMA))
.schema();
+fn is_false(b: &bool) -> bool {
+ !b
+}
+
+#[api]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Options for Changers
+pub struct ChangerOptions {
+ #[serde(default, skip_serializing_if = "is_false")]
+ /// if set to true, tapes are ejected manually before unloading
+ pub eject_before_unload: bool,
+}
+
+pub const CHANGER_OPTIONS_STRING_SCHEMA: Schema = StringSchema::new("Changer options")
+ .format(&ApiStringFormat::PropertyString(
+ &ChangerOptions::API_SCHEMA,
+ ))
+ .schema();
+
#[api(
properties: {
name: {
@@ -51,6 +71,10 @@ Import/Export, i.e. any media in those slots are considered to be
schema: EXPORT_SLOT_LIST_SCHEMA,
optional: true,
},
+ options: {
+ optional: true,
+ schema: CHANGER_OPTIONS_STRING_SCHEMA,
+ },
},
)]
#[derive(Serialize, Deserialize, Updater)]
@@ -62,6 +86,8 @@ pub struct ScsiTapeChanger {
pub path: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub export_slots: Option<String>,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub options: Option<String>,
}
#[api(
diff --git a/src/api2/config/changer.rs b/src/api2/config/changer.rs
index 01908e3b..bff9ed9e 100644
--- a/src/api2/config/changer.rs
+++ b/src/api2/config/changer.rs
@@ -138,6 +138,8 @@ pub fn list_changers(
pub enum DeletableProperty {
/// Delete export-slots.
ExportSlots,
+ /// Delete options.
+ Options,
}
#[api(
@@ -194,6 +196,9 @@ pub fn update_changer(
DeletableProperty::ExportSlots => {
data.export_slots = None;
}
+ DeletableProperty::Options => {
+ data.options = None;
+ }
}
}
}
@@ -222,6 +227,10 @@ pub fn update_changer(
}
}
+ if let Some(options) = update.options {
+ data.options = Some(options);
+ }
+
config.set_data(&name, "changer", &data)?;
pbs_config::drive::save_config(&config)?;
diff --git a/src/bin/proxmox_tape/changer.rs b/src/bin/proxmox_tape/changer.rs
index d1ddb5e0..a8bff173 100644
--- a/src/bin/proxmox_tape/changer.rs
+++ b/src/bin/proxmox_tape/changer.rs
@@ -161,6 +161,7 @@ fn get_config(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error
let options = default_table_format_options()
.column(ColumnConfig::new("name"))
.column(ColumnConfig::new("path"))
+ .column(ColumnConfig::new("options"))
.column(ColumnConfig::new("export-slots"));
format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
diff --git a/src/tape/changer/mod.rs b/src/tape/changer/mod.rs
index bf0a15f4..df63f6f8 100644
--- a/src/tape/changer/mod.rs
+++ b/src/tape/changer/mod.rs
@@ -4,6 +4,7 @@ pub mod mtx;
mod online_status_map;
pub use online_status_map::*;
+use proxmox_schema::ApiType;
use std::path::PathBuf;
@@ -11,9 +12,11 @@ use anyhow::{bail, Error};
use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
-use pbs_api_types::{LtoTapeDrive, ScsiTapeChanger};
+use pbs_api_types::{ChangerOptions, LtoTapeDrive, ScsiTapeChanger};
-use pbs_tape::{sg_pt_changer, ElementStatus, MtxStatus};
+use pbs_tape::{linux_list_drives::open_lto_tape_device, sg_pt_changer, ElementStatus, MtxStatus};
+
+use crate::tape::drive::{LtoTapeHandle, TapeDriver};
/// Interface to SCSI changer devices
pub trait ScsiMediaChange {
@@ -425,6 +428,20 @@ impl MediaChange for MtxMediaChanger {
}
fn unload_media(&mut self, target_slot: Option<u64>) -> Result<MtxStatus, Error> {
+ let options: ChangerOptions = serde_json::from_value(
+ ChangerOptions::API_SCHEMA
+ .parse_property_string(self.config.options.as_deref().unwrap_or_default())?,
+ )?;
+
+ if options.eject_before_unload {
+ let file = open_lto_tape_device(&self.drive.path)?;
+ let mut handle = LtoTapeHandle::new(file)?;
+
+ if handle.medium_present() {
+ handle.eject_media()?;
+ }
+ }
+
if let Some(target_slot) = target_slot {
self.config.unload(target_slot, self.drive_number())
} else {
diff --git a/src/tape/drive/lto/mod.rs b/src/tape/drive/lto/mod.rs
index 2c3a5a63..955dbdf7 100644
--- a/src/tape/drive/lto/mod.rs
+++ b/src/tape/drive/lto/mod.rs
@@ -162,6 +162,11 @@ impl LtoTapeHandle {
.set_medium_removal(true)
.map_err(|err| format_err!("unlock door failed - {}", err))
}
+
+ /// Returns if a medium is present
+ pub fn medium_present(&mut self) -> bool {
+ self.sg_tape.test_unit_ready().is_ok()
+ }
}
impl TapeDriver for LtoTapeHandle {
--
2.39.2
^ permalink raw reply [flat|nested] 4+ messages in thread