all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dietmar Maurer <dietmar@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH 01/11] tape: introduce trait BlockRead
Date: Wed,  7 Apr 2021 12:22:58 +0200	[thread overview]
Message-ID: <20210407102308.9750-2-dietmar@proxmox.com> (raw)
In-Reply-To: <20210407102308.9750-1-dietmar@proxmox.com>

---
 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<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),
+            }
+        }
+    }
+}
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<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)? {
-            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<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.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<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> {
-
-    fn read(&mut self, mut buffer: &mut [u8]) -> Result<usize, io::Error> {
-
-        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 <R: Read> BlockRead for EmulateTapeReader<R> {
+    fn read_block(&mut self, buffer: &mut [u8]) -> Result<BlockReadStatus, std::io::Error> {
+        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<bool, std::io::Error>;
 }
 
-/// 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<R: Read>(
-    reader: &mut R,
-    buffer: &mut [u8],
-) -> Result<bool, std::io::Error> {
+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<BlockReadStatus, std::io::Error>;
 }
-- 
2.20.1




  reply	other threads:[~2021-04-07 10:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07 10:22 [pbs-devel] [PATCH 00/11] Userspace tape driver Dietmar Maurer
2021-04-07 10:22 ` Dietmar Maurer [this message]
2021-04-07 10:22 ` [pbs-devel] [PATCH 02/11] tape: introduce trait BlockWrite Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 03/11] tape: implement LTO userspace driver Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 04/11] tape: implement format/erase Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 05/11] tape: fix LEOM handling Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 06/11] tape: make fsf/bsf driver specific Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 07/11] tape: make sure there is a filemark at the end of the tape Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 08/11] sgutils2: add scsi_mode_sense helper Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 09/11] tape: correctly set/display drive option Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 10/11] tape: pmt - re-implement fsr/bsr Dietmar Maurer
2021-04-07 10:23 ` [pbs-devel] [PATCH 11/11] tape: pmt - re-implement lock/unlock command 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=20210407102308.9750-2-dietmar@proxmox.com \
    --to=dietmar@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