* [pbs-devel] [PATCH 1/2] sgutils2: use thiserror to derive Error
@ 2021-04-12 9:32 Dietmar Maurer
2021-04-12 9:32 ` [pbs-devel] [PATCH 2/2] tape: improve EOT error handling Dietmar Maurer
2021-04-12 10:04 ` [pbs-devel] [PATCH 1/2] sgutils2: use thiserror to derive Error Dominik Csapak
0 siblings, 2 replies; 3+ messages in thread
From: Dietmar Maurer @ 2021-04-12 9:32 UTC (permalink / raw)
To: pbs-devel
---
Cargo.toml | 1 +
src/tools/sgutils2.rs | 42 ++++++++++--------------------------------
2 files changed, 11 insertions(+), 32 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index 1d47423e..8e85675c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,6 +32,7 @@ endian_trait = { version = "0.6", features = ["arrays"] }
env_logger = "0.7"
flate2 = "1.0"
anyhow = "1.0"
+thiserror = "1.0"
futures = "0.3"
h2 = { version = "0.3", features = [ "stream" ] }
handlebars = "3.0"
diff --git a/src/tools/sgutils2.rs b/src/tools/sgutils2.rs
index 3570aca7..df00fbf5 100644
--- a/src/tools/sgutils2.rs
+++ b/src/tools/sgutils2.rs
@@ -17,16 +17,16 @@ use std::ffi::CStr;
use proxmox::tools::io::ReadExt;
-#[derive(Debug)]
+#[derive(thiserror::Error, Debug)]
pub struct SenseInfo {
pub sense_key: u8,
pub asc: u8,
pub ascq: u8,
}
-impl ToString for SenseInfo {
+impl std::fmt::Display for SenseInfo {
- fn to_string(&self) -> String {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let sense_text = SENSE_KEY_DESCRIPTIONS
.get(self.sense_key as usize)
@@ -34,43 +34,21 @@ impl ToString for SenseInfo {
.unwrap_or_else(|| format!("Invalid sense {:02X}", self.sense_key));
if self.asc == 0 && self.ascq == 0 {
- return sense_text;
+ write!(f, "{}", sense_text)?;
}
let additional_sense_text = get_asc_ascq_string(self.asc, self.ascq);
- format!("{}, {}", sense_text, additional_sense_text)
+ write!(f, "{}, {}", sense_text, additional_sense_text)
}
}
-#[derive(Debug)]
+#[derive(thiserror::Error, Debug)]
pub enum ScsiError {
- Error(Error),
- Sense(SenseInfo),
-}
-
-impl std::fmt::Display for ScsiError {
- fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
- match self {
- ScsiError::Error(err) => write!(f, "{}", err),
- ScsiError::Sense(sense) => write!(f, "{}", sense.to_string()),
- }
- }
-}
-
-impl std::error::Error for ScsiError {
- fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
- match self {
- ScsiError::Error(err) => err.source(),
- ScsiError::Sense(_) => None,
- }
- }
-}
-
-impl From<anyhow::Error> for ScsiError {
- fn from(error: anyhow::Error) -> Self {
- Self::Error(error)
- }
+ #[error("{0}")]
+ Error(#[from] Error),
+ #[error("{0}")]
+ Sense(#[from] SenseInfo),
}
impl From<std::io::Error> for ScsiError {
--
2.20.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pbs-devel] [PATCH 2/2] tape: improve EOT error handling
2021-04-12 9:32 [pbs-devel] [PATCH 1/2] sgutils2: use thiserror to derive Error Dietmar Maurer
@ 2021-04-12 9:32 ` Dietmar Maurer
2021-04-12 10:04 ` [pbs-devel] [PATCH 1/2] sgutils2: use thiserror to derive Error Dominik Csapak
1 sibling, 0 replies; 3+ messages in thread
From: Dietmar Maurer @ 2021-04-12 9:32 UTC (permalink / raw)
To: pbs-devel
---
src/api2/tape/drive.rs | 29 ++++-------
src/api2/tape/restore.rs | 27 +++++++---
src/bin/proxmox-tape.rs | 18 +++++--
src/tape/drive/lto/mod.rs | 12 ++---
src/tape/drive/lto/sg_tape.rs | 31 ++++++-----
src/tape/drive/mod.rs | 31 ++++++++---
src/tape/drive/virtual_tape.rs | 22 ++++----
src/tape/file_formats/blocked_reader.rs | 69 ++++++++++++-------------
src/tape/helpers/emulate_tape_reader.rs | 19 ++++---
src/tape/tape_read.rs | 10 ++--
10 files changed, 152 insertions(+), 116 deletions(-)
diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index ca45bb69..c1fd72e1 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -10,7 +10,6 @@ use proxmox::{
identity,
list_subdirs_api_method,
tools::Uuid,
- sys::error::SysError,
api::{
api,
section_config::SectionConfigData,
@@ -60,6 +59,7 @@ use crate::{
Inventory,
MediaCatalog,
MediaId,
+ BlockReadError,
lock_media_set,
lock_media_pool,
lock_unassigned_media_pool,
@@ -528,15 +528,11 @@ pub fn label_media(
drive.rewind()?;
match drive.read_next_file() {
- Ok(Some(_file)) => bail!("media is not empty (format it first)"),
- Ok(None) => { /* EOF mark at BOT, assume tape is empty */ },
+ Ok(_reader) => bail!("media is not empty (format it first)"),
+ Err(BlockReadError::EndOfFile) => { /* EOF mark at BOT, assume tape is empty */ },
+ Err(BlockReadError::EndOfStream) => { /* 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 {
- bail!("media read error - {}", err);
- }
+ bail!("media read error - {}", err);
}
}
@@ -1091,18 +1087,15 @@ fn barcode_label_media_worker(
drive.rewind()?;
match drive.read_next_file() {
- Ok(Some(_file)) => {
+ Ok(_reader) => {
worker.log(format!("media '{}' is not empty (format it first)", label_text));
continue;
}
- Ok(None) => { /* EOF mark at BOT, assume tape is empty */ },
- Err(err) => {
- if err.is_errno(nix::errno::Errno::ENOSPC) || err.is_errno(nix::errno::Errno::EIO) {
- /* assume tape is empty */
- } else {
- worker.warn(format!("media '{}' read error (maybe not empty - format it first)", label_text));
- continue;
- }
+ Err(BlockReadError::EndOfFile) => { /* EOF mark at BOT, assume tape is empty */ },
+ Err(BlockReadError::EndOfStream) => { /* tape is empty */ },
+ Err(_err) => {
+ worker.warn(format!("media '{}' read error (maybe not empty - format it first)", label_text));
+ continue;
}
}
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 78aac73c..afce3449 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -70,6 +70,7 @@ use crate::{
tape::{
TAPE_STATUS_DIR,
TapeRead,
+ BlockReadError,
MediaId,
MediaSet,
MediaCatalog,
@@ -418,12 +419,19 @@ pub fn restore_media(
loop {
let current_file_number = drive.current_file_number()?;
- let reader = match drive.read_next_file()? {
- None => {
+ let reader = match drive.read_next_file() {
+ Err(BlockReadError::EndOfFile) => {
+ task_log!(worker, "skip unexpected filemark at pos {}", current_file_number);
+ continue;
+ }
+ Err(BlockReadError::EndOfStream) => {
task_log!(worker, "detected EOT after {} files", current_file_number);
break;
}
- Some(reader) => reader,
+ Err(BlockReadError::Error(err)) => {
+ return Err(err.into());
+ }
+ Ok(reader) => reader,
};
restore_archive(worker, reader, current_file_number, target, &mut catalog, verbose)?;
@@ -811,12 +819,19 @@ pub fn fast_catalog_restore(
let current_file_number = drive.current_file_number()?;
{ // limit reader scope
- let mut reader = match drive.read_next_file()? {
- None => {
+ let mut reader = match drive.read_next_file() {
+ Err(BlockReadError::EndOfFile) => {
+ task_log!(worker, "skip unexpected filemark at pos {}", current_file_number);
+ continue;
+ }
+ Err(BlockReadError::EndOfStream) => {
task_log!(worker, "detected EOT after {} files", current_file_number);
break;
}
- Some(reader) => reader,
+ Err(BlockReadError::Error(err)) => {
+ return Err(err.into());
+ }
+ Ok(reader) => reader,
};
let header: MediaContentHeader = unsafe { reader.read_le_value()? };
diff --git a/src/bin/proxmox-tape.rs b/src/bin/proxmox-tape.rs
index 1d23b71c..3b3bbafa 100644
--- a/src/bin/proxmox-tape.rs
+++ b/src/bin/proxmox-tape.rs
@@ -43,6 +43,7 @@ use proxmox_backup::{
media_pool::complete_pool_name,
},
tape::{
+ BlockReadError,
drive::{
open_drive,
lock_tape_device,
@@ -587,12 +588,19 @@ fn debug_scan(mut param: Value) -> Result<(), Error> {
loop {
let file_number = drive.current_file_number()?;
- match drive.read_next_file()? {
- None => {
- println!("EOD");
+ match drive.read_next_file() {
+ Err(BlockReadError::EndOfFile) => {
+ println!("filemark");
continue;
- },
- Some(mut reader) => {
+ }
+ Err(BlockReadError::EndOfStream) => {
+ println!("got EOT");
+ return Ok(());
+ }
+ Err(BlockReadError::Error(err)) => {
+ return Err(err.into());
+ }
+ Ok(mut reader) => {
println!("got file number {}", file_number);
let header: Result<MediaContentHeader, _> = unsafe { reader.read_le_value() };
diff --git a/src/tape/drive/lto/mod.rs b/src/tape/drive/lto/mod.rs
index 9dc85ac9..642c3cc7 100644
--- a/src/tape/drive/lto/mod.rs
+++ b/src/tape/drive/lto/mod.rs
@@ -43,6 +43,7 @@ use crate::{
tape::{
TapeRead,
TapeWrite,
+ BlockReadError,
drive::{
TapeDriver,
},
@@ -320,16 +321,9 @@ impl TapeDriver for LtoTapeHandle {
self.sg_tape.format_media(fast)
}
- fn read_next_file<'a>(&'a mut self) -> Result<Option<Box<dyn TapeRead + 'a>>, std::io::Error> {
+ fn read_next_file<'a>(&'a mut self) -> Result<Box<dyn TapeRead + 'a>, BlockReadError> {
let reader = self.sg_tape.open_reader()?;
- let handle = match reader {
- Some(reader) => {
- let reader: Box<dyn TapeRead> = Box::new(reader);
- Some(reader)
- }
- None => None,
- };
-
+ let handle: Box<dyn TapeRead> = Box::new(reader);
Ok(handle)
}
diff --git a/src/tape/drive/lto/sg_tape.rs b/src/tape/drive/lto/sg_tape.rs
index 9f59b321..3f73e025 100644
--- a/src/tape/drive/lto/sg_tape.rs
+++ b/src/tape/drive/lto/sg_tape.rs
@@ -32,7 +32,7 @@ use crate::{
},
tape::{
BlockRead,
- BlockReadStatus,
+ BlockReadError,
BlockWrite,
file_formats::{
BlockedWriter,
@@ -526,11 +526,13 @@ impl SgTape {
}
}
- fn read_block(&mut self, buffer: &mut [u8]) -> Result<BlockReadStatus, std::io::Error> {
+ fn read_block(&mut self, buffer: &mut [u8]) -> Result<usize, BlockReadError> {
let transfer_len = buffer.len();
if transfer_len > 0xFFFFFF {
- proxmox::io_bail!("read failed - buffer too large");
+ return Err(BlockReadError::Error(
+ proxmox::io_format_err!("read failed - buffer too large")
+ ));
}
let mut sg_raw = SgRaw::new(&mut self.file, 0)
@@ -549,21 +551,25 @@ impl SgTape {
let data = match sg_raw.do_in_command(&cmd, buffer) {
Ok(data) => data,
Err(ScsiError::Sense(SenseInfo { sense_key: 0, asc: 0, ascq: 1 })) => {
- return Ok(BlockReadStatus::EndOfFile);
+ return Err(BlockReadError::EndOfFile);
}
Err(ScsiError::Sense(SenseInfo { sense_key: 8, asc: 0, ascq: 5 })) => {
- return Ok(BlockReadStatus::EndOfStream);
+ return Err(BlockReadError::EndOfStream);
}
Err(err) => {
- proxmox::io_bail!("read failed - {}", err);
+ return Err(BlockReadError::Error(
+ proxmox::io_format_err!("read failed - {}", err)
+ ));
}
};
if data.len() != transfer_len {
- proxmox::io_bail!("read failed - unexpected block len ({} != {})", data.len(), buffer.len())
+ return Err(BlockReadError::Error(
+ proxmox::io_format_err!("read failed - unexpected block len ({} != {})", data.len(), buffer.len())
+ ));
}
- Ok(BlockReadStatus::Ok(transfer_len))
+ Ok(transfer_len)
}
pub fn open_writer(&mut self) -> BlockedWriter<SgTapeWriter> {
@@ -571,12 +577,9 @@ impl SgTape {
BlockedWriter::new(writer)
}
- pub fn open_reader(&mut self) -> Result<Option<BlockedReader<SgTapeReader>>, std::io::Error> {
+ pub fn open_reader(&mut self) -> Result<BlockedReader<SgTapeReader>, BlockReadError> {
let reader = SgTapeReader::new(self);
- match BlockedReader::open(reader)? {
- Some(reader) => Ok(Some(reader)),
- None => Ok(None),
- }
+ BlockedReader::open(reader)
}
/// Set important drive options
@@ -702,7 +705,7 @@ impl <'a> SgTapeReader<'a> {
impl <'a> BlockRead for SgTapeReader<'a> {
- fn read_block(&mut self, buffer: &mut [u8]) -> Result<BlockReadStatus, std::io::Error> {
+ fn read_block(&mut self, buffer: &mut [u8]) -> Result<usize, BlockReadError> {
self.sg_tape.read_block(buffer)
}
}
diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
index 0d9e3db3..fd8f503d 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -44,6 +44,7 @@ use crate::{
tape::{
TapeWrite,
TapeRead,
+ BlockReadError,
MediaId,
drive::lto::TapeAlertFlags,
file_formats::{
@@ -86,7 +87,7 @@ pub trait TapeDriver {
fn format_media(&mut self, fast: bool) -> Result<(), Error>;
/// Read/Open the next file
- fn read_next_file<'a>(&'a mut self) -> Result<Option<Box<dyn TapeRead + 'a>>, std::io::Error>;
+ fn read_next_file<'a>(&'a mut self) -> Result<Box<dyn TapeRead + 'a>, BlockReadError>;
/// Write/Append a new file
fn write_file<'a>(&'a mut self) -> Result<Box<dyn TapeWrite + 'a>, std::io::Error>;
@@ -132,9 +133,17 @@ pub trait TapeDriver {
self.rewind()?;
let label = {
- let mut reader = match self.read_next_file()? {
- None => return Ok((None, None)), // tape is empty
- Some(reader) => reader,
+ let mut reader = match self.read_next_file() {
+ Err(BlockReadError::EndOfStream) => {
+ return Ok((None, None)); // tape is empty
+ }
+ Err(BlockReadError::EndOfFile) => {
+ bail!("got unexpected filemark at BOT");
+ }
+ Err(BlockReadError::Error(err)) => {
+ return Err(err.into());
+ }
+ Ok(reader) => reader,
};
let header: MediaContentHeader = unsafe { reader.read_le_value()? };
@@ -155,9 +164,17 @@ pub trait TapeDriver {
let mut media_id = MediaId { label, media_set_label: None };
// try to read MediaSet label
- let mut reader = match self.read_next_file()? {
- None => return Ok((Some(media_id), None)),
- Some(reader) => reader,
+ let mut reader = match self.read_next_file() {
+ Err(BlockReadError::EndOfStream) => {
+ return Ok((Some(media_id), None));
+ }
+ Err(BlockReadError::EndOfFile) => {
+ bail!("got unexpected filemark after label");
+ }
+ Err(BlockReadError::Error(err)) => {
+ return Err(err.into());
+ }
+ Ok(reader) => reader,
};
let header: MediaContentHeader = unsafe { reader.read_le_value()? };
diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs
index a852056a..f7ebc0bb 100644
--- a/src/tape/drive/virtual_tape.rs
+++ b/src/tape/drive/virtual_tape.rs
@@ -15,6 +15,7 @@ use crate::{
tape::{
TapeWrite,
TapeRead,
+ BlockReadError,
changer::{
MediaChange,
MtxStatus,
@@ -261,18 +262,18 @@ impl TapeDriver for VirtualTapeHandle {
}
- fn read_next_file(&mut self) -> Result<Option<Box<dyn TapeRead>>, io::Error> {
+ fn read_next_file(&mut self) -> Result<Box<dyn TapeRead>, BlockReadError> {
let mut status = self.load_status()
- .map_err(|err| io::Error::new(io::ErrorKind::Other, err.to_string()))?;
+ .map_err(|err| BlockReadError::Error(io::Error::new(io::ErrorKind::Other, err.to_string())))?;
match status.current_tape {
Some(VirtualTapeStatus { ref name, ref mut pos }) => {
let index = self.load_tape_index(name)
- .map_err(|err| io::Error::new(io::ErrorKind::Other, err.to_string()))?;
+ .map_err(|err| BlockReadError::Error(io::Error::new(io::ErrorKind::Other, err.to_string())))?;
if *pos >= index.files {
- return Ok(None); // EOM
+ return Err(BlockReadError::EndOfStream);
}
let path = self.tape_file_path(name, *pos);
@@ -282,16 +283,15 @@ impl TapeDriver for VirtualTapeHandle {
*pos += 1;
self.store_status(&status)
- .map_err(|err| io::Error::new(io::ErrorKind::Other, err.to_string()))?;
+ .map_err(|err| BlockReadError::Error(io::Error::new(io::ErrorKind::Other, err.to_string())))?;
let reader = EmulateTapeReader::new(file);
-
- match BlockedReader::open(reader)? {
- Some(reader) => Ok(Some(Box::new(reader))),
- None => Ok(None),
- }
+ let reader = BlockedReader::open(reader)?;
+ Ok(Box::new(reader))
+ }
+ None => {
+ return Err(BlockReadError::Error(proxmox::io_format_err!("drive is empty (no tape loaded).")));
}
- None => proxmox::io_bail!("drive is empty (no tape loaded)."),
}
}
diff --git a/src/tape/file_formats/blocked_reader.rs b/src/tape/file_formats/blocked_reader.rs
index 3df84a1b..936eb24b 100644
--- a/src/tape/file_formats/blocked_reader.rs
+++ b/src/tape/file_formats/blocked_reader.rs
@@ -3,7 +3,7 @@ use std::io::Read;
use crate::tape::{
TapeRead,
BlockRead,
- BlockReadStatus,
+ BlockReadError,
file_formats::{
PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0,
BlockHeader,
@@ -37,15 +37,13 @@ impl <R: BlockRead> BlockedReader<R> {
/// Create a new BlockedReader instance.
///
- /// This tries to read the first block, and returns None if we are
- /// at EOT.
- pub fn open(mut reader: R) -> Result<Option<Self>, std::io::Error> {
+ /// This tries to read the first block. Please inspect the error
+ /// to detect EOF and EOT.
+ pub fn open(mut reader: R) -> Result<Self, BlockReadError> {
let mut buffer = BlockHeader::new();
- if !Self::read_block_frame(&mut buffer, &mut reader)? {
- return Ok(None);
- }
+ Self::read_block_frame(&mut buffer, &mut reader)?;
let (_size, found_end_marker) = Self::check_buffer(&buffer, 0)?;
@@ -58,7 +56,7 @@ impl <R: BlockRead> BlockedReader<R> {
got_eod = true;
}
- Ok(Some(Self {
+ Ok(Self {
reader,
buffer,
found_end_marker,
@@ -67,7 +65,7 @@ impl <R: BlockRead> BlockedReader<R> {
seq_nr: 1,
read_error: false,
read_pos: 0,
- }))
+ })
}
fn check_buffer(buffer: &BlockHeader, seq_nr: u32) -> Result<(usize, bool), std::io::Error> {
@@ -95,7 +93,7 @@ impl <R: BlockRead> BlockedReader<R> {
Ok((size, found_end_marker))
}
- fn read_block_frame(buffer: &mut BlockHeader, reader: &mut R) -> Result<bool, std::io::Error> {
+ fn read_block_frame(buffer: &mut BlockHeader, reader: &mut R) -> Result<(), BlockReadError> {
let data = unsafe {
std::slice::from_raw_parts_mut(
@@ -104,38 +102,28 @@ impl <R: BlockRead> BlockedReader<R> {
)
};
- 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)
- }
+ let bytes = reader.read_block(data)?;
+
+ if bytes != BlockHeader::SIZE {
+ return Err(proxmox::io_format_err!("got wrong block size").into());
}
+
+ Ok(())
}
fn consume_eof_marker(reader: &mut R) -> Result<(), std::io::Error> {
let mut tmp_buf = [0u8; 512]; // use a small buffer for testing EOF
match reader.read_block(&mut tmp_buf) {
- Ok(BlockReadStatus::Ok(_)) => {
+ Ok(_) => {
proxmox::io_bail!("detected tape block after block-stream end marker");
}
- Ok(BlockReadStatus::EndOfFile) => {
+ Err(BlockReadError::EndOfFile) => {
return Ok(());
}
- Ok(BlockReadStatus::EndOfStream) => {
+ Err(BlockReadError::EndOfStream) => {
proxmox::io_bail!("got unexpected end of tape");
}
- Err(err) => {
+ Err(BlockReadError::Error(err)) => {
return Err(err);
}
}
@@ -143,13 +131,22 @@ impl <R: BlockRead> BlockedReader<R> {
fn read_block(&mut self) -> Result<usize, std::io::Error> {
- if !Self::read_block_frame(&mut self.buffer, &mut self.reader)? {
- self.got_eod = true;
- self.read_pos = self.buffer.payload.len();
- if !self.found_end_marker {
- proxmox::io_bail!("detected tape stream without end marker");
+ match Self::read_block_frame(&mut self.buffer, &mut self.reader) {
+ Ok(()) => { /* ok */ }
+ Err(BlockReadError::EndOfFile) => {
+ self.got_eod = true;
+ self.read_pos = self.buffer.payload.len();
+ if !self.found_end_marker {
+ proxmox::io_bail!("detected tape stream without end marker");
+ }
+ return Ok(0); // EOD
+ }
+ Err(BlockReadError::EndOfStream) => {
+ proxmox::io_bail!("got unexpected end of tape");
+ }
+ Err(BlockReadError::Error(err)) => {
+ return Err(err);
}
- return Ok(0); // EOD
}
let (size, found_end_marker) = Self::check_buffer(&self.buffer, self.seq_nr)?;
diff --git a/src/tape/helpers/emulate_tape_reader.rs b/src/tape/helpers/emulate_tape_reader.rs
index fbb19028..92c975f9 100644
--- a/src/tape/helpers/emulate_tape_reader.rs
+++ b/src/tape/helpers/emulate_tape_reader.rs
@@ -4,7 +4,7 @@ use proxmox::tools::io::ReadExt;
use crate::tape::{
BlockRead,
- BlockReadStatus,
+ BlockReadError,
file_formats::PROXMOX_TAPE_BLOCK_SIZE,
};
@@ -24,22 +24,27 @@ impl <R: Read> EmulateTapeReader<R> {
}
impl <R: Read> BlockRead for EmulateTapeReader<R> {
- fn read_block(&mut self, buffer: &mut [u8]) -> Result<BlockReadStatus, std::io::Error> {
+ fn read_block(&mut self, buffer: &mut [u8]) -> Result<usize, BlockReadError> {
if self.got_eof {
- proxmox::io_bail!("detected read after EOF!");
+ return Err(BlockReadError::Error(proxmox::io_format_err!("detected read after EOF!")));
}
match self.reader.read_exact_or_eof(buffer)? {
false => {
self.got_eof = true;
- Ok(BlockReadStatus::EndOfFile)
+ Err(BlockReadError::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);
+ return Err(BlockReadError::Error(
+ proxmox::io_format_err!(
+ "EmulateTapeReader: read_block with wrong block size ({} != {})",
+ buffer.len(),
+ PROXMOX_TAPE_BLOCK_SIZE,
+ )
+ ));
}
- Ok(BlockReadStatus::Ok(buffer.len()))
+ Ok(buffer.len())
}
}
}
diff --git a/src/tape/tape_read.rs b/src/tape/tape_read.rs
index 7cbe2b8b..c617ec61 100644
--- a/src/tape/tape_read.rs
+++ b/src/tape/tape_read.rs
@@ -15,14 +15,18 @@ pub trait TapeRead: Read {
fn has_end_marker(&self) -> Result<bool, std::io::Error>;
}
-pub enum BlockReadStatus {
- Ok(usize),
+#[derive(thiserror::Error, Debug)]
+pub enum BlockReadError {
+ #[error("{0}")]
+ Error(#[from] std::io::Error),
+ #[error("end of file")]
EndOfFile,
+ #[error("end of data stream")]
EndOfStream,
}
/// 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>;
+ fn read_block(&mut self, buffer: &mut [u8]) -> Result<usize, BlockReadError>;
}
--
2.20.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pbs-devel] [PATCH 1/2] sgutils2: use thiserror to derive Error
2021-04-12 9:32 [pbs-devel] [PATCH 1/2] sgutils2: use thiserror to derive Error Dietmar Maurer
2021-04-12 9:32 ` [pbs-devel] [PATCH 2/2] tape: improve EOT error handling Dietmar Maurer
@ 2021-04-12 10:04 ` Dominik Csapak
1 sibling, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2021-04-12 10:04 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dietmar Maurer
comment inline:
On 4/12/21 11:32, Dietmar Maurer wrote:
> ---
> Cargo.toml | 1 +
> src/tools/sgutils2.rs | 42 ++++++++++--------------------------------
> 2 files changed, 11 insertions(+), 32 deletions(-)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index 1d47423e..8e85675c 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -32,6 +32,7 @@ endian_trait = { version = "0.6", features = ["arrays"] }
> env_logger = "0.7"
> flate2 = "1.0"
> anyhow = "1.0"
> +thiserror = "1.0"
> futures = "0.3"
> h2 = { version = "0.3", features = [ "stream" ] }
> handlebars = "3.0"
> diff --git a/src/tools/sgutils2.rs b/src/tools/sgutils2.rs
> index 3570aca7..df00fbf5 100644
> --- a/src/tools/sgutils2.rs
> +++ b/src/tools/sgutils2.rs
> @@ -17,16 +17,16 @@ use std::ffi::CStr;
>
> use proxmox::tools::io::ReadExt;
>
> -#[derive(Debug)]
> +#[derive(thiserror::Error, Debug)]
> pub struct SenseInfo {
> pub sense_key: u8,
> pub asc: u8,
> pub ascq: u8,
> }
>
> -impl ToString for SenseInfo {
> +impl std::fmt::Display for SenseInfo {
>
> - fn to_string(&self) -> String {
> + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
>
> let sense_text = SENSE_KEY_DESCRIPTIONS
> .get(self.sense_key as usize)
> @@ -34,43 +34,21 @@ impl ToString for SenseInfo {
> .unwrap_or_else(|| format!("Invalid sense {:02X}", self.sense_key));
>
> if self.asc == 0 && self.ascq == 0 {
> - return sense_text;
> + write!(f, "{}", sense_text)?;
here a return is missing, else we print sense_text twice and always
get the additional_sense_text
> }
>
> let additional_sense_text = get_asc_ascq_string(self.asc, self.ascq);
>
> - format!("{}, {}", sense_text, additional_sense_text)
> + write!(f, "{}, {}", sense_text, additional_sense_text)
> }
> }
>
> -#[derive(Debug)]
> +#[derive(thiserror::Error, Debug)]
> pub enum ScsiError {
> - Error(Error),
> - Sense(SenseInfo),
> -}
> -
> -impl std::fmt::Display for ScsiError {
> - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> - match self {
> - ScsiError::Error(err) => write!(f, "{}", err),
> - ScsiError::Sense(sense) => write!(f, "{}", sense.to_string()),
> - }
> - }
> -}
> -
> -impl std::error::Error for ScsiError {
> - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
> - match self {
> - ScsiError::Error(err) => err.source(),
> - ScsiError::Sense(_) => None,
> - }
> - }
> -}
> -
> -impl From<anyhow::Error> for ScsiError {
> - fn from(error: anyhow::Error) -> Self {
> - Self::Error(error)
> - }
> + #[error("{0}")]
> + Error(#[from] Error),
> + #[error("{0}")]
> + Sense(#[from] SenseInfo),
> }
>
> impl From<std::io::Error> for ScsiError {
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-12 10:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 9:32 [pbs-devel] [PATCH 1/2] sgutils2: use thiserror to derive Error Dietmar Maurer
2021-04-12 9:32 ` [pbs-devel] [PATCH 2/2] tape: improve EOT error handling Dietmar Maurer
2021-04-12 10:04 ` [pbs-devel] [PATCH 1/2] sgutils2: use thiserror to derive Error Dominik Csapak
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