[pbs-devel] [PATCH proxmox-backup 01/11] tape/drive: add test_device_path_lock

[pbs-devel] [PATCH proxmox-backup 01/11] tape/drive: add test_device_path_lock

[Dominik Csapak]

this makes it possible to detect if the drive was locked in a
non-blocking manner

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/drive/mod.rs | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
index e44e0e36..faa89953 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -496,3 +496,28 @@ fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, Error> {
 
     Ok(DeviceLockGuard(file))
 }
+
+// Same logic as lock_device_path, but uses a timeout of 0, making it
+// non-blocking, and returning if the file is locked or not
+fn test_device_path_lock(device_path: &str) -> Result<bool, Error> {
+
+    let lock_name = crate::tools::systemd::escape_unit(device_path, true);
+
+    let mut path = std::path::PathBuf::from("/var/lock");
+    path.push(lock_name);
+
+    let timeout = std::time::Duration::new(0, 0);
+    let mut file = std::fs::OpenOptions::new().create(true).append(true).open(path)?;
+    match proxmox::tools::fs::lock_file(&mut file, true, Some(timeout)) {
+        // file was not locked, continue
+        Ok(()) => {},
+        // file was locked, return true
+        Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => return Ok(true),
+        Err(err) => bail!("{}", err),
+    }
+
+    let backup_user = crate::backup::backup_user()?;
+    fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
+
+    Ok(false)
+}
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 02/11] tape/drive: add get/set status functions

[Dominik Csapak]

simply writes into/reads from a file in /run, we will use this
for writing the upid (or potential other states) per drive

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/drive/mod.rs | 56 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
index faa89953..4abb95de 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -29,7 +29,12 @@ use proxmox::{
     tools::{
         Uuid,
         io::ReadExt,
-        fs::fchown,
+        fs::{
+            fchown,
+            file_read_optional_string,
+            replace_file,
+            CreateOptions,
+       }
     },
     api::section_config::SectionConfigData,
 };
@@ -453,7 +458,53 @@ pub fn lock_tape_device(
     config: &SectionConfigData,
     drive: &str,
 ) -> Result<DeviceLockGuard, Error> {
+    let path = tape_device_path(config, drive)?;
+    lock_device_path(&path)
+        .map_err(|err| format_err!("unable to lock drive '{}' - {}", drive, err))
+}
+
+/// Writes the given state for the specified drive
+///
+/// This function does not lock, so make sure the drive is locked
+pub fn set_tape_device_state(
+    drive: &str,
+    state: &str,
+) -> Result<(), Error> {
+    let mut path = "/run/proxmox-backup/drive-state".to_string();
+    std::fs::create_dir_all(&path)?;
+    use std::fmt::Write;
+    write!(path, "/{}", drive)?;
+
+    let backup_user = crate::backup::backup_user()?;
+    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0644);
+    let options = CreateOptions::new()
+        .perm(mode)
+        .owner(backup_user.uid)
+        .group(backup_user.gid);
+
+    replace_file(path, state.as_bytes(), options)
+}
 
+/// Get the device state
+pub fn get_tape_device_state(
+    config: &SectionConfigData,
+    drive: &str,
+) -> Result<Option<String>, Error> {
+    let path = format!("/run/proxmox-backup/drive-state/{}", drive);
+    let state = file_read_optional_string(path)?;
+
+    let device_path = tape_device_path(config, drive)?;
+    if test_device_path_lock(&device_path)? {
+        Ok(state)
+    } else {
+        Ok(None)
+    }
+}
+
+fn tape_device_path(
+    config: &SectionConfigData,
+    drive: &str,
+) -> Result<String, Error> {
     match config.sections.get(drive) {
         Some((section_type_name, config)) => {
             let path = match section_type_name.as_ref() {
@@ -465,8 +516,7 @@ pub fn lock_tape_device(
                 }
                 _ => bail!("unknown drive type '{}' - internal error"),
             };
-            lock_device_path(&path)
-                .map_err(|err| format_err!("unable to lock drive '{}' - {}", drive, err))
+            Ok(path)
         }
         None => {
             bail!("no such drive '{}'", drive);
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 03/11] api2/tape/drive: add run_drive_worker wrapper

[Dominik Csapak]

a wrapper for locking, starting the worker and writing the state

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/drive.rs | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 8de8d39d..f4977612 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -1,7 +1,8 @@
+use std::panic::UnwindSafe;
 use std::path::Path;
 use std::sync::Arc;
 
-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Error};
 use serde_json::Value;
 
 use proxmox::{
@@ -62,11 +63,44 @@ use crate::{
             required_media_changer,
             open_drive,
             lock_tape_device,
+            set_tape_device_state,
         },
         changer::update_changer_online_status,
     },
 };
 
+fn run_drive_worker<F>(
+    rpcenv: &dyn RpcEnvironment,
+    drive: String,
+    worker_type: &str,
+    job_id: Option<String>,
+    f: F,
+) -> Result<String, Error>
+where
+    F: Send
+        + UnwindSafe
+        + 'static
+        + FnOnce(Arc<WorkerTask>, SectionConfigData) -> Result<(), Error>,
+{
+    // early check/lock before starting worker
+    let (config, _digest) = config::drive::config()?;
+    let lock_guard = lock_tape_device(&config, &drive)?;
+
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+    WorkerTask::new_thread(worker_type, job_id, auth_id, to_stdout, move |worker| {
+        let _lock_guard = lock_guard;
+        set_tape_device_state(&drive, &worker.upid().to_string())
+            .map_err(|err| format_err!("could not set tape device state: {}", err))?;
+
+        let result = f(worker, config);
+        set_tape_device_state(&drive, "")
+            .map_err(|err| format_err!("could not unset tape device state: {}", err))?;
+        result
+    })
+}
+
 #[api(
     input: {
         properties: {
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 04/11] api2/tape/drive: use 'run_drive_worker' where possible

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/drive.rs | 223 ++++++++++-------------------------------
 1 file changed, 52 insertions(+), 171 deletions(-)

diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index f4977612..7a6461b0 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -124,32 +124,20 @@ pub fn load_media(
     label_text: String,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
     let job_id = format!("{}:{}", drive, label_text);
 
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "load-media",
         Some(job_id),
-        auth_id,
-        to_stdout,
-        move |worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-
+        move |worker, config| {
             task_log!(worker, "loading media '{}' into drive '{}'", label_text, drive);
             let (mut changer, _) = required_media_changer(&config, &drive)?;
             changer.load_media(&label_text)?;
 
             Ok(())
-        }
+        },
     )?;
 
     Ok(upid_str.into())
@@ -241,29 +229,18 @@ pub fn unload(
     target_slot: Option<u64>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "unload-media",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-
+        move |worker, config| {
             task_log!(worker, "unloading media from drive '{}'", drive);
 
             let (mut changer, _) = required_media_changer(&config, &drive)?;
             changer.unload_media(target_slot)?;
             Ok(())
-        }
+        },
     )?;
 
     Ok(upid_str.into())
@@ -298,23 +275,12 @@ pub fn erase_media(
     label_text: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "erase-media",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-
+        move |worker, config| {
             if let Some(ref label) = label_text {
                 task_log!(worker, "try to load media '{}'", label);
                 if let Some((mut changer, _)) = media_changer(&config, &drive)? {
@@ -368,7 +334,7 @@ pub fn erase_media(
             }
 
             Ok(())
-        }
+        },
     )?;
 
     Ok(upid_str.into())
@@ -391,27 +357,16 @@ pub fn rewind(
     drive: String,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "rewind-media",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |_worker| {
-            let _lock_guard = lock_guard; // keep lock guard
+        move |_worker, config| {
             let mut drive = open_drive(&config, &drive)?;
             drive.rewind()?;
             Ok(())
-        }
+        },
     )?;
 
     Ok(upid_str.into())
@@ -434,32 +389,21 @@ pub fn eject_media(
     drive: String,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "eject-media",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |_worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-
-             if let Some((mut changer, _)) = media_changer(&config, &drive)? {
+        move |_worker, config| {
+            if let Some((mut changer, _)) = media_changer(&config, &drive)? {
                 changer.unload_media(None)?;
             } else {
                 let mut drive = open_drive(&config, &drive)?;
                 drive.eject_media()?;
             }
             Ok(())
-        })?;
+        },
+    )?;
 
     Ok(upid_str.into())
 }
@@ -495,9 +439,6 @@ pub fn label_media(
     label_text: String,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
     if let Some(ref pool) = pool {
         let (pool_config, _digest) = config::media_pool::config()?;
 
@@ -505,22 +446,12 @@ pub fn label_media(
             bail!("no such pool ('{}')", pool);
         }
     }
-
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "label-media",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-
+        move |worker, config| {
             let mut drive = open_drive(&config, &drive)?;
 
             drive.rewind()?;
@@ -545,7 +476,7 @@ pub fn label_media(
             };
 
             write_media_label(worker, &mut drive, label, pool)
-        }
+        },
     )?;
 
     Ok(upid_str.into())
@@ -757,24 +688,12 @@ pub fn clean_drive(
     drive: String,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "clean-drive",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-
+        move |worker, config| {
             let (mut changer, _changer_name) = required_media_changer(&config, &drive)?;
 
             worker.log("Starting drive clean");
@@ -784,7 +703,8 @@ pub fn clean_drive(
             worker.log("Drive cleaned sucessfully");
 
             Ok(())
-        })?;
+        },
+    )?;
 
     Ok(upid_str.into())
 }
@@ -891,24 +811,12 @@ pub fn update_inventory(
     read_all_labels: Option<bool>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "inventory-update",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-
+        move |worker, config| {
             let (mut changer, changer_name) = required_media_changer(&config, &drive)?;
 
             let label_text_list = changer.online_media_label_texts()?;
@@ -960,7 +868,7 @@ pub fn update_inventory(
                 changer.unload_media(None)?;
             }
             Ok(())
-        }
+        },
     )?;
 
     Ok(upid_str.into())
@@ -989,7 +897,6 @@ pub fn barcode_label_media(
     pool: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
     if let Some(ref pool) = pool {
         let (pool_config, _digest) = config::media_pool::config()?;
 
@@ -998,24 +905,12 @@ pub fn barcode_label_media(
         }
     }
 
-    let (drive_config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&drive_config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "barcode-label-media",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-            barcode_label_media_worker(worker, drive, &drive_config, pool)
-        }
+        move |worker, config| barcode_label_media_worker(worker, drive, &config, pool),
     )?;
 
     Ok(upid_str.into())
@@ -1027,7 +922,6 @@ fn barcode_label_media_worker(
     drive_config: &SectionConfigData,
     pool: Option<String>,
 ) -> Result<(), Error> {
-
     let (mut changer, changer_name) = required_media_changer(drive_config, &drive)?;
 
     let label_text_list = changer.online_media_label_texts()?;
@@ -1202,27 +1096,15 @@ pub fn catalog_media(
     verbose: Option<bool>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
     let verbose = verbose.unwrap_or(false);
     let force = force.unwrap_or(false);
 
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
-    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
-
-    let upid_str = WorkerTask::new_thread(
+    let upid_str = run_drive_worker(
+        rpcenv,
+        drive.clone(),
         "catalog-media",
         Some(drive.clone()),
-        auth_id,
-        to_stdout,
-        move |worker| {
-            let _lock_guard = lock_guard; // keep lock guard
-
+        move |worker, config| {
             let mut drive = open_drive(&config, &drive)?;
 
             drive.rewind()?;
@@ -1279,8 +1161,7 @@ pub fn catalog_media(
             restore_media(&worker, &mut drive, &media_id, None, verbose)?;
 
             Ok(())
-
-        }
+        },
     )?;
 
     Ok(upid_str.into())
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 05/11] api2/tape/drive: add wrapper for tokio::task::spawn_blocking

[Dominik Csapak]

similar to the worker wrapper, lock, write status, run code, unset status

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/drive.rs | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 7a6461b0..b4cd4aa6 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -101,6 +101,26 @@ where
     })
 }
 
+async fn run_drive_blocking_task<F, R>(drive: String, state: String, f: F) -> Result<R, Error>
+where
+    F: Send + 'static + FnOnce(SectionConfigData) -> Result<R, Error>,
+    R: Send + 'static,
+{
+    // early check/lock before starting worker
+    let (config, _digest) = config::drive::config()?;
+    let lock_guard = lock_tape_device(&config, &drive)?;
+    tokio::task::spawn_blocking(move || {
+        let _lock_guard = lock_guard;
+        set_tape_device_state(&drive, &state)
+            .map_err(|err| format_err!("could not set tape device state: {}", err))?;
+        let result = f(config);
+        set_tape_device_state(&drive, "")
+            .map_err(|err| format_err!("could not unset tape device state: {}", err))?;
+        result
+    })
+    .await?
+}
+
 #[api(
     input: {
         properties: {
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 06/11] api2/tape/drive: use run_drive_blocking_task where possible

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
best viewed with '-w'
 src/api2/tape/drive.rs | 245 ++++++++++++++++++++---------------------
 1 file changed, 119 insertions(+), 126 deletions(-)

diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index b4cd4aa6..5ee56441 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -180,16 +180,15 @@ pub fn load_media(
 ///
 /// Issue a media load request to the associated changer device.
 pub async fn load_slot(drive: String, source_slot: u64) -> Result<(), Error> {
-
-    let (config, _digest) = config::drive::config()?;
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    tokio::task::spawn_blocking(move || {
-        let _lock_guard = lock_guard; // keep lock guard
-
-        let (mut changer, _) = required_media_changer(&config, &drive)?;
-        changer.load_media_from_slot(source_slot)
-    }).await?
+    run_drive_blocking_task(
+        drive.clone(),
+        format!("load from slot {}", source_slot),
+        move |config| {
+            let (mut changer, _) = required_media_changer(&config, &drive)?;
+            changer.load_media_from_slot(source_slot)
+        },
+    )
+    .await
 }
 
 #[api(
@@ -211,19 +210,22 @@ pub async fn load_slot(drive: String, source_slot: u64) -> Result<(), Error> {
 )]
 /// Export media with specified label
 pub async fn export_media(drive: String, label_text: String) -> Result<u64, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    tokio::task::spawn_blocking(move || {
-        let _lock_guard = lock_guard; // keep lock guard
-
-        let (mut changer, changer_name) = required_media_changer(&config, &drive)?;
-        match changer.export_media(&label_text)? {
-            Some(slot) => Ok(slot),
-            None => bail!("media '{}' is not online (via changer '{}')", label_text, changer_name),
+    run_drive_blocking_task(
+        drive.clone(),
+        format!("export media {}", label_text),
+        move |config| {
+            let (mut changer, changer_name) = required_media_changer(&config, &drive)?;
+            match changer.export_media(&label_text)? {
+                Some(slot) => Ok(slot),
+                None => bail!(
+                    "media '{}' is not online (via changer '{}')",
+                    label_text,
+                    changer_name
+                ),
+            }
         }
-    }).await?
+    )
+    .await
 }
 
 #[api(
@@ -584,29 +586,26 @@ pub async fn restore_key(
     drive: String,
     password: String,
 ) -> Result<(), Error> {
+    run_drive_blocking_task(
+        drive.clone(),
+        "restore key".to_string(),
+        move |config| {
+            let mut drive = open_drive(&config, &drive)?;
 
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    tokio::task::spawn_blocking(move || {
-        let _lock_guard = lock_guard; // keep lock guard
-
-        let mut drive = open_drive(&config, &drive)?;
+            let (_media_id, key_config) = drive.read_label()?;
 
-        let (_media_id, key_config) = drive.read_label()?;
+            if let Some(key_config) = key_config {
+                let password_fn = || { Ok(password.as_bytes().to_vec()) };
+                let (key, ..) = key_config.decrypt(&password_fn)?;
+                config::tape_encryption_keys::insert_key(key, key_config, true)?;
+            } else {
+                bail!("media does not contain any encryption key configuration");
+            }
 
-        if let Some(key_config) = key_config {
-            let password_fn = || { Ok(password.as_bytes().to_vec()) };
-            let (key, ..) = key_config.decrypt(&password_fn)?;
-            config::tape_encryption_keys::insert_key(key, key_config, true)?;
-        } else {
-            bail!("media does not contain any encryption key configuration");
+            Ok(())
         }
-
-        Ok(())
-    }).await?
+    )
+    .await
 }
 
  #[api(
@@ -630,65 +629,62 @@ pub async fn read_label(
     drive: String,
     inventorize: Option<bool>,
 ) -> Result<MediaIdFlat, Error> {
+    run_drive_blocking_task(
+        drive.clone(),
+        "reading label".to_string(),
+        move |config| {
+            let mut drive = open_drive(&config, &drive)?;
 
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    tokio::task::spawn_blocking(move || {
-        let _lock_guard = lock_guard; // keep lock guard
-
-        let mut drive = open_drive(&config, &drive)?;
-
-        let (media_id, _key_config) = drive.read_label()?;
-
-        let media_id = match media_id {
-            Some(media_id) => {
-                let mut flat = MediaIdFlat {
-                    uuid: media_id.label.uuid.clone(),
-                    label_text: media_id.label.label_text.clone(),
-                    ctime: media_id.label.ctime,
-                    media_set_ctime: None,
-                    media_set_uuid: None,
-                    encryption_key_fingerprint: None,
-                    pool: None,
-                    seq_nr: None,
-                };
-                if let Some(ref set) = media_id.media_set_label {
-                    flat.pool = Some(set.pool.clone());
-                    flat.seq_nr = Some(set.seq_nr);
-                    flat.media_set_uuid = Some(set.uuid.clone());
-                    flat.media_set_ctime = Some(set.ctime);
-                    flat.encryption_key_fingerprint = set
-                        .encryption_key_fingerprint
-                        .as_ref()
-                        .map(|fp| crate::tools::format::as_fingerprint(fp.bytes()));
-
-                    let encrypt_fingerprint = set.encryption_key_fingerprint.clone()
-                        .map(|fp| (fp, set.uuid.clone()));
+            let (media_id, _key_config) = drive.read_label()?;
+
+            let media_id = match media_id {
+                Some(media_id) => {
+                    let mut flat = MediaIdFlat {
+                        uuid: media_id.label.uuid.clone(),
+                        label_text: media_id.label.label_text.clone(),
+                        ctime: media_id.label.ctime,
+                        media_set_ctime: None,
+                        media_set_uuid: None,
+                        encryption_key_fingerprint: None,
+                        pool: None,
+                        seq_nr: None,
+                    };
+                    if let Some(ref set) = media_id.media_set_label {
+                        flat.pool = Some(set.pool.clone());
+                        flat.seq_nr = Some(set.seq_nr);
+                        flat.media_set_uuid = Some(set.uuid.clone());
+                        flat.media_set_ctime = Some(set.ctime);
+                        flat.encryption_key_fingerprint = set
+                            .encryption_key_fingerprint
+                            .as_ref()
+                            .map(|fp| crate::tools::format::as_fingerprint(fp.bytes()));
+
+                        let encrypt_fingerprint = set.encryption_key_fingerprint.clone()
+                            .map(|fp| (fp, set.uuid.clone()));
+
+                        if let Err(err) = drive.set_encryption(encrypt_fingerprint) {
+                            // try, but ignore errors. just log to stderr
+                            eprintln!("uable to load encryption key: {}", err);
+                        }
+                    }
 
-                    if let Err(err) = drive.set_encryption(encrypt_fingerprint) {
-                        // try, but ignore errors. just log to stderr
-                        eprintln!("uable to load encryption key: {}", err);
+                    if let Some(true) = inventorize {
+                        let state_path = Path::new(TAPE_STATUS_DIR);
+                        let mut inventory = Inventory::load(state_path)?;
+                        inventory.store(media_id, false)?;
                     }
-                }
 
-                if let Some(true) = inventorize {
-                    let state_path = Path::new(TAPE_STATUS_DIR);
-                    let mut inventory = Inventory::load(state_path)?;
-                    inventory.store(media_id, false)?;
+                    flat
                 }
+                None => {
+                    bail!("Media is empty (no label).");
+                }
+            };
 
-                flat
-            }
-            None => {
-                bail!("Media is empty (no label).");
-            }
-        };
-
-        Ok(media_id)
-    }).await?
+            Ok(media_id)
+        }
+    )
+    .await
 }
 
 #[api(
@@ -755,49 +751,46 @@ pub fn clean_drive(
 pub async fn inventory(
     drive: String,
 ) -> Result<Vec<LabelUuidMap>, Error> {
+    run_drive_blocking_task(
+        drive.clone(),
+        "inventorize".to_string(),
+        move |config| {
+            let (mut changer, changer_name) = required_media_changer(&config, &drive)?;
 
-    let (config, _digest) = config::drive::config()?;
-
-    // early check/lock before starting worker
-    let lock_guard = lock_tape_device(&config, &drive)?;
-
-    tokio::task::spawn_blocking(move || {
-        let _lock_guard = lock_guard; // keep lock guard
+            let label_text_list = changer.online_media_label_texts()?;
 
-        let (mut changer, changer_name) = required_media_changer(&config, &drive)?;
+            let state_path = Path::new(TAPE_STATUS_DIR);
 
-        let label_text_list = changer.online_media_label_texts()?;
+            let mut inventory = Inventory::load(state_path)?;
 
-        let state_path = Path::new(TAPE_STATUS_DIR);
+            update_changer_online_status(
+                &config,
+                &mut inventory,
+                &changer_name,
+                &label_text_list,
+            )?;
 
-        let mut inventory = Inventory::load(state_path)?;
+            let mut list = Vec::new();
 
-        update_changer_online_status(
-            &config,
-            &mut inventory,
-            &changer_name,
-            &label_text_list,
-        )?;
+            for label_text in label_text_list.iter() {
+                if label_text.starts_with("CLN") {
+                    // skip cleaning unit
+                    continue;
+                }
 
-        let mut list = Vec::new();
+                let label_text = label_text.to_string();
 
-        for label_text in label_text_list.iter() {
-            if label_text.starts_with("CLN") {
-                // skip cleaning unit
-                continue;
+                if let Some(media_id) = inventory.find_media_by_label_text(&label_text) {
+                    list.push(LabelUuidMap { label_text, uuid: Some(media_id.label.uuid.clone()) });
+                } else {
+                    list.push(LabelUuidMap { label_text, uuid: None });
+                }
             }
 
-            let label_text = label_text.to_string();
-
-            if let Some(media_id) = inventory.find_media_by_label_text(&label_text) {
-                list.push(LabelUuidMap { label_text, uuid: Some(media_id.label.uuid.clone()) });
-            } else {
-                list.push(LabelUuidMap { label_text, uuid: None });
-            }
+            Ok(list)
         }
-
-        Ok(list)
-    }).await?
+    )
+    .await
 }
 
 #[api(
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 07/11] api2/tape/drive: wrap some api calls in run_drive_blocking_task

[Dominik Csapak]

those calls could also block, so we have to run them in a blocking
tokio task, as to not block the current thread

nice side effect is that we now also update the state for that
drive in those instances

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/drive.rs | 64 +++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 29 deletions(-)

diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 5ee56441..eeda9e21 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -1014,16 +1014,18 @@ fn barcode_label_media_worker(
     },
 )]
 /// Read Cartridge Memory (Medium auxiliary memory attributes)
-pub fn cartridge_memory(drive: String) -> Result<Vec<MamAttribute>, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-
-    let _lock_guard = lock_tape_device(&config, &drive)?;
-
-    let drive_config: LinuxTapeDrive = config.lookup("linux", &drive)?;
-    let mut handle = drive_config.open()?;
+pub async fn cartridge_memory(drive: String) -> Result<Vec<MamAttribute>, Error> {
+    run_drive_blocking_task(
+        drive.clone(),
+        "reading cartridge memory".to_string(),
+        move |config| {
+            let drive_config: LinuxTapeDrive = config.lookup("linux", &drive)?;
+            let mut handle = drive_config.open()?;
 
-    handle.cartridge_memory()
+            handle.cartridge_memory()
+        }
+    )
+    .await
 }
 
 #[api(
@@ -1039,16 +1041,18 @@ pub fn cartridge_memory(drive: String) -> Result<Vec<MamAttribute>, Error> {
     },
 )]
 /// Read Volume Statistics (SCSI log page 17h)
-pub fn volume_statistics(drive: String) -> Result<Lp17VolumeStatistics, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-
-    let _lock_guard = lock_tape_device(&config, &drive)?;
-
-    let drive_config: LinuxTapeDrive = config.lookup("linux", &drive)?;
-    let mut handle = drive_config.open()?;
+pub async fn volume_statistics(drive: String) -> Result<Lp17VolumeStatistics, Error> {
+    run_drive_blocking_task(
+        drive.clone(),
+        "reading volume statistics".to_string(),
+        move |config| {
+            let drive_config: LinuxTapeDrive = config.lookup("linux", &drive)?;
+            let mut handle = drive_config.open()?;
 
-    handle.volume_statistics()
+            handle.volume_statistics()
+        }
+    )
+    .await
 }
 
 #[api(
@@ -1064,20 +1068,22 @@ pub fn volume_statistics(drive: String) -> Result<Lp17VolumeStatistics, Error> {
     },
 )]
 /// Get drive/media status
-pub fn status(drive: String) -> Result<LinuxDriveAndMediaStatus, Error> {
-
-    let (config, _digest) = config::drive::config()?;
-
-    let _lock_guard = lock_tape_device(&config, &drive)?;
-
-    let drive_config: LinuxTapeDrive = config.lookup("linux", &drive)?;
+pub async fn status(drive: String) -> Result<LinuxDriveAndMediaStatus, Error> {
+    run_drive_blocking_task(
+        drive.clone(),
+        "reading drive status".to_string(),
+        move |config| {
+            let drive_config: LinuxTapeDrive = config.lookup("linux", &drive)?;
 
-    // Note: use open_linux_tape_device, because this also works if no medium loaded
-    let file = open_linux_tape_device(&drive_config.path)?;
+            // Note: use open_linux_tape_device, because this also works if no medium loaded
+            let file = open_linux_tape_device(&drive_config.path)?;
 
-    let mut handle = LinuxTapeHandle::new(file);
+            let mut handle = LinuxTapeHandle::new(file);
 
-    handle.get_drive_and_media_status()
+            handle.get_drive_and_media_status()
+        }
+    )
+    .await
 }
 
 #[api(
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 08/11] api2/tape/changer: add drive state to changer status output

[Dominik Csapak]

if we can find the drive in the config and it has a state

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/changer.rs       | 22 ++++++++++++++++++++++
 src/api2/types/tape/changer.rs |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/src/api2/tape/changer.rs b/src/api2/tape/changer.rs
index dc98d85b..212ca8eb 100644
--- a/src/api2/tape/changer.rs
+++ b/src/api2/tape/changer.rs
@@ -1,3 +1,4 @@
+use std::collections::HashMap;
 use std::path::Path;
 
 use anyhow::Error;
@@ -11,6 +12,7 @@ use crate::{
     api2::types::{
         CHANGER_NAME_SCHEMA,
         ChangerListEntry,
+        LinuxTapeDrive,
         MtxEntryKind,
         MtxStatusEntry,
         ScsiTapeChanger,
@@ -25,6 +27,7 @@ use crate::{
             ScsiMediaChange,
             mtx_status_to_online_set,
         },
+        drive::get_tape_device_state,
         lookup_device_identification,
     },
 };
@@ -66,9 +69,26 @@ pub async fn get_status(name: String) -> Result<Vec<MtxStatusEntry>, Error> {
 
     inventory.update_online_status(&map)?;
 
+    let drive_list: Vec<LinuxTapeDrive> = config.convert_to_typed_array("linux")?;
+    let mut drive_map: HashMap<u64, String> = HashMap::new();
+
+    for drive in drive_list {
+        if let Some(changer) = drive.changer {
+            if changer != name {
+                continue;
+            }
+            let num = drive.changer_drivenum.unwrap_or(0);
+            drive_map.insert(num, drive.name.clone());
+        }
+    }
+
     let mut list = Vec::new();
 
     for (id, drive_status) in status.drives.iter().enumerate() {
+        let mut state = None;
+        if let Some(drive) = drive_map.get(&(id as u64)) {
+            state = get_tape_device_state(&config, &drive)?;
+        }
         let entry = MtxStatusEntry {
             entry_kind: MtxEntryKind::Drive,
             entry_id: id as u64,
@@ -78,6 +98,7 @@ pub async fn get_status(name: String) -> Result<Vec<MtxStatusEntry>, Error> {
                 ElementStatus::VolumeTag(tag) => Some(tag.to_string()),
             },
             loaded_slot: drive_status.loaded_slot,
+            state,
         };
         list.push(entry);
     }
@@ -96,6 +117,7 @@ pub async fn get_status(name: String) -> Result<Vec<MtxStatusEntry>, Error> {
                 ElementStatus::VolumeTag(tag) => Some(tag.to_string()),
             },
             loaded_slot: None,
+            state: None,
         };
         list.push(entry);
     }
diff --git a/src/api2/types/tape/changer.rs b/src/api2/types/tape/changer.rs
index f9d30acd..780bf669 100644
--- a/src/api2/types/tape/changer.rs
+++ b/src/api2/types/tape/changer.rs
@@ -129,4 +129,7 @@ pub struct MtxStatusEntry {
     /// The slot the drive was loaded from
     #[serde(skip_serializing_if="Option::is_none")]
     pub loaded_slot: Option<u64>,
+    /// The current state of the drive
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub state: Option<String>,
 }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 09/11] api2/tape/{backup, restore}, proxmox-tape: set device state

[Dominik Csapak]

set the drive device state everywhere we lock it, so that we
know what it currently does

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/backup.rs  | 13 +++++++++++++
 src/api2/tape/restore.rs | 13 +++++++++++++
 src/bin/proxmox-tape.rs  |  3 +++
 3 files changed, 29 insertions(+)

diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 522b4097..a9c54e7d 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -45,6 +45,7 @@ use crate::{
         drive::{
             media_changer,
             lock_tape_device,
+            set_tape_device_state,
         },
         changer::update_changer_online_status,
     },
@@ -90,6 +91,7 @@ pub fn do_tape_backup_job(
         move |worker| {
             let _drive_lock = drive_lock; // keep lock guard
 
+            set_tape_device_state(&tape_job.drive, &worker.upid().to_string())?;
             job.start(&worker.upid().to_string())?;
 
             let eject_media = false;
@@ -119,6 +121,14 @@ pub fn do_tape_backup_job(
                 );
             }
 
+            if let Err(err) = set_tape_device_state(&tape_job.drive, "") {
+                eprintln!(
+                    "could not unset drive state for {}: {}",
+                    tape_job.drive,
+                    err
+                );
+            }
+
             job_result
         }
     )?;
@@ -216,7 +226,10 @@ pub fn backup(
         to_stdout,
         move |worker| {
             let _drive_lock = drive_lock; // keep lock guard
+            set_tape_device_state(&drive, &worker.upid().to_string())?;
             backup_worker(&worker, datastore, &drive, &pool_config, eject_media, export_media_set)?;
+            // ignore errors
+            let _ = set_tape_device_state(&drive, "");
             Ok(())
         }
     )?;
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 498b49df..707b8f36 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -69,6 +69,7 @@ use crate::{
             TapeDriver,
             request_and_load_media,
             lock_tape_device,
+            set_tape_device_state,
         },
     },
 };
@@ -134,6 +135,8 @@ pub fn restore(
         move |worker| {
             let _drive_lock = drive_lock; // keep lock guard
 
+            set_tape_device_state(&drive, &worker.upid().to_string())?;
+
             let _lock = MediaPool::lock(status_path, &pool)?;
 
             let members = inventory.compute_media_set_members(&media_set_uuid)?;
@@ -189,6 +192,16 @@ pub fn restore(
             }
 
             task_log!(worker, "Restore mediaset '{}' done", media_set);
+
+            if let Err(err) = set_tape_device_state(&drive, "") {
+                task_log!(
+                    worker,
+                    "could not unset drive state for {}: {}",
+                    drive,
+                    err
+                );
+            }
+
             Ok(())
         }
     )?;
diff --git a/src/bin/proxmox-tape.rs b/src/bin/proxmox-tape.rs
index eb792986..74b9c128 100644
--- a/src/bin/proxmox-tape.rs
+++ b/src/bin/proxmox-tape.rs
@@ -43,6 +43,7 @@ use proxmox_backup::{
         drive::{
             open_drive,
             lock_tape_device,
+            set_tape_device_state,
         },
         complete_media_label_text,
         complete_media_set_uuid,
@@ -543,6 +544,7 @@ fn move_to_eom(mut param: Value) -> Result<(), Error> {
     let drive = extract_drive_name(&mut param, &config)?;
 
     let _lock = lock_tape_device(&config, &drive)?;
+    set_tape_device_state(&drive, "moving to eom")?;
 
     let mut drive = open_drive(&config, &drive)?;
 
@@ -572,6 +574,7 @@ fn debug_scan(mut param: Value) -> Result<(), Error> {
     let drive = extract_drive_name(&mut param, &config)?;
 
     let _lock = lock_tape_device(&config, &drive)?;
+    set_tape_device_state(&drive, "debug scan")?;
 
     let mut drive = open_drive(&config, &drive)?;
 
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 10/11] ui: tape: fix eslint warnings (trailing comma)

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/tape/BackupJobs.js    | 6 +++---
 www/tape/ChangerStatus.js | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/www/tape/BackupJobs.js b/www/tape/BackupJobs.js
index e2c03365..80ed99a5 100644
--- a/www/tape/BackupJobs.js
+++ b/www/tape/BackupJobs.js
@@ -3,7 +3,7 @@ Ext.define('pbs-tape-backup-job-status', {
     fields: [
 	'id', 'store', 'pool', 'drive', 'store', 'schedule', 'comment',
 	{ name: 'eject-media', type: 'boolean' },
-	{ name: 'export-media-set', type: 'boolean' }
+	{ name: 'export-media-set', type: 'boolean' },
     ],
     idProperty: 'id',
     proxy: {
@@ -32,7 +32,7 @@ Ext.define('PBS.config.TapeBackupJobView', {
 
 	init: function(view) {
 	    Proxmox.Utils.monStoreErrors(view, view.getStore().rstore);
-	}
+	},
     },
 
     listeners: {
@@ -107,5 +107,5 @@ Ext.define('PBS.config.TapeBackupJobView', {
 	let me = this;
 
 	me.callParent();
-    }
+    },
 });
diff --git a/www/tape/ChangerStatus.js b/www/tape/ChangerStatus.js
index d00d712f..436bae72 100644
--- a/www/tape/ChangerStatus.js
+++ b/www/tape/ChangerStatus.js
@@ -422,7 +422,7 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 		    method: 'GET',
 		    params: {
 			"update-status": false,
-		    }
+		    },
 		});
 
 		let [status, drives, tapes_list] = await Promise.all([status_fut, drives_fut, tapes_fut]);
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 11/11] ui: tape/ChangerStatus: show the state of the drive

[Dominik Csapak]

an optimize the columns for smaller layouts (1280 width)
we show either:
* Idle
* spinner + status (if no upid)
* spinner + rendered UPID (clickable, opens task viewer)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
we probably want to have an updatestore for only the drive state
of the changer (such an api call would not cost much, since it
only checks a lock + read from tmpfs) to have an up-to-date view
of the drive state. If we have that, we can enable/disable
buttons/options for busy drives.

 www/tape/ChangerStatus.js | 47 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/www/tape/ChangerStatus.js b/www/tape/ChangerStatus.js
index 436bae72..5a1c6073 100644
--- a/www/tape/ChangerStatus.js
+++ b/www/tape/ChangerStatus.js
@@ -498,6 +498,42 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 	    }
 	    return status;
 	},
+
+	renderState: function(value, md, record) {
+	    if (!value) {
+		return gettext('Idle');
+	    }
+
+	    let icon = '<i class="fa fa-spinner fa-pulse fa-fw"></i>';
+
+	    if (value.startsWith("UPID")) {
+		let upid = Proxmox.Utils.parse_task_upid(value);
+		md.tdCls = "pointer";
+		return `${icon} ${upid.desc}`;
+	    }
+
+	    return `${icon} ${value}`;
+	},
+
+	control: {
+	    'grid[reference=drives]': {
+		cellclick: function(table, td, ci, rec, tr, ri, e) {
+		    if (!e.position.column.dataIndex === 'state') {
+			return;
+		    }
+
+		    let upid = rec.data.state;
+		    if (!upid || !upid.startsWith("UPID")) {
+			return;
+		    }
+
+		    Ext.create('Proxmox.window.TaskViewer', {
+			autoShow: true,
+			upid,
+		    });
+		},
+	    },
+	},
     },
 
     listeners: {
@@ -641,7 +677,7 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 				    text: gettext('Inventory'),
 				    dataIndex: 'is-labeled',
 				    renderer: 'renderIsLabeled',
-				    flex: 1,
+				    flex: 1.5,
 				},
 				{
 				    text: gettext("Name"),
@@ -650,10 +686,17 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 				    flex: 1,
 				    renderer: Ext.htmlEncode,
 				},
+				{
+				    text: gettext('State'),
+				    dataIndex: 'state',
+				    flex: 3,
+				    renderer: 'renderState',
+				},
 				{
 				    text: gettext("Vendor"),
 				    sortable: true,
 				    dataIndex: 'vendor',
+				    hidden: true,
 				    flex: 1,
 				    renderer: Ext.htmlEncode,
 				},
@@ -661,6 +704,7 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 				    text: gettext("Model"),
 				    sortable: true,
 				    dataIndex: 'model',
+				    hidden: true,
 				    flex: 1,
 				    renderer: Ext.htmlEncode,
 				},
@@ -668,6 +712,7 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 				    text: gettext("Serial"),
 				    sortable: true,
 				    dataIndex: 'serial',
+				    hidden: true,
 				    flex: 1,
 				    renderer: Ext.htmlEncode,
 				},
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup 11/11] ui: tape/ChangerStatus: show the state of the drive

[Dominik Csapak]

On 2/18/21 15:40, Dominik Csapak wrote:
> an optimize the columns for smaller layouts (1280 width)
> we show either:
> * Idle
> * spinner + status (if no upid)
> * spinner + rendered UPID (clickable, opens task viewer)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> we probably want to have an updatestore for only the drive state
> of the changer (such an api call would not cost much, since it
> only checks a lock + read from tmpfs) to have an up-to-date view
> of the drive state. If we have that, we can enable/disable
> buttons/options for busy drives.
> 
>   www/tape/ChangerStatus.js | 47 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/www/tape/ChangerStatus.js b/www/tape/ChangerStatus.js
> index 436bae72..5a1c6073 100644
> --- a/www/tape/ChangerStatus.js
> +++ b/www/tape/ChangerStatus.js
> @@ -498,6 +498,42 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
>   	    }
>   	    return status;
>   	},
> +
> +	renderState: function(value, md, record) {
> +	    if (!value) {
> +		return gettext('Idle');
> +	    }
> +
> +	    let icon = '<i class="fa fa-spinner fa-pulse fa-fw"></i>';
> +
> +	    if (value.startsWith("UPID")) {
> +		let upid = Proxmox.Utils.parse_task_upid(value);
> +		md.tdCls = "pointer";
> +		return `${icon} ${upid.desc}`;
> +	    }
> +
> +	    return `${icon} ${value}`;
> +	},
> +
> +	control: {
> +	    'grid[reference=drives]': {
> +		cellclick: function(table, td, ci, rec, tr, ri, e) {
> +		    if (!e.position.column.dataIndex === 'state') {

this line should be

if (e.position.column.dataIndex !== 'state') {

i'll fix in a potential v2 or alternatively send a fixup

> +			return;
> +		    }
> +
> +		    let upid = rec.data.state;
> +		    if (!upid || !upid.startsWith("UPID")) {
> +			return;
> +		    }
> +
> +		    Ext.create('Proxmox.window.TaskViewer', {
> +			autoShow: true,
> +			upid,
> +		    });
> +		},
> +	    },
> +	},
>       },
>   
>       listeners: {
> @@ -641,7 +677,7 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
>   				    text: gettext('Inventory'),
>   				    dataIndex: 'is-labeled',
>   				    renderer: 'renderIsLabeled',
> -				    flex: 1,
> +				    flex: 1.5,
>   				},
>   				{
>   				    text: gettext("Name"),
> @@ -650,10 +686,17 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
>   				    flex: 1,
>   				    renderer: Ext.htmlEncode,
>   				},
> +				{
> +				    text: gettext('State'),
> +				    dataIndex: 'state',
> +				    flex: 3,
> +				    renderer: 'renderState',
> +				},
>   				{
>   				    text: gettext("Vendor"),
>   				    sortable: true,
>   				    dataIndex: 'vendor',
> +				    hidden: true,
>   				    flex: 1,
>   				    renderer: Ext.htmlEncode,
>   				},
> @@ -661,6 +704,7 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
>   				    text: gettext("Model"),
>   				    sortable: true,
>   				    dataIndex: 'model',
> +				    hidden: true,
>   				    flex: 1,
>   				    renderer: Ext.htmlEncode,
>   				},
> @@ -668,6 +712,7 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
>   				    text: gettext("Serial"),
>   				    sortable: true,
>   				    dataIndex: 'serial',
> +				    hidden: true,
>   				    flex: 1,
>   				    renderer: Ext.htmlEncode,
>   				},
>