Skip to content

Disk operations should be u64 not usize#1116

Open
iximeow wants to merge 1 commit intomasterfrom
ixi/u64-pedantics
Open

Disk operations should be u64 not usize#1116
iximeow wants to merge 1 commit intomasterfrom
ixi/u64-pedantics

Conversation

@iximeow
Copy link
Copy Markdown
Member

@iximeow iximeow commented Apr 16, 2026

Not that we're ever building Propolis on a target where usize is not u64, but I'm going to follow this with a pass that adjusts conversions between u64 and usize and getting the types lined up is kinda helpful.

this is the patch I mentioned here about moving some u64 to usize - I really just moved ByteOffset and ByteLen to u64 and followed the types a bit.

there are a few different kinds of arguments around this:

  • guest interfaces to provide file offsets (NVMe commands, virtio-block commands) use 64-bit types because those specifications tolerate disks larger than 4GiB, so this lines up Propolis with devices a bit better
  • usize as file op sizes means if we did build Propolis for a 32-bit target a bunch of file stuff that was legal becomes broken (abstractly, there's nothing forbidding Propolis from building for say i686 even though we have absolutely no reason to)
  • operations on actual files (and so block/file.rs) will be 64-bit even if we build for 32-bit targets - libc::off_t on illumos is 8 bytes for some reasons I'd really rather not track down. this also applies for BlockIndex from Crucible.

... it happens to line up disk bits and Oximeter reporting better, so that's nice. this doesn't actually change anything and this will go in after #1105. by then I'll have another change or two lined up after this that are more interesting.

Not that we're ever building Propolis on a target where `usize` is not
`u64`, but I'm going to follow this with a pass that adjusts conversions
between u64 and usize and getting the types lined up is kinda helpful.
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.

1 participant