all lists on 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 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