From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 3/4] tape: implement 6 byte fallback for MODE SENSE/SELECT
Date: Thu, 30 Mar 2023 13:28:44 +0200 [thread overview]
Message-ID: <20230330112845.2620964-3-d.csapak@proxmox.com> (raw)
In-Reply-To: <20230330112845.2620964-1-d.csapak@proxmox.com>
there are tape drives (esp. virtual ones) that don't implement the
10-byte variants of MODE SENSE/SELECT. Since the pages we set/request
are never bigger than 255 bytes anyway, we can implement a fallback
with the 6 byte variant here.
Implementing this as a fallback to make sure that existing working
drives keep the existing implementation.
Tested with Starwind VTL.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
pbs-tape/src/sg_tape.rs | 44 ++++++++-
pbs-tape/src/sgutils2.rs | 197 ++++++++++++++++++++++++++++++++-------
2 files changed, 202 insertions(+), 39 deletions(-)
diff --git a/pbs-tape/src/sg_tape.rs b/pbs-tape/src/sg_tape.rs
index 6a5569ac..97d90201 100644
--- a/pbs-tape/src/sg_tape.rs
+++ b/pbs-tape/src/sg_tape.rs
@@ -30,8 +30,9 @@ use pbs_api_types::{Lp17VolumeStatistics, LtoDriveAndMediaStatus, MamAttribute};
use crate::{
sgutils2::{
- alloc_page_aligned_buffer, scsi_inquiry, scsi_mode_sense, scsi_request_sense, InquiryInfo,
- ModeBlockDescriptor, ModeParameterHeader, ScsiError, SenseInfo, SgRaw,
+ alloc_page_aligned_buffer, scsi_inquiry, scsi_mode_sense, scsi_request_sense,
+ sense_err_is_invalid_command, InquiryInfo, ModeBlockDescriptor, ModeParameterHeader,
+ ModeParameterHeader10, ModeParameterHeader6, ScsiError, SenseInfo, SgRaw,
},
BlockRead, BlockReadError, BlockWrite, BlockedReader, BlockedWriter,
};
@@ -764,6 +765,7 @@ impl SgTape {
let mut data = Vec::new();
unsafe {
+ let head: ModeParameterHeader10 = head.clone().into();
data.write_be_value(head)?;
data.write_be_value(block_descriptor)?;
data.write_be_value(page)?;
@@ -782,9 +784,41 @@ impl SgTape {
buffer[..data.len()].copy_from_slice(&data[..]);
- sg_raw
- .do_out_command(&cmd, &buffer[..data.len()])
- .map_err(|err| format_err!("set drive options failed - {}", err))?;
+ match sg_raw.do_out_command(&cmd, &buffer[..data.len()]) {
+ Ok(()) => {}
+ Err(ScsiError::Sense(err)) if sense_err_is_invalid_command(&err) => {
+ let mut data = Vec::new();
+ unsafe {
+ let head: ModeParameterHeader6 = head.try_into()?;
+ data.write_be_value(head)?;
+ data.write_be_value(block_descriptor)?;
+ data.write_be_value(page)?;
+ }
+
+ let mut cmd = Vec::new();
+ cmd.push(0x15); // MODE SELECT(6)
+ cmd.push(0b0001_0000); // PF=1
+ cmd.extend([0, 0]); //reserved
+
+ if data.len() > u8::MAX as usize {
+ bail!("set drive options (mode select(6)) failed - parameters too long")
+ }
+
+ cmd.push(data.len() as u8);
+ cmd.push(0); // control
+
+ let mut buffer = alloc_page_aligned_buffer(4096)?;
+
+ buffer[..data.len()].copy_from_slice(&data[..]);
+
+ sg_raw
+ .do_out_command(&cmd, &buffer[..data.len()])
+ .map_err(|err| {
+ format_err!("set drive options (mode select(6)) failed - {err}")
+ })?;
+ }
+ Err(err) => bail!("set drive options (mode select(10)) failed - {err}"),
+ }
Ok(())
}
diff --git a/pbs-tape/src/sgutils2.rs b/pbs-tape/src/sgutils2.rs
index dd1aa1b4..91444cec 100644
--- a/pbs-tape/src/sgutils2.rs
+++ b/pbs-tape/src/sgutils2.rs
@@ -224,7 +224,7 @@ pub struct InquiryInfo {
#[repr(C, packed)]
#[derive(Endian, Debug, Copy, Clone)]
-pub struct ModeParameterHeader {
+pub struct ModeParameterHeader10 {
pub mode_data_len: u16,
// Note: medium_type and density_code are not the same. On HP
// drives, medium_type provides very limited information and is
@@ -232,27 +232,106 @@ pub struct ModeParameterHeader {
pub medium_type: u8,
pub flags3: u8,
reserved4: [u8; 2],
- pub block_descriptior_len: u16,
+ pub block_descriptor_len: u16,
+}
+
+// header for the short variant of MODE SENSE/SELECT
+#[repr(C, packed)]
+#[derive(Endian, Debug, Copy, Clone)]
+pub struct ModeParameterHeader6 {
+ pub mode_data_len: u8,
+ // Note: medium_type and density_code are not the same. On HP
+ // drives, medium_type provides very limited information and is
+ // not compatible with IBM.
+ pub medium_type: u8,
+ pub flags2: u8,
+ pub block_descriptor_len: u8,
+}
+
+#[derive(Clone)]
+pub struct ModeParameterHeader {
+ pub mode_data_len: u16,
+ // Note: medium_type and density_code are not the same. On HP
+ // drives, medium_type provides very limited information and is
+ // not compatible with IBM.
+ pub medium_type: u8,
+ pub flags: u8,
+ pub block_descriptor_len: u16,
+}
+
+impl TryFrom<ModeParameterHeader> for ModeParameterHeader6 {
+ type Error = Error;
+
+ fn try_from(value: ModeParameterHeader) -> Result<Self, Self::Error> {
+ if value.mode_data_len > u8::MAX as u16 {
+ bail!("mode_data_len too big for 6 byte mode parameter header")
+ }
+
+ if value.block_descriptor_len > u8::MAX as u16 {
+ bail!("block_descriptor_len too big for 6 byte mode parameter header")
+ }
+
+ Ok(Self {
+ mode_data_len: value.mode_data_len as u8,
+ medium_type: value.medium_type,
+ flags2: value.flags,
+ block_descriptor_len: value.block_descriptor_len as u8,
+ })
+ }
+}
+
+impl From<ModeParameterHeader> for ModeParameterHeader10 {
+ fn from(value: ModeParameterHeader) -> Self {
+ Self {
+ mode_data_len: value.mode_data_len,
+ medium_type: value.medium_type,
+ flags3: value.flags,
+ block_descriptor_len: value.block_descriptor_len,
+ reserved4: [0, 0],
+ }
+ }
+}
+
+impl From<ModeParameterHeader10> for ModeParameterHeader {
+ fn from(val: ModeParameterHeader10) -> Self {
+ Self {
+ mode_data_len: val.mode_data_len,
+ medium_type: val.medium_type,
+ flags: val.flags3,
+ block_descriptor_len: val.block_descriptor_len,
+ }
+ }
+}
+
+impl From<ModeParameterHeader6> for ModeParameterHeader {
+ fn from(val: ModeParameterHeader6) -> Self {
+ Self {
+ mode_data_len: val.mode_data_len as u16,
+ medium_type: val.medium_type,
+ flags: val.flags2,
+ block_descriptor_len: val.block_descriptor_len as u16,
+ }
+ }
}
impl ModeParameterHeader {
#[allow(clippy::unusual_byte_groupings)]
pub fn buffer_mode(&self) -> u8 {
- (self.flags3 & 0b0_111_0000) >> 4
+ (self.flags & 0b0_111_0000) >> 4
}
#[allow(clippy::unusual_byte_groupings)]
pub fn set_buffer_mode(&mut self, buffer_mode: bool) {
- let mut mode = self.flags3 & 0b1_000_1111;
+ let mut mode = self.flags & 0b1_000_1111;
if buffer_mode {
mode |= 0b0_001_0000;
}
- self.flags3 = mode;
+ self.flags = mode;
}
#[allow(clippy::unusual_byte_groupings)]
pub fn write_protect(&self) -> bool {
- (self.flags3 & 0b1_000_0000) != 0
+ (self.flags & 0b1_000_0000) != 0
}
}
@@ -670,6 +749,13 @@ pub fn scsi_inquiry<F: AsRawFd>(file: &mut F) -> Result<InquiryInfo, Error> {
.map_err(|err: Error| format_err!("decode inquiry page failed - {}", err))
}
+/// True if the given sense info is INVALID COMMAND OPERATION CODE
+/// means that the device does not know/support the command
+/// https://www.t10.org/lists/asc-num.htm#ASC_20
+pub fn sense_err_is_invalid_command(err: &SenseInfo) -> bool {
+ err.sense_key == SENSE_KEY_ILLEGAL_REQUEST && err.asc == 0x20 && err.ascq == 0x00
+}
+
/// Run SCSI Mode Sense
///
/// Warning: P needs to be repr(C, packed)]
@@ -695,43 +781,86 @@ pub fn scsi_mode_sense<F: AsRawFd, P: Endian>(
cmd.extend(allocation_len.to_be_bytes()); // allocation len
cmd.push(0); //control
- let data = sg_raw
- .do_command(&cmd)
- .map_err(|err| format_err!("mode sense failed - {}", err))?;
-
- proxmox_lang::try_block!({
- let mut reader = data;
+ let (head, mut reader): (ModeParameterHeader, &[u8]) = match sg_raw.do_command(&cmd) {
+ Ok(data) => {
+ let mut reader = data;
+
+ let head: ModeParameterHeader10 = unsafe { reader.read_be_value()? };
+ let expected_len = head.mode_data_len as usize + 2;
+
+ use std::cmp::Ordering;
+ match data.len().cmp(&expected_len) {
+ Ordering::Less => bail!(
+ "wrong mode_data_len: got {}, expected {}",
+ data.len(),
+ expected_len
+ ),
+ Ordering::Greater => {
+ // Note: Some hh7 drives returns the allocation_length
+ // instead of real data_len
+ let header_size = std::mem::size_of::<ModeParameterHeader10>();
+ reader = &data[header_size..expected_len];
+ }
+ _ => (),
+ }
- let head: ModeParameterHeader = unsafe { reader.read_be_value()? };
- let expected_len = head.mode_data_len as usize + 2;
-
- use std::cmp::Ordering;
- match data.len().cmp(&expected_len) {
- Ordering::Less => bail!(
- "wrong mode_data_len: got {}, expected {}",
- data.len(),
- expected_len
- ),
- Ordering::Greater => {
- // Note: Some hh7 drives returns the allocation_length
- // instead of real data_len
- let header_size = std::mem::size_of::<ModeParameterHeader>();
- reader = &data[header_size..expected_len];
+ (head.into(), reader)
+ }
+ Err(ScsiError::Sense(err)) if sense_err_is_invalid_command(&err) => {
+ // fall back to small mode sense
+ let mut cmd = vec![0x1A]; // MODE SENSE(6)
+ if disable_block_descriptor {
+ cmd.push(8); // DBD=1 (Disable Block Descriptors)
+ } else {
+ cmd.push(0); // DBD=0 (Include Block Descriptors)
+ }
+ cmd.push(page_code & 63); // report current values for page_code
+ cmd.push(sub_page_code);
+
+ cmd.push(0xFF); // allocation len
+ cmd.push(0); //control
+ let data = sg_raw
+ .do_command(&cmd)
+ .map_err(|err| format_err!("mode sense(6) failed - {err}"))?;
+
+ let mut reader = data;
+
+ let head: ModeParameterHeader6 = unsafe { reader.read_be_value()? };
+ let expected_len = head.mode_data_len as usize + 1;
+
+ use std::cmp::Ordering;
+ match data.len().cmp(&expected_len) {
+ Ordering::Less => bail!(
+ "wrong mode_data_len: got {}, expected {}",
+ data.len(),
+ expected_len
+ ),
+ Ordering::Greater => {
+ // Note: Some hh7 drives returns the allocation_length
+ // instead of real data_len
+ let header_size = std::mem::size_of::<ModeParameterHeader6>();
+ reader = &data[header_size..expected_len];
+ }
+ _ => (),
}
- _ => (),
+
+ (head.into(), reader)
}
+ Err(err) => bail!("mode sense(10) failed - {err}"),
+ };
- if disable_block_descriptor && head.block_descriptior_len != 0 {
- let len = head.block_descriptior_len;
- bail!("wrong block_descriptior_len: {}, expected 0", len);
+ proxmox_lang::try_block!({
+ if disable_block_descriptor && head.block_descriptor_len != 0 {
+ let len = head.block_descriptor_len;
+ bail!("wrong block_descriptor_len: {}, expected 0", len);
}
let mut block_descriptor: Option<ModeBlockDescriptor> = None;
if !disable_block_descriptor {
- if head.block_descriptior_len != 8 {
- let len = head.block_descriptior_len;
- bail!("wrong block_descriptior_len: {}, expected 8", len);
+ if head.block_descriptor_len != 8 {
+ let len = head.block_descriptor_len;
+ bail!("wrong block_descriptor_len: {}, expected 8", len);
}
block_descriptor = Some(unsafe { reader.read_be_value()? });
--
2.30.2
next prev parent reply other threads:[~2023-03-30 11:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-30 11:28 [pbs-devel] [PATCH proxmox-backup 1/4] tape: use correct MAM attribute definitions Dominik Csapak
2023-03-30 11:28 ` [pbs-devel] [PATCH proxmox-backup 2/4] tape: continue without MAM media_usage page Dominik Csapak
2023-04-20 12:19 ` [pbs-devel] applied " Dietmar Maurer
2023-03-30 11:28 ` Dominik Csapak [this message]
2023-03-30 13:38 ` [pbs-devel] [PATCH proxmox-backup 3/4] tape: implement 6 byte fallback for MODE SENSE/SELECT Dominik Csapak
2023-03-30 11:28 ` [pbs-devel] [PATCH proxmox-backup 4/4] tape: typo fixes Dominik Csapak
2023-04-20 12:18 ` [pbs-devel] applied [PATCH proxmox-backup 1/4] tape: use correct MAM attribute definitions Dietmar Maurer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230330112845.2620964-3-d.csapak@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal