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 3F7657115F for ; Wed, 7 Apr 2021 12:24:14 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3D7BDF3BD for ; Wed, 7 Apr 2021 12:24:14 +0200 (CEST) Received: from elsa.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP id 17CB4F337 for ; Wed, 7 Apr 2021 12:24:07 +0200 (CEST) Received: by elsa.proxmox.com (Postfix, from userid 0) id 05010AEAF20; Wed, 7 Apr 2021 12:24:07 +0200 (CEST) From: Dietmar Maurer To: pbs-devel@lists.proxmox.com Date: Wed, 7 Apr 2021 12:22:59 +0200 Message-Id: <20210407102308.9750-3-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 Subject: [pbs-devel] [PATCH 02/11] tape: introduce trait BlockWrite 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:14 -0000 --- src/tape/drive/linux_tape.rs | 126 +++++++++++++++--------- src/tape/drive/virtual_tape.rs | 3 +- src/tape/file_formats/blocked_writer.rs | 37 +++++-- src/tape/helpers/emulate_tape_writer.rs | 37 +++---- src/tape/tape_write.rs | 61 ++---------- 5 files changed, 141 insertions(+), 123 deletions(-) diff --git a/src/tape/drive/linux_tape.rs b/src/tape/drive/linux_tape.rs index 9e86d0a3..d556f0f5 100644 --- a/src/tape/drive/linux_tape.rs +++ b/src/tape/drive/linux_tape.rs @@ -1,7 +1,7 @@ //! Driver for Linux SCSI tapes use std::fs::{OpenOptions, File}; -use std::io::Read; +use std::io::{Read, Write}; use std::os::unix::fs::OpenOptionsExt; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::convert::TryFrom; @@ -9,8 +9,10 @@ use std::convert::TryFrom; use anyhow::{bail, format_err, Error}; use nix::fcntl::{fcntl, FcntlArg, OFlag}; -use proxmox::sys::error::SysResult; -use proxmox::tools::Uuid; +use proxmox::{ + tools::Uuid, + sys::error::{SysError, SysResult}, +}; use crate::{ config, @@ -25,6 +27,7 @@ use crate::{ LinuxDriveAndMediaStatus, }, tape::{ + BlockWrite, BlockRead, BlockReadStatus, TapeRead, @@ -257,10 +260,9 @@ impl LinuxTapeHandle { Ok(()) } - /// Write a single EOF mark - pub fn write_eof_mark(&self) -> Result<(), Error> { - tape_write_eof_mark(&self.file)?; - Ok(()) + /// Write a single EOF mark without flushing buffers + pub fn write_eof_mark(&mut self) -> Result<(), std::io::Error> { + tape_write_eof_mark(&mut self.file) } /// Set the drive's block length to the value specified. @@ -519,10 +521,8 @@ impl TapeDriver for LinuxTapeHandle { fn write_file<'a>(&'a mut self) -> Result, std::io::Error> { - let handle = TapeWriterHandle { - writer: BlockedWriter::new(&mut self.file), - }; - + let writer = LinuxTapeWriter::new(&mut self.file); + let handle = BlockedWriter::new(writer); Ok(Box::new(handle)) } @@ -545,27 +545,27 @@ impl TapeDriver for LinuxTapeHandle { self.set_encryption(None)?; - let mut handle = TapeWriterHandle { - writer: BlockedWriter::new(&mut self.file), - }; + { // limit handle scope + let mut handle = self.write_file()?; - let mut value = serde_json::to_value(media_set_label)?; - if media_set_label.encryption_key_fingerprint.is_some() { - match key_config { - Some(key_config) => { - value["key-config"] = serde_json::to_value(key_config)?; - } - None => { - bail!("missing encryption key config"); + let mut value = serde_json::to_value(media_set_label)?; + if media_set_label.encryption_key_fingerprint.is_some() { + match key_config { + Some(key_config) => { + value["key-config"] = serde_json::to_value(key_config)?; + } + None => { + bail!("missing encryption key config"); + } } } - } - let raw = serde_json::to_string_pretty(&value)?; + let raw = serde_json::to_string_pretty(&value)?; - let header = MediaContentHeader::new(PROXMOX_BACKUP_MEDIA_SET_LABEL_MAGIC_1_0, raw.len() as u32); - handle.write_header(&header, raw.as_bytes())?; - handle.finish(false)?; + let header = MediaContentHeader::new(PROXMOX_BACKUP_MEDIA_SET_LABEL_MAGIC_1_0, raw.len() as u32); + handle.write_header(&header, raw.as_bytes())?; + handle.finish(false)?; + } self.sync()?; // sync data to tape @@ -655,7 +655,7 @@ impl TapeDriver for LinuxTapeHandle { } /// Write a single EOF mark without flushing buffers -fn tape_write_eof_mark(file: &File) -> Result<(), std::io::Error> { +fn tape_write_eof_mark(file: &mut File) -> Result<(), std::io::Error> { let cmd = mtop { mt_op: MTCmd::MTWEOFI, mt_count: 1 }; @@ -745,29 +745,67 @@ pub fn read_tapedev_options(file: &File) -> Result { } -/// like BlockedWriter, but writes EOF mark on finish -pub struct TapeWriterHandle<'a> { - writer: BlockedWriter<&'a mut File>, +struct LinuxTapeWriter<'a> { + /// Assumes that 'file' is a linux tape device. + file: &'a mut File, } -impl TapeWrite for TapeWriterHandle<'_> { - - fn write_all(&mut self, data: &[u8]) -> Result { - self.writer.write_all(data) +impl <'a> LinuxTapeWriter<'a> { + pub fn new(file: &'a mut File) -> Self { + Self { file } } +} - fn bytes_written(&self) -> usize { - self.writer.bytes_written() - } +impl <'a> BlockWrite for LinuxTapeWriter<'a> { - fn finish(&mut self, incomplete: bool) -> Result { - let leof = self.writer.finish(incomplete)?; - tape_write_eof_mark(self.writer.writer_ref_mut())?; - Ok(leof) + /// Write a single block to a linux tape device + /// + /// EOM Behaviour on Linux: When the end of medium early warning is + /// encountered, the current write is finished and the number of bytes + /// is returned. The next write returns -1 and errno is set to + /// ENOSPC. To enable writing a trailer, the next write is allowed to + /// proceed and, if successful, the number of bytes is returned. After + /// this, -1 and the number of bytes are alternately returned until + /// the physical end of medium (or some other error) is encountered. + /// + /// See: https://github.com/torvalds/linux/blob/master/Documentation/scsi/st.rst + /// + /// On success, this returns if we en countered a EOM condition. + fn write_block(&mut self, data: &[u8]) -> Result { + + let mut leof = false; + + loop { + match self.file.write(data) { + Ok(count) if count == data.len() => return Ok(leof), + Ok(count) if count > 0 => { + proxmox::io_bail!( + "short block write ({} < {}). Tape drive uses wrong block size.", + count, data.len()); + } + Ok(_) => { // count is 0 here, assume EOT + return Err(std::io::Error::from_raw_os_error(nix::errno::Errno::ENOSPC as i32)); + } + // handle interrupted system call + Err(err) if err.kind() == std::io::ErrorKind::Interrupted => { + continue; + } + // detect and handle LEOM (early warning) + Err(err) if err.is_errno(nix::errno::Errno::ENOSPC) => { + if leof { + return Err(err); + } else { + leof = true; + continue; // next write will succeed + } + } + Err(err) => return Err(err), + } + } } - fn logical_end_of_media(&self) -> bool { - self.writer.logical_end_of_media() + fn write_filemark(&mut self) -> Result<(), std::io::Error> { + tape_write_eof_mark(&mut self.file) } } diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs index 2fc9fdea..54e0887f 100644 --- a/src/tape/drive/virtual_tape.rs +++ b/src/tape/drive/virtual_tape.rs @@ -276,8 +276,7 @@ impl TapeDriver for VirtualTapeHandle { free_space = self.max_size - used_space; } - let writer = Box::new(file); - let writer = Box::new(EmulateTapeWriter::new(writer, free_space)); + let writer = EmulateTapeWriter::new(file, free_space); let writer = Box::new(BlockedWriter::new(writer)); Ok(writer) diff --git a/src/tape/file_formats/blocked_writer.rs b/src/tape/file_formats/blocked_writer.rs index 961b2ed2..33fa2955 100644 --- a/src/tape/file_formats/blocked_writer.rs +++ b/src/tape/file_formats/blocked_writer.rs @@ -1,10 +1,8 @@ -use std::io::Write; - use proxmox::tools::vec; use crate::tape::{ TapeWrite, - tape_device_write_block, + BlockWrite, file_formats::{ BlockHeader, BlockHeaderFlags, @@ -16,16 +14,27 @@ use crate::tape::{ /// This type implement 'TapeWrite'. Data written is assembled to /// equally sized blocks (see 'BlockHeader'), which are then written /// to the underlying writer. -pub struct BlockedWriter { +pub struct BlockedWriter { writer: W, buffer: Box, buffer_pos: usize, seq_nr: u32, logical_end_of_media: bool, bytes_written: usize, + wrote_eof: bool, +} + +impl Drop for BlockedWriter { + + // Try to make sure to end the file with a filemark + fn drop(&mut self) { + if !self.wrote_eof { + let _ = self.writer.write_filemark(); + } + } } -impl BlockedWriter { +impl BlockedWriter { /// Allow access to underlying writer pub fn writer_ref_mut(&mut self) -> &mut W { @@ -41,6 +50,7 @@ impl BlockedWriter { seq_nr: 0, logical_end_of_media: false, bytes_written: 0, + wrote_eof: false, } } @@ -52,7 +62,16 @@ impl BlockedWriter { BlockHeader::SIZE, ) }; - tape_device_write_block(writer, data) + writer.write_block(data) + } + + fn write_eof(&mut self) -> Result<(), std::io::Error> { + if self.wrote_eof { + proxmox::io_bail!("BlockedWriter: detected multiple EOF writes"); + } + self.wrote_eof = true; + + self.writer.write_filemark() } fn write(&mut self, data: &[u8]) -> Result { @@ -85,7 +104,7 @@ impl BlockedWriter { } -impl TapeWrite for BlockedWriter { +impl TapeWrite for BlockedWriter { fn write_all(&mut self, mut data: &[u8]) -> Result { while !data.is_empty() { @@ -113,7 +132,9 @@ impl TapeWrite for BlockedWriter { self.buffer.set_seq_nr(self.seq_nr); self.seq_nr += 1; self.bytes_written += BlockHeader::SIZE; - Self::write_block(&self.buffer, &mut self.writer) + let leom = Self::write_block(&self.buffer, &mut self.writer)?; + self.write_eof()?; + Ok(leom) } /// Returns if the writer already detected the logical end of media diff --git a/src/tape/helpers/emulate_tape_writer.rs b/src/tape/helpers/emulate_tape_writer.rs index b385d6b3..eb4f29d7 100644 --- a/src/tape/helpers/emulate_tape_writer.rs +++ b/src/tape/helpers/emulate_tape_writer.rs @@ -1,6 +1,9 @@ use std::io::{self, Write}; -use crate::tape::file_formats::PROXMOX_TAPE_BLOCK_SIZE; +use crate::tape::{ + BlockWrite, + file_formats::PROXMOX_TAPE_BLOCK_SIZE, +}; /// Emulate tape write behavior on a normal Writer /// @@ -11,7 +14,7 @@ pub struct EmulateTapeWriter { block_nr: usize, max_blocks: usize, writer: W, - leom_sent: bool, + wrote_eof: bool, } impl EmulateTapeWriter { @@ -27,16 +30,16 @@ impl EmulateTapeWriter { Self { block_nr: 0, - leom_sent: false, + wrote_eof: false, writer, max_blocks, } } } -impl Write for EmulateTapeWriter { +impl BlockWrite for EmulateTapeWriter { - fn write(&mut self, buffer: &[u8]) -> Result { + fn write_block(&mut self, buffer: &[u8]) -> Result { if buffer.len() != PROXMOX_TAPE_BLOCK_SIZE { proxmox::io_bail!("EmulateTapeWriter: got write with wrong block size ({} != {}", @@ -47,22 +50,22 @@ impl Write for EmulateTapeWriter { return Err(io::Error::from_raw_os_error(nix::errno::Errno::ENOSPC as i32)); } - if self.block_nr >= self.max_blocks { - if !self.leom_sent { - self.leom_sent = true; - return Err(io::Error::from_raw_os_error(nix::errno::Errno::ENOSPC as i32)); - } else { - self.leom_sent = false; - } - } - self.writer.write_all(buffer)?; self.block_nr += 1; - Ok(buffer.len()) + if self.block_nr > self.max_blocks { + Ok(true) + } else { + Ok(false) + } } - fn flush(&mut self) -> Result<(), io::Error> { - proxmox::io_bail!("EmulateTapeWriter does not support flush"); + fn write_filemark(&mut self) -> Result<(), std::io::Error> { + if self.wrote_eof { + proxmox::io_bail!("EmulateTapeWriter: detected multiple EOF writes"); + } + // do nothing, just record the call + self.wrote_eof = true; + Ok(()) } } diff --git a/src/tape/tape_write.rs b/src/tape/tape_write.rs index 8a3d4fd6..593d1a29 100644 --- a/src/tape/tape_write.rs +++ b/src/tape/tape_write.rs @@ -1,9 +1,5 @@ -use std::io::Write; - use endian_trait::Endian; -use proxmox::sys::error::SysError; - use crate::tape::file_formats::MediaContentHeader; /// Write trait for tape devices @@ -53,53 +49,14 @@ pub trait TapeWrite { } } -/// Write a single block to a tape device -/// -/// Assumes that 'writer' is a linux tape device. -/// -/// EOM Behaviour on Linux: When the end of medium early warning is -/// encountered, the current write is finished and the number of bytes -/// is returned. The next write returns -1 and errno is set to -/// ENOSPC. To enable writing a trailer, the next write is allowed to -/// proceed and, if successful, the number of bytes is returned. After -/// this, -1 and the number of bytes are alternately returned until -/// the physical end of medium (or some other error) is encountered. -/// -/// See: https://github.com/torvalds/linux/blob/master/Documentation/scsi/st.rst -/// -/// On success, this returns if we en countered a EOM condition. -pub fn tape_device_write_block( - writer: &mut W, - data: &[u8], -) -> Result { - - let mut leof = false; +/// Write streams of blocks +pub trait BlockWrite { + /// Write a data block + /// + /// Returns true if the drive reached the Logical End Of Media + /// (early warning) + fn write_block(&mut self, buffer: &[u8]) -> Result; - loop { - match writer.write(data) { - Ok(count) if count == data.len() => return Ok(leof), - Ok(count) if count > 0 => { - proxmox::io_bail!( - "short block write ({} < {}). Tape drive uses wrong block size.", - count, data.len()); - } - Ok(_) => { // count is 0 here, assume EOT - return Err(std::io::Error::from_raw_os_error(nix::errno::Errno::ENOSPC as i32)); - } - // handle interrupted system call - Err(err) if err.kind() == std::io::ErrorKind::Interrupted => { - continue; - } - // detect and handle LEOM (early warning) - Err(err) if err.is_errno(nix::errno::Errno::ENOSPC) => { - if leof { - return Err(err); - } else { - leof = true; - continue; // next write will succeed - } - } - Err(err) => return Err(err), - } - } + /// Write a filemark + fn write_filemark(&mut self) -> Result<(), std::io::Error>; } -- 2.20.1