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 3E58D7114E for ; Wed, 7 Apr 2021 12:24:13 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3BF9FF3B6 for ; Wed, 7 Apr 2021 12:24:13 +0200 (CEST) Received: from elsa.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0FA31F335 for ; Wed, 7 Apr 2021 12:24:07 +0200 (CEST) Received: by elsa.proxmox.com (Postfix, from userid 0) id 01891AEAF1C; Wed, 7 Apr 2021 12:24:06 +0200 (CEST) From: Dietmar Maurer To: pbs-devel@lists.proxmox.com Date: Wed, 7 Apr 2021 12:22:58 +0200 Message-Id: <20210407102308.9750-2-dietmar@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210407102308.9750-1-dietmar@proxmox.com> References: <20210407102308.9750-1-dietmar@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [drive.rs] Subject: [pbs-devel] [PATCH 01/11] tape: introduce trait BlockRead 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: Wed, 07 Apr 2021 10:24:13 -0000 --- src/api2/tape/drive.rs | 1 + src/tape/drive/linux_tape.rs | 53 ++++++++++++++++++- src/tape/drive/virtual_tape.rs | 3 +- src/tape/file_formats/blocked_reader.rs | 70 +++++++++++++++++++------ src/tape/helpers/emulate_tape_reader.rs | 64 ++++++++++------------ src/tape/tape_read.rs | 35 ++++--------- 6 files changed, 145 insertions(+), 81 deletions(-) diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs index b753eb5b..b17f4203 100644 --- a/src/api2/tape/drive.rs +++ b/src/api2/tape/drive.rs @@ -531,6 +531,7 @@ pub fn label_media( Ok(Some(_file)) => bail!("media is not empty (erase first)"), Ok(None) => { /* EOF mark at BOT, assume tape is empty */ }, Err(err) => { + println!("TEST {:?}", err); if err.is_errno(nix::errno::Errno::ENOSPC) || err.is_errno(nix::errno::Errno::EIO) { /* assume tape is empty */ } else { diff --git a/src/tape/drive/linux_tape.rs b/src/tape/drive/linux_tape.rs index f8949196..9e86d0a3 100644 --- a/src/tape/drive/linux_tape.rs +++ b/src/tape/drive/linux_tape.rs @@ -1,6 +1,7 @@ //! Driver for Linux SCSI tapes use std::fs::{OpenOptions, File}; +use std::io::Read; use std::os::unix::fs::OpenOptionsExt; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::convert::TryFrom; @@ -24,6 +25,8 @@ use crate::{ LinuxDriveAndMediaStatus, }, tape::{ + BlockRead, + BlockReadStatus, TapeRead, TapeWrite, drive::{ @@ -507,7 +510,8 @@ impl TapeDriver for LinuxTapeHandle { } fn read_next_file<'a>(&'a mut self) -> Result>, std::io::Error> { - match BlockedReader::open(&mut self.file)? { + let reader = LinuxTapeReader::new(&mut self.file); + match BlockedReader::open(reader)? { Some(reader) => Ok(Some(Box::new(reader))), None => Ok(None), } @@ -766,3 +770,50 @@ impl TapeWrite for TapeWriterHandle<'_> { self.writer.logical_end_of_media() } } + +pub struct LinuxTapeReader<'a> { + /// Assumes that 'file' is a linux tape device. + file: &'a mut File, + got_eof: bool, +} + +impl <'a> LinuxTapeReader<'a> { + + pub fn new(file: &'a mut File) -> Self { + Self { file, got_eof: false } + } +} + +impl <'a> BlockRead for LinuxTapeReader<'a> { + + /// Read a single block from a linux tape device + /// + /// Return true on success, false on EOD + fn read_block(&mut self, buffer: &mut [u8]) -> Result { + loop { + match self.file.read(buffer) { + Ok(0) => { + let eod = self.got_eof; + self.got_eof = true; + if eod { + return Ok(BlockReadStatus::EndOfStream); + } else { + return Ok(BlockReadStatus::EndOfFile); + } + } + Ok(count) => { + if count == buffer.len() { + return Ok(BlockReadStatus::Ok(count)); + } + proxmox::io_bail!("short block read ({} < {}). Tape drive uses wrong block size.", + count, buffer.len()); + } + // handle interrupted system call + Err(err) if err.kind() == std::io::ErrorKind::Interrupted => { + continue; + } + Err(err) => return Err(err), + } + } + } +} diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs index d6b3d0c9..2fc9fdea 100644 --- a/src/tape/drive/virtual_tape.rs +++ b/src/tape/drive/virtual_tape.rs @@ -222,8 +222,7 @@ impl TapeDriver for VirtualTapeHandle { self.store_status(&status) .map_err(|err| io::Error::new(io::ErrorKind::Other, err.to_string()))?; - let reader = Box::new(file); - let reader = Box::new(EmulateTapeReader::new(reader)); + let reader = EmulateTapeReader::new(file); match BlockedReader::open(reader)? { Some(reader) => Ok(Some(Box::new(reader))), diff --git a/src/tape/file_formats/blocked_reader.rs b/src/tape/file_formats/blocked_reader.rs index 3ef7e7f4..3df84a1b 100644 --- a/src/tape/file_formats/blocked_reader.rs +++ b/src/tape/file_formats/blocked_reader.rs @@ -2,7 +2,8 @@ use std::io::Read; use crate::tape::{ TapeRead, - tape_device_read_block, + BlockRead, + BlockReadStatus, file_formats::{ PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0, BlockHeader, @@ -32,7 +33,7 @@ pub struct BlockedReader { read_pos: usize, } -impl BlockedReader { +impl BlockedReader { /// Create a new BlockedReader instance. /// @@ -103,15 +104,41 @@ impl BlockedReader { ) }; - tape_device_read_block(reader, data) + match reader.read_block(data) { + Ok(BlockReadStatus::Ok(bytes)) => { + if bytes != BlockHeader::SIZE { + proxmox::io_bail!("got wrong block size"); + } + Ok(true) + } + Ok(BlockReadStatus::EndOfFile) => { + Ok(false) + } + Ok(BlockReadStatus::EndOfStream) => { + return Err(std::io::Error::from_raw_os_error(nix::errno::Errno::ENOSPC as i32)); + } + Err(err) => { + Err(err) + } + } } fn consume_eof_marker(reader: &mut R) -> Result<(), std::io::Error> { let mut tmp_buf = [0u8; 512]; // use a small buffer for testing EOF - if tape_device_read_block(reader, &mut tmp_buf)? { - proxmox::io_bail!("detected tape block after stream end marker"); + match reader.read_block(&mut tmp_buf) { + Ok(BlockReadStatus::Ok(_)) => { + proxmox::io_bail!("detected tape block after block-stream end marker"); + } + Ok(BlockReadStatus::EndOfFile) => { + return Ok(()); + } + Ok(BlockReadStatus::EndOfStream) => { + proxmox::io_bail!("got unexpected end of tape"); + } + Err(err) => { + return Err(err); + } } - Ok(()) } fn read_block(&mut self) -> Result { @@ -141,7 +168,7 @@ impl BlockedReader { } } -impl TapeRead for BlockedReader { +impl TapeRead for BlockedReader { fn is_incomplete(&self) -> Result { if !self.got_eod { @@ -163,7 +190,7 @@ impl TapeRead for BlockedReader { } } -impl Read for BlockedReader { +impl Read for BlockedReader { fn read(&mut self, buffer: &mut [u8]) -> Result { @@ -207,6 +234,7 @@ mod test { use anyhow::Error; use crate::tape::{ TapeWrite, + helpers::{EmulateTapeReader, EmulateTapeWriter}, file_formats::{ PROXMOX_TAPE_BLOCK_SIZE, BlockedReader, @@ -218,11 +246,14 @@ mod test { let mut tape_data = Vec::new(); - let mut writer = BlockedWriter::new(&mut tape_data); + { + let writer = EmulateTapeWriter::new(&mut tape_data, 1024*1024*10); + let mut writer = BlockedWriter::new(writer); - writer.write_all(data)?; + writer.write_all(data)?; - writer.finish(false)?; + writer.finish(false)?; + } assert_eq!( tape_data.len(), @@ -231,6 +262,7 @@ mod test { ); let reader = &mut &tape_data[..]; + let reader = EmulateTapeReader::new(reader); let mut reader = BlockedReader::open(reader)?.unwrap(); let mut read_data = Vec::with_capacity(PROXMOX_TAPE_BLOCK_SIZE); @@ -263,6 +295,7 @@ mod test { fn no_data() -> Result<(), Error> { let tape_data = Vec::new(); let reader = &mut &tape_data[..]; + let reader = EmulateTapeReader::new(reader); let reader = BlockedReader::open(reader)?; assert!(reader.is_none()); @@ -273,13 +306,16 @@ mod test { fn no_end_marker() -> Result<(), Error> { let mut tape_data = Vec::new(); { - let mut writer = BlockedWriter::new(&mut tape_data); + let writer = EmulateTapeWriter::new(&mut tape_data, 1024*1024); + let mut writer = BlockedWriter::new(writer); // write at least one block let data = proxmox::sys::linux::random_data(PROXMOX_TAPE_BLOCK_SIZE)?; writer.write_all(&data)?; // but do not call finish here } + let reader = &mut &tape_data[..]; + let reader = EmulateTapeReader::new(reader); let mut reader = BlockedReader::open(reader)?.unwrap(); let mut data = Vec::with_capacity(PROXMOX_TAPE_BLOCK_SIZE); @@ -292,13 +328,17 @@ mod test { fn small_read_buffer() -> Result<(), Error> { let mut tape_data = Vec::new(); - let mut writer = BlockedWriter::new(&mut tape_data); + { + let writer = EmulateTapeWriter::new(&mut tape_data, 1024*1024); + let mut writer = BlockedWriter::new(writer); - writer.write_all(b"ABC")?; + writer.write_all(b"ABC")?; - writer.finish(false)?; + writer.finish(false)?; + } let reader = &mut &tape_data[..]; + let reader = EmulateTapeReader::new(reader); let mut reader = BlockedReader::open(reader)?.unwrap(); let mut buf = [0u8; 1]; diff --git a/src/tape/helpers/emulate_tape_reader.rs b/src/tape/helpers/emulate_tape_reader.rs index 1b6d4c5e..fbb19028 100644 --- a/src/tape/helpers/emulate_tape_reader.rs +++ b/src/tape/helpers/emulate_tape_reader.rs @@ -1,56 +1,46 @@ -use std::io::{self, Read}; +use std::io::Read; -use crate::tape::file_formats::PROXMOX_TAPE_BLOCK_SIZE; +use proxmox::tools::io::ReadExt; + +use crate::tape::{ + BlockRead, + BlockReadStatus, + file_formats::PROXMOX_TAPE_BLOCK_SIZE, +}; /// Emulate tape read behavior on a normal Reader /// /// Tapes reads are always return one whole block PROXMOX_TAPE_BLOCK_SIZE. -pub struct EmulateTapeReader { +pub struct EmulateTapeReader { reader: R, + got_eof: bool, } impl EmulateTapeReader { pub fn new(reader: R) -> Self { - Self { reader } + Self { reader, got_eof: false } } } -impl Read for EmulateTapeReader { - - fn read(&mut self, mut buffer: &mut [u8]) -> Result { - - let initial_buffer_len = buffer.len(); // store, check later - - let mut bytes = 0; - - while !buffer.is_empty() { - match self.reader.read(buffer) { - Ok(0) => break, - Ok(n) => { - bytes += n; - let tmp = buffer; - buffer = &mut tmp[n..]; +impl BlockRead for EmulateTapeReader { + fn read_block(&mut self, buffer: &mut [u8]) -> Result { + if self.got_eof { + proxmox::io_bail!("detected read after EOF!"); + } + match self.reader.read_exact_or_eof(buffer)? { + false => { + self.got_eof = true; + Ok(BlockReadStatus::EndOfFile) + } + true => { + // test buffer len after EOF test (to allow EOF test with small buffers in BufferedReader) + if buffer.len() != PROXMOX_TAPE_BLOCK_SIZE { + proxmox::io_bail!("EmulateTapeReader: read_block with wrong block size ({} != {})", + buffer.len(), PROXMOX_TAPE_BLOCK_SIZE); } - Err(ref e) if e.kind() == io::ErrorKind::Interrupted => {} - Err(e) => return Err(e), + Ok(BlockReadStatus::Ok(buffer.len())) } } - - if bytes == 0 { - return Ok(0); - } - - // test buffer len after EOF test (to allow EOF test with small buffers in BufferedReader) - if initial_buffer_len != PROXMOX_TAPE_BLOCK_SIZE { - proxmox::io_bail!("EmulateTapeReader: got read with wrong block size ({} != {})", - buffer.len(), PROXMOX_TAPE_BLOCK_SIZE); - } - - if !buffer.is_empty() { - Err(io::Error::new(io::ErrorKind::UnexpectedEof, "failed to fill whole buffer")) - } else { - Ok(bytes) - } } } diff --git a/src/tape/tape_read.rs b/src/tape/tape_read.rs index c1ba52e6..7cbe2b8b 100644 --- a/src/tape/tape_read.rs +++ b/src/tape/tape_read.rs @@ -15,31 +15,14 @@ pub trait TapeRead: Read { fn has_end_marker(&self) -> Result; } -/// Read a single block from a tape device -/// -/// Assumes that 'reader' is a linux tape device. -/// -/// Return true on success, false on EOD -pub fn tape_device_read_block( - reader: &mut R, - buffer: &mut [u8], -) -> Result { +pub enum BlockReadStatus { + Ok(usize), + EndOfFile, + EndOfStream, +} - loop { - match reader.read(buffer) { - Ok(0) => { return Ok(false); /* EOD */ } - Ok(count) => { - if count == buffer.len() { - return Ok(true); - } - proxmox::io_bail!("short block read ({} < {}). Tape drive uses wrong block size.", - count, buffer.len()); - } - // handle interrupted system call - Err(err) if err.kind() == std::io::ErrorKind::Interrupted => { - continue; - } - Err(err) => return Err(err), - } - } +/// Read streams of blocks +pub trait BlockRead { + /// Read the next block (whole buffer) + fn read_block(&mut self, buffer: &mut [u8]) -> Result; } -- 2.20.1