tape: introduce trait BlockRead

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 {

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<Option<Box<dyn TapeRead + 'a>>, 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<BlockReadStatus, std::io::Error> {
+        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),
+            }
+        }
+    }
+}

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 = EmulateTapeReader::new(file);
-                let reader = Box::new(EmulateTapeReader::new(reader));
 
                 match BlockedReader::open(reader)? {
                     Some(reader) => Ok(Some(Box::new(reader))),

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<R> {
     read_pos: usize,
 }
 
-impl <R: Read> BlockedReader<R> {
+impl <R: BlockRead> BlockedReader<R> {
 
     /// Create a new BlockedReader instance.
     ///
@@ -103,15 +104,41 @@ impl <R: Read> BlockedReader<R> {
             )
         };
 
-        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)? {
+        match reader.read_block(&mut tmp_buf) {
-            proxmox::io_bail!("detected tape block after stream end marker");
+            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<usize, std::io::Error> {
@@ -141,7 +168,7 @@ impl <R: Read> BlockedReader<R> {
     }
 }
 
-impl <R: Read> TapeRead for BlockedReader<R> {
+impl <R: BlockRead> TapeRead for BlockedReader<R> {
 
     fn is_incomplete(&self) -> Result<bool, std::io::Error> {
         if !self.got_eod {
@@ -163,7 +190,7 @@ impl <R: Read> TapeRead for BlockedReader<R> {
     }
 }
 
-impl <R: Read> Read for BlockedReader<R> {
+impl <R: BlockRead> Read for BlockedReader<R> {
 
     fn read(&mut self, buffer: &mut [u8]) -> Result<usize, std::io::Error> {
 
@@ -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.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.finish(false)?;
+        }
 
         let reader = &mut &tape_data[..];
+        let reader = EmulateTapeReader::new(reader);
         let mut reader = BlockedReader::open(reader)?.unwrap();
 
         let mut buf = [0u8; 1];

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<R> {
+pub struct EmulateTapeReader<R: Read> {
     reader: R,
+    got_eof: bool,
 }
 
 impl <R: Read> EmulateTapeReader<R> {
 
     pub fn new(reader: R) -> Self {
-        Self { reader }
+        Self { reader, got_eof: false }
     }
 }
 
-impl <R: Read> Read for EmulateTapeReader<R> {
+impl <R: Read> BlockRead for EmulateTapeReader<R> {
-
+    fn read_block(&mut self, buffer: &mut [u8]) -> Result<BlockReadStatus, std::io::Error> {
-    fn read(&mut self, mut buffer: &mut [u8]) -> Result<usize, io::Error> {
+        if self.got_eof {
-
+            proxmox::io_bail!("detected read after EOF!");
-        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..];
         }
-                Err(ref e) if e.kind() == io::ErrorKind::Interrupted => {}
+        match self.reader.read_exact_or_eof(buffer)? {
-                Err(e) => return Err(e),
+            false => {
+                self.got_eof = true;
+                Ok(BlockReadStatus::EndOfFile)
             }
-        }
+            true => {
-
-        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 {
+                if buffer.len() != PROXMOX_TAPE_BLOCK_SIZE {
-            proxmox::io_bail!("EmulateTapeReader: got read with wrong block size ({} != {})",
+                    proxmox::io_bail!("EmulateTapeReader: read_block with wrong block size ({} != {})",
                                       buffer.len(), PROXMOX_TAPE_BLOCK_SIZE);
                 }
-
+                Ok(BlockReadStatus::Ok(buffer.len()))
-        if !buffer.is_empty() {
+            }
-            Err(io::Error::new(io::ErrorKind::UnexpectedEof, "failed to fill whole buffer"))
-        } else {
-            Ok(bytes)
         }
     }
 }

src/tape/tape_read.rs

@@ -15,31 +15,14 @@ pub trait TapeRead: Read {
     fn has_end_marker(&self) -> Result<bool, std::io::Error>;
 }
 
-/// Read a single block from a tape device
+pub enum BlockReadStatus {
-///
+    Ok(usize),
-/// Assumes that 'reader' is a linux tape device.
+    EndOfFile,
-///
+    EndOfStream,
-/// Return true on success, false on EOD
+}
-pub fn tape_device_read_block<R: Read>(
+
-    reader: &mut R,
+/// Read streams of blocks
-    buffer: &mut [u8],
+pub trait BlockRead {
-) -> Result<bool, std::io::Error> {
+    /// Read the next block (whole buffer)
-
+    fn read_block(&mut self, buffer: &mut [u8]) -> Result<BlockReadStatus, std::io::Error>;
-    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),
-        }
-    }
 }