WIP Enforce FAT32 Minimum Cluster Count #4139

Smithx10 · 2025-07-03T16:57:50Z

This PR fixes #4123 and improves the -f image to not pull if image exists in local docker repository.

What I did:
Enforced the minimum cluster count for FAT32 was achieved.

Currently its difficult to iterate on images defined for -f. Made a change so if the -f image is local it doesn't try to pull it. Thinking about it now... it should probably honor the --pull Always pull images flag.[

Questions:
Would it be better to have a "raw-efi" that produced the current as few clusters as needed and a raw-efi-valid?

Currently the raw-efi maker appends "text" to the grub cmdline option. Is this ok? I believe this breaks intended ttyS0 output since it is always the last entry. https://github.com/linuxkit/linuxkit/blob/master/docs/faq.md#console-not-displaying-init-or-containerd-output-at-boot We could fix that in this pr if so.

cat >> EFI/BOOT/grub.cfg <<EOF
set timeout=0
set gfxpayload=text
menuentry 'LinuxKit ISO Image' {
	linux /kernel ${CMDLINE} text
  initrd /initrd.img
}
EOF

deitch · 2025-07-03T17:36:46Z

src/cmd/linuxkit/moby/build/docker.go

-	if err := pull.Run(); err != nil {
-		if exitError, ok := err.(*exec.ExitError); ok {
-			return fmt.Errorf("docker pull %s failed: %v output:\n%s", img, err, exitError.Stderr)
+	// Only pull if its not local.


I don't think there is any need for this change. When you do docker pull, it pulls the tag, checks the manifest, and if its digest matches the local, it doesn't pull anything. This is effectively a no-cost op.

What am I doing wrong here then?

I built this image using

linuxkit pkg build ./tools/mkimage-raw-efi --docker

Docker version 28.2.2, build e6534b4eb7 ❯ docker images | grep 2bbcc8d38a9c44a50378ec6de9e3100e9901ac34-dirty-d6aa51a linuxkit/mkimage-raw-efi 2bbcc8d38a9c44a50378ec6de9e3100e9901ac34-dirty-d6aa51a d8c776f56d8f 14 hours ago 28.3MB linuxkit/mkimage-raw-efi 2bbcc8d38a9c44a50378ec6de9e3100e9901ac34-dirty-d6aa51a-amd64 d8c776f56d8f 14 hours ago 28.3MB ❯ docker pull linuxkit/mkimage-raw-efi:2bbcc8d38a9c44a50378ec6de9e3100e9901ac34-dirty-d6aa51a Error response from daemon: manifest for linuxkit/mkimage-raw-efi:2bbcc8d38a9c44a50378ec6de9e3100e9901ac34-dirty-d6aa51a not found: manifest unknown: manifest unknown

Ah, that is a good point. It works if the image already is on the registry. docker pull explicitly means, "check the registry and get the latest manifest hash." If it is not published, it will not work.

I wouldn't change that for the use case, or at least not as part of an unrelated PR. You can test it manually via docker build like you clearly did (or more likely linuxkit pkg build), and then do the 2-step I showed in the other comment:

linuxkit build --format tar-kernel-initrd

cat test-initrd.tar | docker run -i linuxkit/mkimage-raw-efi:<your_tag> > test-efi.img

Sounds good. I'll revert that.

Is there a desire to maybe have a user wiki or someplace to update guides / tricks etc. While I'm using this, It will be nice to share.

wiki not so much, but the docs/ directory is there, no one will complain about contributions. 😄

deitch · 2025-07-03T17:39:48Z

tools/mkimage-raw-efi/make-efi

 ESP_FILE_SIZE_KB=$(( ( ( ($ESP_DATA_SIZE + $FAT_OVERHEAD_SIZE + 1024 - 1) / 1024 ) + 1024 - 1) / 1024 * 1024 ))
+
+# Enforce valid FAT32 cluster count
+MIN_CLUSTERS=65525


I see what you did here. This is good.

Is there any way to do this without the while :; do loop? This is just math, so it should be possible to figure it out without the loop.

Maybe something in the opposite direction:

Calculate the minimum ESP_FILE_SIZE_KB, which you know if you have all of the other numbers

If ESP_FILE_SIZE_KB is too small, just make it the minimum

I asked chatgpt for its idea of the most precise way to do it. Its wild lol.
resulted in the following:
fsck.fat 4.2 (2021-01-31)
/dev/zvol/zroot/virt/lkit-part1: 6 files, 26269/65653 clusters

I think we can just get away with setting it to 33.1 MB or something to enforce it

To calculate it though I believe its recursive, unless this is wrong.
total_size → cluster_count → FAT size → total_size again
That means:
You think you can calculate it deterministically...
But unless you account for FAT size based on cluster count, you'll underestimate.

Yeah, but you're looking for the minimum. Shouldn't that minimum be a fixed number?

I asked chatgpt for its idea of the most precise way to do it. Its wild lol.

It really is. I am involved with a project where I needed some complex SQL for all 4 supported databases. While it got some things wring, overall, it got most things right. It would have taken me 3-4x to get it myself.

What if you just made a minimum size?

Looking at the final command:

mkfs.vfat -v -F $FAT_SIZE -S $SECTOR_SIZE -s $SECTORS_PER_CLUSTER -R $RESERVED_SECTORS -C $ESP_FILE $(( $ESP_FILE_SIZE_KB )) > /dev/null

We already have:

FAT_SIZE = 32

SECTOR_SIZE = 512

SECTORS_PER_CLUSTER = 8 (so cluster size = 4K)

Since the minimum number of clusters is 65525, the minimum volume size must be 256 MB. Could we just do:

ESP_MIN_SIZE=$(( $SECTOR_SIZE * $SECTORS_PER_CLUSTER * $MIN_CLUSTERS )) ESP_DATA_SIZE=$(( $KERNEL_FILE_SIZE + $INITRD_FILE_SIZE + $EFI_FILE_SIZE + $GRUB_FILE_SIZE )) if [ "$ESP_DATA_SIZE" -lt "$ESP_MIN_SIZE ]; then ESP_DATA_SIZE="$ESP_MIN_SIZE" fi

That worked.

❯ sudo fsck.vfat /dev/zvol/zroot/virt/lkit-part1
fsck.fat 4.2 (2021-01-31)
/dev/zvol/zroot/virt/lkit-part1: 6 files, 26269/65653 clusters

deitch · 2025-07-04T15:56:55Z

Oops. Commit needs to be signed.

Smithx10 · 2025-07-07T16:53:38Z

Did I get the signing right? I used -S but thinking that may have been wrong*

deitch · 2025-07-10T12:53:56Z

Ah, no, completely different things.

From git commit --help:

-s, --signoff, --no-signoff
           Add a Signed-off-by trailer by the committer at the end of the commit log message. The meaning of a signoff depends on the project to which you’re
           committing. For example, it may certify that the committer has the rights to submit the work under the project’s license or agrees to some
           contributor representation, such as a Developer Certificate of Origin. (See https://developercertificate.org for the one used by the Linux kernel and
           Git projects.) Consult the documentation or leadership of the project to which you’re contributing to understand how the signoffs are used in that
           project.

           The --no-signoff option can be used to countermand an earlier --signoff option on the command line.

vs:

-S[<keyid>], --gpg-sign[=<keyid>], --no-gpg-sign
           GPG-sign commits. The keyid argument is optional and defaults to the committer identity; if specified, it must be stuck to the option without a
           space.  --no-gpg-sign is useful to countermand both commit.gpgSign configuration variable, and earlier --gpg-sign.

-S gives it far more than it asks for.

Signed-off-by: Bruce Smith <bruce_smith_it@fastmail.com>

ChrisIgel · 2025-10-09T00:43:41Z

I'm pretty sure the current change is at the wrong position. Lines 71-73 should probably be replaced with this:

MIN_CLUSTERS=65525
ESP_MIN_SIZE=$(( $SECTOR_SIZE * $SECTORS_PER_CLUSTER * $MIN_CLUSTERS ))
ESP_DATA_SIZE=$(( $KERNEL_FILE_SIZE + $INITRD_FILE_SIZE + $EFI_FILE_SIZE + $GRUB_FILE_SIZE ))
if [ "$ESP_DATA_SIZE" -lt "$ESP_MIN_SIZE" ]; then
    echo "adjusted data size because of min size threshold"
    ESP_DATA_SIZE="$ESP_MIN_SIZE"
fi

deitch · 2025-10-09T14:00:40Z

I think @ChrisIgel is correct. The change is good, but it should be higher up to replace what is there.

By the way, where is $MIN_CLUSTERS defined?

deitch reviewed Jul 3, 2025

View reviewed changes

Smithx10 changed the title ~~WIP Enforce FAT32 Minimum Cluster Count & build -f dev UX cache behavior~~ WIP Enforce FAT32 Minimum Cluster Count Jul 3, 2025

Smithx10 force-pushed the raw_efi_4123 branch from 873702d to baedc65 Compare July 4, 2025 15:42

deitch approved these changes Jul 4, 2025

View reviewed changes

Smithx10 force-pushed the raw_efi_4123 branch from baedc65 to 78947db Compare July 4, 2025 18:33

Enforce a min of 256MB.

905513e

Signed-off-by: Bruce Smith <bruce_smith_it@fastmail.com>

Smithx10 force-pushed the raw_efi_4123 branch from 78947db to 905513e Compare July 10, 2025 21:47

ChrisIgel mentioned this pull request Oct 9, 2025

Switch to systemd-boot for raw-efi builds #4183

Merged

WIP Enforce FAT32 Minimum Cluster Count #4139

Are you sure you want to change the base?

WIP Enforce FAT32 Minimum Cluster Count #4139

Uh oh!

Conversation

Smithx10 commented Jul 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Smithx10 Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deitch commented Jul 4, 2025

Uh oh!

Smithx10 commented Jul 7, 2025

Uh oh!

deitch commented Jul 10, 2025

Uh oh!

ChrisIgel commented Oct 9, 2025

Uh oh!

deitch commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Smithx10 Jul 3, 2025 •

edited

Loading