From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id E71FD905FA for ; Thu, 30 Mar 2023 13:29:18 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C6DD21B49D for ; Thu, 30 Mar 2023 13:28:48 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 30 Mar 2023 13:28:47 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 7D2814195C for ; Thu, 30 Mar 2023 13:28:47 +0200 (CEST) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Thu, 30 Mar 2023 13:28:44 +0200 Message-Id: <20230330112845.2620964-3-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230330112845.2620964-1-d.csapak@proxmox.com> References: <20230330112845.2620964-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.140 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_2 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_4 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup 3/4] tape: implement 6 byte fallback for MODE SENSE/SELECT X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Mar 2023 11:29:18 -0000 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 --- 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 for ModeParameterHeader6 { + type Error = Error; + + fn try_from(value: ModeParameterHeader) -> Result { + 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 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 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 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(file: &mut F) -> Result { .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( 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::(); + 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::(); - 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::(); + 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 = 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