Skip to content

Support partial last EXT4 block group#717

Open
zeel2104 wants to merge 1 commit into
apple:mainfrom
zeel2104:fix-ext4-partial-last-block-group
Open

Support partial last EXT4 block group#717
zeel2104 wants to merge 1 commit into
apple:mainfrom
zeel2104:fix-ext4-partial-last-block-group

Conversation

@zeel2104

@zeel2104 zeel2104 commented May 1, 2026

Copy link
Copy Markdown

Summary

Fixes #647 by supporting a partial final EXT4 block group.

The formatter previously rounded filesystem sizes up to the next block group boundary, so a requested 160 MiB filesystem became 256 MiB. This change keeps the requested size, reports the actual filesystem block count, and marks blocks outside a short final group as used in the bitmap without counting them as filesystem blocks.

Testing

  • swift test --filter ContainerizationEXT4Tests
@dkovba dkovba requested a review from wlan0 May 6, 2026 22:48
@zeel2104 zeel2104 force-pushed the fix-ext4-partial-last-block-group branch from dd22578 to 5c0fe36 Compare May 6, 2026 23:53
@zeel2104

zeel2104 commented May 8, 2026

Copy link
Copy Markdown
Author

@wlan0
Hi, I resolved the issue which was occurring in CI because of not signing the commit
It is ready to be reviewed again

@wlan0

wlan0 commented May 12, 2026

Copy link
Copy Markdown
Contributor

@zeel2104 thanks for the PR!

I think this still has an edge-case layout bug for very short final groups.

The new code allows any non-zero remainder in the final block group, but this loop still places the added group’s bitmap blocks at:

    group * blocksPerGroup + inodeTableSizePerGroup
    group * blocksPerGroup + inodeTableSizePerGroup + 1

For the 160 MiB case the final group has 8192 blocks, so this is fine. But for a size like 128 MiB + 4 KiB, group 1 has only 1 filesystem block. WithinodeTableSizePerGrouparound 512, the descriptor points to bitmap blocks outside s_blocks_count, and the write seeks beyond the declared filesystem/image size.

Could we either:

  1. round up only the tiny remainder case so the last group has at least inodeTableSizePerGroup + 2 blocks, or
  2. explicitly lay out the last group’s metadata somewhere valid and set the required feature bits if that depends on flex_bg-style placement?

Please add a regression test around the boundary, e.g. minDiskSize: 128.mib() + 4.kib(), checking both file size and descriptor bitmap/table locations are < superblock block count.

@wlan0 wlan0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is still a layout bug

#717 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants