public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity
@ 2024-05-13 10:49 Dominik Csapak
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/4] tape: add functions to parse drive " Dominik Csapak
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Dominik Csapak @ 2024-05-13 10:49 UTC (permalink / raw)
  To: pbs-devel

Whenever an external activity is done on a tape drive (e.g. via `pmt`, a
changer webui, or an automatic action like calibrating), most of our
tape commands will hang and nothing on the webui is showing any
activity.

This is most annoying when loading new LTO-9 tapes into a drive, since
it is blocked up to two hours, without any indication that work is done.

This can be frustrating, so this series queries and shows the device
activity from the drive when it's suitable.

Dominik Csapak (4):
  tape: add functions to parse drive device activity
  tape: add drive activity to drive status api
  tape: drive status: make some depend on the activity
  tape: include drive activity in status

 pbs-api-types/src/tape/drive.rs           | 71 ++++++++++++++++
 pbs-tape/src/sg_tape.rs                   | 62 +++++++++-----
 pbs-tape/src/sg_tape/device_status.rs     | 99 +++++++++++++++++++++++
 pbs-tape/src/sg_tape/volume_statistics.rs |  8 +-
 src/api2/tape/drive.rs                    | 14 ++++
 www/Utils.js                              | 34 +++++++-
 www/tape/ChangerStatus.js                 | 11 ++-
 www/tape/DriveStatus.js                   |  6 +-
 8 files changed, 278 insertions(+), 27 deletions(-)
 create mode 100644 pbs-tape/src/sg_tape/device_status.rs

-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 1/4] tape: add functions to parse drive device activity
  2024-05-13 10:49 [pbs-devel] [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dominik Csapak
@ 2024-05-13 10:49 ` Dominik Csapak
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 2/4] tape: add drive activity to drive status api Dominik Csapak
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2024-05-13 10:49 UTC (permalink / raw)
  To: pbs-devel

we use the VHF part from the DT Device Activity page for that.
This is intended to query the drive for it's current state and activity.

Currently only the activity is parsed and used.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-api-types/src/tape/drive.rs           | 65 +++++++++++++++
 pbs-tape/src/sg_tape.rs                   |  3 +
 pbs-tape/src/sg_tape/device_status.rs     | 99 +++++++++++++++++++++++
 pbs-tape/src/sg_tape/volume_statistics.rs |  8 +-
 4 files changed, 171 insertions(+), 4 deletions(-)
 create mode 100644 pbs-tape/src/sg_tape/device_status.rs

diff --git a/pbs-api-types/src/tape/drive.rs b/pbs-api-types/src/tape/drive.rs
index 626c5d9c3..de569980a 100644
--- a/pbs-api-types/src/tape/drive.rs
+++ b/pbs-api-types/src/tape/drive.rs
@@ -276,3 +276,68 @@ pub struct Lp17VolumeStatistics {
     /// Volume serial number
     pub serial: String,
 }
+
+/// The DT Device Activity from DT Device Status LP page
+#[api]
+#[derive(Copy, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum DeviceActivity {
+    /// No activity
+    NoActivity,
+    /// Cleaning
+    Cleaning,
+    /// Loading
+    Loading,
+    /// Unloading
+    Unloading,
+    /// Other unspecified activity
+    Other,
+    /// Reading
+    Reading,
+    /// Writing
+    Writing,
+    /// Locating
+    Locating,
+    /// Rewinding
+    Rewinding,
+    /// Erasing
+    Erasing,
+    /// Formatting
+    Formatting,
+    /// Calibrating
+    Calibrating,
+    /// Other (DT)
+    OtherDT,
+    /// Updating microcode
+    MicrocodeUpdate,
+    /// Reading encrypted data
+    ReadingEncrypted,
+    /// Writing encrypted data
+    WritingEncrypted,
+}
+
+impl TryFrom<u8> for DeviceActivity {
+    type Error = Error;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        Ok(match value {
+            0x00 => DeviceActivity::NoActivity,
+            0x01 => DeviceActivity::Cleaning,
+            0x02 => DeviceActivity::Loading,
+            0x03 => DeviceActivity::Unloading,
+            0x04 => DeviceActivity::Other,
+            0x05 => DeviceActivity::Reading,
+            0x06 => DeviceActivity::Writing,
+            0x07 => DeviceActivity::Locating,
+            0x08 => DeviceActivity::Rewinding,
+            0x09 => DeviceActivity::Erasing,
+            0x0A => DeviceActivity::Formatting,
+            0x0B => DeviceActivity::Calibrating,
+            0x0C => DeviceActivity::OtherDT,
+            0x0D => DeviceActivity::MicrocodeUpdate,
+            0x0E => DeviceActivity::ReadingEncrypted,
+            0x0F => DeviceActivity::WritingEncrypted,
+            other => bail!("invalid DT device activity value: {:x}", other),
+        })
+    }
+}
diff --git a/pbs-tape/src/sg_tape.rs b/pbs-tape/src/sg_tape.rs
index 146d05507..85be6b071 100644
--- a/pbs-tape/src/sg_tape.rs
+++ b/pbs-tape/src/sg_tape.rs
@@ -15,6 +15,9 @@ mod volume_statistics;
 use proxmox_uuid::Uuid;
 pub use volume_statistics::*;
 
+mod device_status;
+pub use device_status::*;
+
 mod tape_alert_flags;
 pub use tape_alert_flags::*;
 
diff --git a/pbs-tape/src/sg_tape/device_status.rs b/pbs-tape/src/sg_tape/device_status.rs
new file mode 100644
index 000000000..353ba0a7b
--- /dev/null
+++ b/pbs-tape/src/sg_tape/device_status.rs
@@ -0,0 +1,99 @@
+use std::os::fd::AsRawFd;
+
+use anyhow::{bail, format_err, Error};
+
+use pbs_api_types::DeviceActivity;
+use proxmox_io::ReadExt;
+
+use super::LpParameterHeader;
+use crate::sgutils2::SgRaw;
+
+/// SCSI command to query volume statistics
+///
+/// CDB: LOG SENSE / LP11h DT Device Activity
+///
+/// Only returns the Device Activity result from the VHF data
+pub fn read_device_activity<F: AsRawFd>(file: &mut F) -> Result<DeviceActivity, Error> {
+    let data = sg_read_dt_device_status(file)?;
+
+    decode_dt_device_status(&data)
+        .map_err(|err| format_err!("decode dt device status failed - {}", err))
+}
+
+#[allow(clippy::vec_init_then_push)]
+fn sg_read_dt_device_status<F: AsRawFd>(file: &mut F) -> Result<Vec<u8>, Error> {
+    let alloc_len: u16 = 8192;
+    let mut sg_raw = SgRaw::new(file, alloc_len as usize)?;
+
+    let mut cmd = Vec::new();
+    cmd.push(0x4D); // LOG SENSE
+    cmd.push(0);
+    cmd.push((1 << 6) | 0x11); // DT Device Status log page
+    cmd.push(0); // Subpage 0
+    cmd.push(0);
+    cmd.push(0);
+    cmd.push(0);
+    cmd.extend(alloc_len.to_be_bytes()); // alloc len
+    cmd.push(0u8); // control byte
+
+    sg_raw.set_timeout(1); // use short timeout
+    sg_raw
+        .do_command(&cmd)
+        .map_err(|err| format_err!("read tape dt device status failed - {}", err))
+        .map(|v| v.to_vec())
+}
+
+fn decode_dt_device_status(data: &[u8]) -> Result<DeviceActivity, Error> {
+    if !((data[0] & 0x7f) == 0x11 && data[1] == 0) {
+        bail!("invalid response");
+    }
+
+    let mut reader = &data[2..];
+
+    let page_len: u16 = unsafe { reader.read_be_value()? };
+
+    let page_len = page_len as usize;
+
+    if (page_len + 4) > data.len() {
+        bail!("invalid page length");
+    } else {
+        // Note: Quantum hh7 returns the allocation_length instead of real data_len
+        reader = &data[4..page_len + 4];
+    }
+
+    let mut page_valid = false;
+
+    let mut activity = DeviceActivity::Other;
+
+    loop {
+        if reader.is_empty() {
+            break;
+        }
+        let head: LpParameterHeader = unsafe { reader.read_be_value()? };
+
+        match head.parameter_code {
+            0x0000 => {
+                let vhf_descriptor = reader.read_exact_allocated(head.parameter_len as usize)?;
+
+                if vhf_descriptor.len() != 4 {
+                    bail!("invalid VHF data descriptor");
+                }
+
+                activity = vhf_descriptor[2].try_into()?;
+
+                if vhf_descriptor[0] & 0x01 == 1 {
+                    page_valid = true;
+                }
+            }
+            _ => {
+                reader.read_exact_allocated(head.parameter_len as usize)?;
+            }
+        }
+    }
+
+    if !page_valid {
+        bail!("missing page-valid parameter");
+    }
+
+    Ok(activity)
+}
diff --git a/pbs-tape/src/sg_tape/volume_statistics.rs b/pbs-tape/src/sg_tape/volume_statistics.rs
index f27a682c1..d3815db09 100644
--- a/pbs-tape/src/sg_tape/volume_statistics.rs
+++ b/pbs-tape/src/sg_tape/volume_statistics.rs
@@ -46,10 +46,10 @@ fn sg_read_volume_statistics<F: AsRawFd>(file: &mut F) -> Result<Vec<u8>, Error>
 
 #[repr(C, packed)]
 #[derive(Endian)]
-struct LpParameterHeader {
-    parameter_code: u16,
-    control: u8,
-    parameter_len: u8,
+pub(crate) struct LpParameterHeader {
+    pub parameter_code: u16,
+    pub control: u8,
+    pub parameter_len: u8,
 }
 
 fn decode_volume_statistics(data: &[u8]) -> Result<Lp17VolumeStatistics, Error> {
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 2/4] tape: add drive activity to drive status api
  2024-05-13 10:49 [pbs-devel] [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dominik Csapak
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/4] tape: add functions to parse drive " Dominik Csapak
@ 2024-05-13 10:49 ` Dominik Csapak
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 3/4] tape: drive status: make some depend on the activity Dominik Csapak
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2024-05-13 10:49 UTC (permalink / raw)
  To: pbs-devel

and show it in the gui for single drives. Adds the known values for the
activity to the UI.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-api-types/src/tape/drive.rs |  3 +++
 pbs-tape/src/sg_tape.rs         |  5 ++++-
 www/Utils.js                    | 29 +++++++++++++++++++++++++++++
 www/tape/DriveStatus.js         |  4 ++++
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/pbs-api-types/src/tape/drive.rs b/pbs-api-types/src/tape/drive.rs
index de569980a..caa6b3b3e 100644
--- a/pbs-api-types/src/tape/drive.rs
+++ b/pbs-api-types/src/tape/drive.rs
@@ -216,6 +216,9 @@ pub struct LtoDriveAndMediaStatus {
     /// Estimated tape wearout factor (assuming max. 16000 end-to-end passes)
     #[serde(skip_serializing_if = "Option::is_none")]
     pub medium_wearout: Option<f64>,
+    /// Current device activity
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub drive_activity: Option<DeviceActivity>,
 }
 
 #[api()]
diff --git a/pbs-tape/src/sg_tape.rs b/pbs-tape/src/sg_tape.rs
index 85be6b071..d185dd1ac 100644
--- a/pbs-tape/src/sg_tape.rs
+++ b/pbs-tape/src/sg_tape.rs
@@ -31,7 +31,8 @@ use proxmox_io::{ReadExt, WriteExt};
 use proxmox_sys::error::SysResult;
 
 use pbs_api_types::{
-    Lp17VolumeStatistics, LtoDriveAndMediaStatus, LtoTapeDrive, MamAttribute, TapeDensity,
+    DeviceActivity, Lp17VolumeStatistics, LtoDriveAndMediaStatus, LtoTapeDrive, MamAttribute,
+    TapeDensity,
 };
 
 use crate::linux_list_drives::open_lto_tape_device;
@@ -960,6 +961,7 @@ impl SgTape {
     pub fn get_drive_and_media_status(&mut self) -> Result<LtoDriveAndMediaStatus, Error> {
         let drive_status = self.read_drive_status()?;
 
+        let drive_activity = read_device_activity(&mut self.file).ok();
         let alert_flags = self
             .tape_alert_flags()
             .map(|flags| format!("{:?}", flags))
@@ -983,6 +985,7 @@ impl SgTape {
             medium_passes: None,
             medium_wearout: None,
             volume_mounts: None,
+            drive_activity,
         };
 
         if self.test_unit_ready().is_ok() {
diff --git a/www/Utils.js b/www/Utils.js
index 1d7351a32..df03a4c87 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -659,6 +659,9 @@ Ext.define('PBS.Utils', {
 	    if (key === 'bytes-read' || key === 'bytes-written') {
 		val = Proxmox.Utils.format_size(val);
 	    }
+	    if (key === 'drive-activity') {
+		val = PBS.Utils.renderDriveActivity(val);
+	    }
 	    list.push({ key: key, value: val });
 	}
 
@@ -692,6 +695,32 @@ Ext.define('PBS.Utils', {
 	}).show();
     },
 
+    tapeDriveActivities: {
+	'no-activity': gettext('No Activity'),
+	'cleaning': gettext('Cleaning'),
+	'loading': gettext('Loading'),
+	'unloading': gettext('Unloading'),
+	'other': gettext('Other Activity'),
+	'reading': gettext('Reading data'),
+	'writing': gettext('Writing data'),
+	'locating': gettext('Locating'),
+	'rewinding': gettext('Rewinding'),
+	'erasing': gettext('Erasing'),
+	'formatting': gettext('Formatting'),
+	'calibrating': gettext('Calibrating'),
+	'other-dt': gettext('Other DT Activity'),
+	'microcode-update': gettext('Updating Microcode'),
+	'reading-encrypted': gettext('Reading encrypted data'),
+	'writing-encrypted': gettext('Writing encrypted data'),
+    },
+
+    renderDriveActivity: function(value) {
+	if (!value) {
+	    return Proxmox.Utils.unknownText;
+	}
+	return PBS.Utils.tapeDriveActivities[value] ?? value;
+    },
+
     renderDriveState: function(value, md) {
 	if (!value) {
 	    return gettext('Idle');
diff --git a/www/tape/DriveStatus.js b/www/tape/DriveStatus.js
index 2c55fc97f..a392202ca 100644
--- a/www/tape/DriveStatus.js
+++ b/www/tape/DriveStatus.js
@@ -348,6 +348,10 @@ Ext.define('PBS.TapeManagement.DriveStatusGrid', {
 	    header: gettext('Compression'),
 	    renderer: Proxmox.Utils.format_boolean,
 	},
+	'drive-activity': {
+	    header: gettext('Drive Activity'),
+	    renderer: PBS.Utils.renderDriveActivity,
+	},
 	'file-number': {
 	    header: gettext('Tape Position'),
 	    renderer: function(value, mD, r, rI, cI, store) {
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 3/4] tape: drive status: make some depend on the activity
  2024-05-13 10:49 [pbs-devel] [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dominik Csapak
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/4] tape: add functions to parse drive " Dominik Csapak
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 2/4] tape: add drive activity to drive status api Dominik Csapak
@ 2024-05-13 10:49 ` Dominik Csapak
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 4/4] tape: include drive activity in status Dominik Csapak
  2024-05-14  8:32 ` [pbs-devel] applied: [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dietmar Maurer
  4 siblings, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2024-05-13 10:49 UTC (permalink / raw)
  To: pbs-devel

when the tape drive has an activity (and the tape is in motion), certain
calls block until the operation is finished. Since we cannot predict how
long it's going to be and it can be quite long in certain cases,
skip those calls when the drive is doing anything.

If we cannot determine the activity, try to do the queries.

We have to extend the check for a loaded drive in the UI, since the
position is not available during any activity.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-tape/src/sg_tape.rs | 48 +++++++++++++++++++++++++----------------
 www/tape/DriveStatus.js |  2 +-
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/pbs-tape/src/sg_tape.rs b/pbs-tape/src/sg_tape.rs
index d185dd1ac..5cda91943 100644
--- a/pbs-tape/src/sg_tape.rs
+++ b/pbs-tape/src/sg_tape.rs
@@ -962,10 +962,18 @@ impl SgTape {
         let drive_status = self.read_drive_status()?;
 
         let drive_activity = read_device_activity(&mut self.file).ok();
-        let alert_flags = self
-            .tape_alert_flags()
-            .map(|flags| format!("{:?}", flags))
-            .ok();
+
+        // some operations block when the tape moves, which can take up to 2 hours
+        // (e.g. for calibrating) so skip those queries while it's doing that
+        let is_moving = !matches!(drive_activity, None | Some(DeviceActivity::NoActivity));
+
+        let alert_flags = if !is_moving {
+            self.tape_alert_flags()
+                .map(|flags| format!("{:?}", flags))
+                .ok()
+        } else {
+            None
+        };
 
         let mut status = LtoDriveAndMediaStatus {
             vendor: self.info().vendor.clone(),
@@ -993,10 +1001,12 @@ impl SgTape {
                 status.write_protect = Some(drive_status.write_protect);
             }
 
-            let position = self.position()?;
+            if !is_moving {
+                let position = self.position()?;
 
-            status.file_number = Some(position.logical_file_id);
-            status.block_number = Some(position.logical_object_number);
+                status.file_number = Some(position.logical_file_id);
+                status.block_number = Some(position.logical_object_number);
+            }
 
             if let Ok(mam) = self.cartridge_memory() {
                 match mam_extract_media_usage(&mam) {
@@ -1010,20 +1020,22 @@ impl SgTape {
                     }
                 }
 
-                if let Ok(volume_stats) = self.volume_statistics() {
-                    let passes = std::cmp::max(
-                        volume_stats.beginning_of_medium_passes,
-                        volume_stats.middle_of_tape_passes,
-                    );
+                if !is_moving {
+                    if let Ok(volume_stats) = self.volume_statistics() {
+                        let passes = std::cmp::max(
+                            volume_stats.beginning_of_medium_passes,
+                            volume_stats.middle_of_tape_passes,
+                        );
 
-                    // assume max. 16000 medium passes
-                    // see: https://en.wikipedia.org/wiki/Linear_Tape-Open
-                    let wearout: f64 = (passes as f64) / 16000.0_f64;
+                        // assume max. 16000 medium passes
+                        // see: https://en.wikipedia.org/wiki/Linear_Tape-Open
+                        let wearout: f64 = (passes as f64) / 16000.0_f64;
 
-                    status.medium_passes = Some(passes);
-                    status.medium_wearout = Some(wearout);
+                        status.medium_passes = Some(passes);
+                        status.medium_wearout = Some(wearout);
 
-                    status.volume_mounts = Some(volume_stats.volume_mounts);
+                        status.volume_mounts = Some(volume_stats.volume_mounts);
+                    }
                 }
             }
         }
diff --git a/www/tape/DriveStatus.js b/www/tape/DriveStatus.js
index a392202ca..007273f67 100644
--- a/www/tape/DriveStatus.js
+++ b/www/tape/DriveStatus.js
@@ -45,7 +45,7 @@ Ext.define('PBS.TapeManagement.DriveStatus', {
 	onLoad: function() {
 	    let me = this;
 	    let statusgrid = me.lookup('statusgrid');
-	    let online = statusgrid.getObjectValue('file-number') !== undefined;
+	    let online = statusgrid.getObjectValue('file-number') !== undefined || statusgrid.getObjectValue('manufactured');
 	    let vm = me.getViewModel();
 	    vm.set('online', online);
 	    let title = online ? gettext('Status') : gettext('Status (No Tape loaded)');
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 4/4] tape: include drive activity in status
  2024-05-13 10:49 [pbs-devel] [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dominik Csapak
                   ` (2 preceding siblings ...)
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 3/4] tape: drive status: make some depend on the activity Dominik Csapak
@ 2024-05-13 10:49 ` Dominik Csapak
  2024-05-14  8:32 ` [pbs-devel] applied: [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dietmar Maurer
  4 siblings, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2024-05-13 10:49 UTC (permalink / raw)
  To: pbs-devel

Since we don't query each drives status seperately, but rely on a single
call to the drives listing parameter for that, we now add the option
to query the activity there too. This makes that data avaiable for us
to show in a seperate (by default hidden) column.

Also we show the activity in the 'State' column when the drive is idle
from our perspective. This is useful when e.g. an LTO-9 tape is loaded
the first time and is calibrating, since that happens automatically.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-api-types/src/tape/drive.rs |  3 +++
 pbs-tape/src/sg_tape.rs         |  6 ++++++
 src/api2/tape/drive.rs          | 14 ++++++++++++++
 www/Utils.js                    |  5 ++++-
 www/tape/ChangerStatus.js       | 11 +++++++++--
 5 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/pbs-api-types/src/tape/drive.rs b/pbs-api-types/src/tape/drive.rs
index caa6b3b3e..2b788bd67 100644
--- a/pbs-api-types/src/tape/drive.rs
+++ b/pbs-api-types/src/tape/drive.rs
@@ -93,6 +93,9 @@ pub struct DriveListEntry {
     /// the state of the drive if locked
     #[serde(skip_serializing_if = "Option::is_none")]
     pub state: Option<String>,
+    /// Current device activity
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub activity: Option<DeviceActivity>,
 }
 
 #[api()]
diff --git a/pbs-tape/src/sg_tape.rs b/pbs-tape/src/sg_tape.rs
index 5cda91943..f30481b31 100644
--- a/pbs-tape/src/sg_tape.rs
+++ b/pbs-tape/src/sg_tape.rs
@@ -172,6 +172,12 @@ impl SgTape {
         })
     }
 
+    /// Read device activity
+    pub fn device_activity(config: &LtoTapeDrive) -> Result<DeviceActivity, Error> {
+        let mut file = open_lto_tape_device(&config.path)?;
+        read_device_activity(&mut file)
+    }
+
     /// Access to file descriptor - useful for testing
     pub fn file_mut(&mut self) -> &mut File {
         &mut self.file
diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 5a5d39d9e..7a791e09b 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -3,6 +3,7 @@ use std::panic::UnwindSafe;
 use std::sync::Arc;
 
 use anyhow::{bail, format_err, Error};
+use pbs_tape::sg_tape::SgTape;
 use serde_json::Value;
 
 use proxmox_router::{
@@ -1411,6 +1412,12 @@ pub fn catalog_media(
                 schema: CHANGER_NAME_SCHEMA,
                 optional: true,
             },
+            "query-activity": {
+                type: bool,
+                description: "If true, queries and returns the drive activity for each drive.",
+                optional: true,
+                default: false,
+            },
         },
     },
     returns: {
@@ -1428,6 +1435,7 @@ pub fn catalog_media(
 /// List drives
 pub fn list_drives(
     changer: Option<String>,
+    query_activity: bool,
     _param: Value,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Vec<DriveListEntry>, Error> {
@@ -1454,10 +1462,16 @@ pub fn list_drives(
 
         let info = lookup_device_identification(&lto_drives, &drive.path);
         let state = get_tape_device_state(&config, &drive.name)?;
+        let activity = if query_activity {
+            SgTape::device_activity(&drive).ok()
+        } else {
+            None
+        };
         let entry = DriveListEntry {
             config: drive,
             info,
             state,
+            activity,
         };
         list.push(entry);
     }
diff --git a/www/Utils.js b/www/Utils.js
index df03a4c87..4853be36c 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -721,8 +721,11 @@ Ext.define('PBS.Utils', {
 	return PBS.Utils.tapeDriveActivities[value] ?? value;
     },
 
-    renderDriveState: function(value, md) {
+    renderDriveState: function(value, md, rec) {
 	if (!value) {
+	    if (rec?.data?.activity && rec?.data?.activity !== 'no-activity') {
+		return PBS.Utils.renderDriveActivity(rec.data.activity);
+	    }
 	    return gettext('Idle');
 	}
 
diff --git a/www/tape/ChangerStatus.js b/www/tape/ChangerStatus.js
index fdafc459e..e18af90e4 100644
--- a/www/tape/ChangerStatus.js
+++ b/www/tape/ChangerStatus.js
@@ -1,6 +1,6 @@
 Ext.define('pbs-slot-model', {
     extend: 'Ext.data.Model',
-    fields: ['entry-id', 'label-text', 'is-labeled', ' model', 'name', 'vendor', 'serial', 'state', 'status', 'pool',
+    fields: ['entry-id', 'label-text', 'is-labeled', ' model', 'name', 'vendor', 'serial', 'state', 'status', 'pool', 'activity',
 	{
 	    name: 'is-blocked',
 	    calculate: function(data) {
@@ -488,7 +488,7 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 		});
 		let drives_fut = Proxmox.Async.api2({
 		    timeout: 5*60*1000,
-		    url: `/api2/extjs/tape/drive?changer=${encodeURIComponent(changer)}`,
+		    url: `/api2/extjs/tape/drive?query-activity=true&changer=${encodeURIComponent(changer)}`,
 		});
 
 		let tapes_fut = Proxmox.Async.api2({
@@ -852,6 +852,13 @@ Ext.define('PBS.TapeManagement.ChangerStatus', {
 				    flex: 1,
 				    renderer: Ext.htmlEncode,
 				},
+				{
+				    text: gettext('Activity'),
+				    dataIndex: 'activity',
+				    renderer: PBS.Utils.renderDriveActivity,
+				    hidden: true,
+				    flex: 1,
+				},
 				{
 				    text: gettext('State'),
 				    dataIndex: 'state',
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pbs-devel] applied: [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity
  2024-05-13 10:49 [pbs-devel] [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dominik Csapak
                   ` (3 preceding siblings ...)
  2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 4/4] tape: include drive activity in status Dominik Csapak
@ 2024-05-14  8:32 ` Dietmar Maurer
  4 siblings, 0 replies; 6+ messages in thread
From: Dietmar Maurer @ 2024-05-14  8:32 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

applied
> On 13.5.2024 12:49 CEST Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> Whenever an external activity is done on a tape drive (e.g. via `pmt`, a
> changer webui, or an automatic action like calibrating), most of our
> tape commands will hang and nothing on the webui is showing any
> activity.
> 
> This is most annoying when loading new LTO-9 tapes into a drive, since
> it is blocked up to two hours, without any indication that work is done.
> 
> This can be frustrating, so this series queries and shows the device
> activity from the drive when it's suitable.
> 
> Dominik Csapak (4):
>   tape: add functions to parse drive device activity
>   tape: add drive activity to drive status api
>   tape: drive status: make some depend on the activity
>   tape: include drive activity in status
> 
>  pbs-api-types/src/tape/drive.rs           | 71 ++++++++++++++++
>  pbs-tape/src/sg_tape.rs                   | 62 +++++++++-----
>  pbs-tape/src/sg_tape/device_status.rs     | 99 +++++++++++++++++++++++
>  pbs-tape/src/sg_tape/volume_statistics.rs |  8 +-
>  src/api2/tape/drive.rs                    | 14 ++++
>  www/Utils.js                              | 34 +++++++-
>  www/tape/ChangerStatus.js                 | 11 ++-
>  www/tape/DriveStatus.js                   |  6 +-
>  8 files changed, 278 insertions(+), 27 deletions(-)
>  create mode 100644 pbs-tape/src/sg_tape/device_status.rs
> 
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-14  8:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-13 10:49 [pbs-devel] [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dominik Csapak
2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/4] tape: add functions to parse drive " Dominik Csapak
2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 2/4] tape: add drive activity to drive status api Dominik Csapak
2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 3/4] tape: drive status: make some depend on the activity Dominik Csapak
2024-05-13 10:49 ` [pbs-devel] [PATCH proxmox-backup 4/4] tape: include drive activity in status Dominik Csapak
2024-05-14  8:32 ` [pbs-devel] applied: [PATCH proxmox-backup 0/4] tape: improve UX by showing device activity Dietmar Maurer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal