[pbs-devel] [PATCH proxmox-backup] tape: move 'eject-before-unload' to a plain changer config option

[pbs-devel] [PATCH proxmox-backup] tape: move 'eject-before-unload' to a plain changer config option

[Dominik Csapak]

instead of having it in a property string. For now this should be fine,
and if we need many more such options, we can still move them into a
property string if we want.

Also update the cli command in the docs on how to set it now.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 docs/tape-backup.rst              |  2 +-
 pbs-api-types/src/tape/changer.rs | 34 ++++++-------------------------
 src/api2/config/changer.rs        | 12 +++++------
 src/bin/proxmox_tape/changer.rs   |  2 +-
 src/tape/changer/mod.rs           | 10 ++-------
 5 files changed, 16 insertions(+), 44 deletions(-)

diff --git a/docs/tape-backup.rst b/docs/tape-backup.rst
index c528d578..b890d81a 100644
--- a/docs/tape-backup.rst
+++ b/docs/tape-backup.rst
@@ -367,7 +367,7 @@ You can set these options with `proxmox-tape` like this:
 
 .. code-block:: console
 
- # proxmox-tape changer update sl3 --options eject-before-unload=true
+ # proxmox-tape changer update sl3 --eject-before-unload true
 
 
 .. _tape_drive_config:
diff --git a/pbs-api-types/src/tape/changer.rs b/pbs-api-types/src/tape/changer.rs
index 9e36b12e..df3823cf 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, ApiType, ArraySchema, IntegerSchema, Schema, StringSchema, Updater,
+    api, ApiStringFormat, ArraySchema, IntegerSchema, Schema, StringSchema, Updater,
 };
 
 use crate::{OptionalDeviceIdentification, PROXMOX_SAFE_ID_FORMAT};
@@ -39,29 +39,6 @@ Import/Export, i.e. any media in those slots are considered to be
 .format(&ApiStringFormat::PropertyString(&SLOT_ARRAY_SCHEMA))
 .schema();
 
-#[api(
-    properties: {
-        "eject-before-unload": {
-            optional: true,
-            default: false,
-        },
-    },
-)]
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-/// Options for Changers
-pub struct ChangerOptions {
-    #[serde(skip_serializing_if = "Option::is_none")]
-    /// if set to true, tapes are ejected manually before unloading
-    pub eject_before_unload: Option<bool>,
-}
-
-pub const CHANGER_OPTIONS_STRING_SCHEMA: Schema = StringSchema::new("Changer options")
-    .format(&ApiStringFormat::PropertyString(
-        &ChangerOptions::API_SCHEMA,
-    ))
-    .schema();
-
 #[api(
     properties: {
         name: {
@@ -74,10 +51,10 @@ pub const CHANGER_OPTIONS_STRING_SCHEMA: Schema = StringSchema::new("Changer opt
             schema: EXPORT_SLOT_LIST_SCHEMA,
             optional: true,
         },
-        options: {
+        "eject-before-unload": {
             optional: true,
-            schema: CHANGER_OPTIONS_STRING_SCHEMA,
-        },
+            default: false,
+        }
     },
 )]
 #[derive(Serialize, Deserialize, Updater)]
@@ -90,7 +67,8 @@ pub struct ScsiTapeChanger {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub export_slots: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub options: Option<String>,
+    /// if set to true, tapes are ejected manually before unloading
+    pub eject_before_unload: Option<bool>,
 }
 
 #[api(
diff --git a/src/api2/config/changer.rs b/src/api2/config/changer.rs
index bff9ed9e..db0ea14a 100644
--- a/src/api2/config/changer.rs
+++ b/src/api2/config/changer.rs
@@ -138,8 +138,8 @@ pub fn list_changers(
 pub enum DeletableProperty {
     /// Delete export-slots.
     ExportSlots,
-    /// Delete options.
-    Options,
+    /// Delete eject-before-unload.
+    EjectBeforeUnload,
 }
 
 #[api(
@@ -196,8 +196,8 @@ pub fn update_changer(
                 DeletableProperty::ExportSlots => {
                     data.export_slots = None;
                 }
-                DeletableProperty::Options => {
-                    data.options = None;
+                DeletableProperty::EjectBeforeUnload => {
+                    data.eject_before_unload = None;
                 }
             }
         }
@@ -227,8 +227,8 @@ pub fn update_changer(
         }
     }
 
-    if let Some(options) = update.options {
-        data.options = Some(options);
+    if let Some(eject_before_unload) = update.eject_before_unload {
+        data.eject_before_unload = Some(eject_before_unload);
     }
 
     config.set_data(&name, "changer", &data)?;
diff --git a/src/bin/proxmox_tape/changer.rs b/src/bin/proxmox_tape/changer.rs
index a8bff173..8deff27f 100644
--- a/src/bin/proxmox_tape/changer.rs
+++ b/src/bin/proxmox_tape/changer.rs
@@ -161,7 +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("eject-before-unload"))
         .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 9d90e29d..18ea0f46 100644
--- a/src/tape/changer/mod.rs
+++ b/src/tape/changer/mod.rs
@@ -4,7 +4,6 @@ pub mod mtx;
 
 mod online_status_map;
 pub use online_status_map::*;
-use proxmox_schema::ApiType;
 
 use std::path::PathBuf;
 
@@ -12,7 +11,7 @@ use anyhow::{bail, Error};
 
 use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
 
-use pbs_api_types::{ChangerOptions, LtoTapeDrive, ScsiTapeChanger};
+use pbs_api_types::{LtoTapeDrive, ScsiTapeChanger};
 
 use pbs_tape::{linux_list_drives::open_lto_tape_device, sg_pt_changer, ElementStatus, MtxStatus};
 
@@ -428,12 +427,7 @@ 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.unwrap_or(false) {
+        if self.config.eject_before_unload.unwrap_or(false) {
             let file = open_lto_tape_device(&self.drive.path)?;
             let mut handle = LtoTapeHandle::new(file)?;
 
-- 
2.30.2

[pbs-devel] applied: [PATCH proxmox-backup] tape: move 'eject-before-unload' to a plain changer config option

[Dietmar Maurer]

applied, thanks!