From: "Max R. Carrara" <m.carrara@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [PATCH proxmox-backup v2 06/10] tape: tape block: represent tape block header with its own struct
Date: Fri, 21 Aug 2026 16:02:30 +0200 [thread overview]
Message-ID: <20260821140238.615302-7-m.carrara@proxmox.com> (raw)
In-Reply-To: <20260821140238.615302-1-m.carrara@proxmox.com>
Instead of defining the tape block header's fields inline, move them
into a new struct called `TapeBlockHeader` and adapt existing methods
correspondingly. Introduce methods for any fields that lacked them in
the first place so that they can be accessed again.
Add a static assertion that ensures that `TapeBlockHeader` always has
a size of 16, since its size is not allowed to change.
Finally, adapt sites that used direct field access for any header
fields to use each field's respective method.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
pbs-tape/src/blocked_reader.rs | 8 ++--
pbs-tape/src/blocked_writer.rs | 7 ++--
pbs-tape/src/tape_block.rs | 76 ++++++++++++++++++++++++----------
3 files changed, 62 insertions(+), 29 deletions(-)
diff --git a/pbs-tape/src/blocked_reader.rs b/pbs-tape/src/blocked_reader.rs
index ef3f8d217..cf8150ba3 100644
--- a/pbs-tape/src/blocked_reader.rs
+++ b/pbs-tape/src/blocked_reader.rs
@@ -43,7 +43,7 @@ impl<R: BlockRead> BlockedReader<R> {
let mut got_eod = false;
if found_end_marker {
- incomplete = tape_block.flags.contains(TapeBlockFlags::INCOMPLETE);
+ incomplete = tape_block.flags().contains(TapeBlockFlags::INCOMPLETE);
Self::consume_eof_marker(&mut reader)?;
got_eod = true;
}
@@ -64,7 +64,7 @@ impl<R: BlockRead> BlockedReader<R> {
tape_block: &TapeBlock,
seq_nr: u32,
) -> Result<(usize, bool), std::io::Error> {
- if tape_block.magic != PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0 {
+ if tape_block.magic() != PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0 {
proxmox_lang::io_bail!(
"got tape block with unknown magic number - not written by PBS or incompatible LTO version"
);
@@ -79,7 +79,7 @@ impl<R: BlockRead> BlockedReader<R> {
}
let size = tape_block.size();
- let found_end_marker = tape_block.flags.contains(TapeBlockFlags::END_OF_STREAM);
+ let found_end_marker = tape_block.flags().contains(TapeBlockFlags::END_OF_STREAM);
if size > tape_block.payload.len() {
proxmox_lang::io_bail!(
@@ -150,7 +150,7 @@ impl<R: BlockRead> BlockedReader<R> {
if found_end_marker {
// consume EOF mark
self.found_end_marker = true;
- self.incomplete = self.tape_block.flags.contains(TapeBlockFlags::INCOMPLETE);
+ self.incomplete = self.tape_block.flags().contains(TapeBlockFlags::INCOMPLETE);
Self::consume_eof_marker(&mut self.reader)?;
self.got_eod = true;
}
diff --git a/pbs-tape/src/blocked_writer.rs b/pbs-tape/src/blocked_writer.rs
index 9aab0cbb0..5e94f54b8 100644
--- a/pbs-tape/src/blocked_writer.rs
+++ b/pbs-tape/src/blocked_writer.rs
@@ -77,7 +77,7 @@ impl<W: BlockWrite> BlockedWriter<W> {
let rest = rest - bytes;
if rest == 0 {
- self.tape_block.flags = TapeBlockFlags::empty();
+ self.tape_block.set_flags(TapeBlockFlags::empty());
self.tape_block.set_size(self.tape_block.payload.len());
self.tape_block.set_seq_nr(self.seq_nr);
self.seq_nr += 1;
@@ -116,10 +116,11 @@ impl<W: BlockWrite> TapeWrite for BlockedWriter<W> {
/// END_OF_STREAM flag.
fn finish(&mut self, incomplete: bool) -> Result<bool, std::io::Error> {
vec::clear(&mut self.tape_block.payload[self.buffer_pos..]);
- self.tape_block.flags = TapeBlockFlags::END_OF_STREAM;
+ let mut flags = TapeBlockFlags::END_OF_STREAM;
if incomplete {
- self.tape_block.flags |= TapeBlockFlags::INCOMPLETE;
+ flags |= TapeBlockFlags::INCOMPLETE;
}
+ self.tape_block.set_flags(flags);
self.tape_block.set_size(self.buffer_pos);
self.tape_block.set_seq_nr(self.seq_nr);
self.seq_nr += 1;
diff --git a/pbs-tape/src/tape_block.rs b/pbs-tape/src/tape_block.rs
index 4ab1f5ce9..2db5ed137 100644
--- a/pbs-tape/src/tape_block.rs
+++ b/pbs-tape/src/tape_block.rs
@@ -1,8 +1,24 @@
use bitflags::bitflags;
+use proxmox_lang::static_assert_size;
+
use crate::PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0;
use crate::PROXMOX_TAPE_BLOCK_SIZE;
+#[repr(C, packed)]
+struct TapeBlockHeader {
+ /// Fixed value: `PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0`
+ magic: [u8; 8],
+ /// See [`TapeBlockFlags`].
+ flags: TapeBlockFlags,
+ /// Size as 3 bytes unsigned, little endian.
+ size: [u8; 3],
+ /// Block sequence number.
+ seq_nr: u32,
+}
+
+static_assert_size!(TapeBlockHeader, 16);
+
/// A [`TapeBlock`] consists of a tape header followed by a data payload.
///
/// All tape files are written as sequence of blocks.
@@ -16,13 +32,7 @@ use crate::PROXMOX_TAPE_BLOCK_SIZE;
/// sequence number (`seq_nr`) which may be used for additional error checking.
#[repr(C, packed)]
pub struct TapeBlock {
- /// fixed value `PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0`
- pub magic: [u8; 8],
- pub flags: TapeBlockFlags,
- /// size as 3 bytes unsigned, little endian
- pub size: [u8; 3],
- /// block sequence number
- pub seq_nr: u32,
+ header: TapeBlockHeader,
pub payload: [u8],
}
@@ -51,28 +61,50 @@ impl TapeBlock {
let ptr = alloc_zeroed(Layout::from_size_align(Self::SIZE, page_size).unwrap());
Box::from_raw(core::ptr::slice_from_raw_parts_mut(ptr, Self::SIZE - 16) as *mut Self)
};
- buffer.magic = PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0;
+ buffer.header.magic = PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0;
buffer
}
- /// Set the `size` field
+ /// Returns the magic value of this tape block's header.
+ pub fn magic(&self) -> [u8; 8] {
+ self.header.magic
+ }
+
+ /// Returns the [`TapeBlockFlags`] currently set.
+ pub fn flags(&self) -> TapeBlockFlags {
+ self.header.flags
+ }
+
+ /// Sets new [`TapeBlockFlags`].
+ pub fn set_flags(&mut self, flags: TapeBlockFlags) {
+ self.header.flags = flags;
+ }
+
+ /// Returns the size of the tape block's data.
+ ///
+ /// Note that this value is at most `2^24 - 1`, since the size is
+ /// represented using 24 bits under the hood.
+ pub fn size(&self) -> usize {
+ let raw_size = self.header.size;
+ (raw_size[0] as usize) + ((raw_size[1] as usize) << 8) + ((raw_size[2] as usize) << 16)
+ }
+
+ /// Sets the size of the tape block's data.
+ ///
+ /// Note that the passed value will be truncated to 24 bits, since the size
+ /// is represented using 24 bits under the hood.
pub fn set_size(&mut self, size: usize) {
let size = size.to_le_bytes();
- self.size.copy_from_slice(&size[..3]);
+ self.header.size.copy_from_slice(&size[..3]);
}
- /// Returns the `size` field
- pub fn size(&self) -> usize {
- (self.size[0] as usize) + ((self.size[1] as usize) << 8) + ((self.size[2] as usize) << 16)
- }
-
- /// Set the `seq_nr` field
- pub fn set_seq_nr(&mut self, seq_nr: u32) {
- self.seq_nr = seq_nr.to_le();
- }
-
- /// Returns the `seq_nr` field
+ /// Returns the sequence number of the tape block.
pub fn seq_nr(&self) -> u32 {
- u32::from_le(self.seq_nr)
+ u32::from_le(self.header.seq_nr)
+ }
+
+ /// Sets the tape block's sequence number.
+ pub fn set_seq_nr(&mut self, seq_nr: u32) {
+ self.header.seq_nr = seq_nr.to_le();
}
}
--
2.47.3
next prev parent reply other threads:[~2026-08-21 14:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-21 14:02 [PATCH proxmox{,-backup} v2 00/10] Fix Undefined Behavior in Tape Block Header Deallocation Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox v2 01/10] proxmox-alloc: introduce proxmox-alloc with `LayoutAwareBox<T>` type Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox v2 02/10] proxmox-alloc: document undefined behavior regarding custom allocs Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 03/10] tape: move tape block structs into separate file module Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 04/10] tape: rename `BlockHeader` and `BlockHeaderFlags` Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 05/10] tape: blocked_{reader,writer}: rename `buffer` to `tape_block` Max R. Carrara
2026-08-21 14:02 ` Max R. Carrara [this message]
2026-08-21 14:02 ` [PATCH proxmox-backup v2 07/10] tape: tape block: make `payload` field private Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 08/10] tape: blocked_{reader,writer}: remove haphazard `unsafe` blocks Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 09/10] tape: tape block: fix undefined behavior on tape block deallocation Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 10/10] tape: sgutils2: fix undefined behavior in dealloc of buffer Max R. Carrara
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=20260821140238.615302-7-m.carrara@proxmox.com \
--to=m.carrara@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox