public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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





  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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal