Skip to content

Block size buffers#1105

Merged
mkeeter merged 3 commits intooxidecomputer:mainfrom
mkeeter:block-size-buffers
Jan 24, 2024
Merged

Block size buffers#1105
mkeeter merged 3 commits intooxidecomputer:mainfrom
mkeeter:block-size-buffers

Conversation

@mkeeter
Copy link
Contributor

@mkeeter mkeeter commented Jan 23, 2024

This PR changes the struct Buffer to enforce that it is always an exact multiple of blocks.

The property is enforced at construction: the only way to build or resize a Buffer is to provide a tuple of (block_count, block_size). Various implementors of BlockIO also now check that their reads and writes are block-aligned, returning the existing CrucibleError::DataLenUnaligned if that's not the case.

These changes are a step towards per-block ownership, but make sense as a standalone PR:

  • This PR touches a bunch of lines, but is mostly mechanical changes
  • The upcoming per-block ownership PR will touch fewer lines, but will be making trickier logical changes

Copy link
Contributor

@jmpesp jmpesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

}
if Self::MAX_CHUNK_SIZE % block_size as usize != 0 {
crucible_bail!(
InvalidNumberOfBlocks,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe should be

    #[error("Data length not block size multiple")]
    DataLenUnaligned,

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added static assertions relating MAX_CHUNK_SIZE and MAX_BLOCK_SIZE, so this should always be true and I switched it to an assertion (4c2c657)

@mkeeter mkeeter merged commit 627f3c9 into oxidecomputer:main Jan 24, 2024
@mkeeter mkeeter deleted the block-size-buffers branch January 24, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants