linuxkit / linuxkit

A toolkit for building secure, portable and lean operating systems for containers
Apache License 2.0
8.26k stars 1.01k forks source link

raw-bios format creates incorrect partition type #3147

Open medic15 opened 6 years ago

medic15 commented 6 years ago

Description

The raw-bios output format creates a disk image with a FAT file system in a Linux partition.

Steps to reproduce the issue:

$ linuxkit build -format raw-bios linuxkit.yml 
Extract kernel image: linuxkit/kernel:4.14.58
Add init containers:
Process init image: linuxkit/init:v0.6
. . .
Add files:
  etc/linuxkit-config
Create outputs:
  linuxkit-bios.img

Describe the results you received:

$ fdisk -l linuxkit-bios.img 
Disk linuxkit-bios.img: 67 MiB, 70254592 bytes, 137216 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0xaadcbff4

Device             Boot Start    End Sectors Size Id Type
linuxkit-bios.img1 *     2048 131071  129024  63M 83 Linux

Describe the results you expected:

The partition should be a type compatible with vfat. FAT32 should be a safe choice since even a minimal image is larger than the minimum FAT32 partition size.

Additional information:

Here's a patch for the fix:

diff --git a/tools/mkimage-raw-bios/make-bios b/tools/mkimage-raw-bios/make-bios
index abec07d..74c27f4 100755
--- a/tools/mkimage-raw-bios/make-bios
+++ b/tools/mkimage-raw-bios/make-bios
@@ -84,7 +84,7 @@ dd if=/dev/zero of=$IMGFILE bs=1M count=$MB_BLOCKS

 echo "w" | fdisk $IMGFILE || true
 # format one large partition of the apprropriate size
-echo ","$ESP_FILE_SIZE_SECTORS",;" | sfdisk $IMGFILE
+echo ","$ESP_FILE_SIZE_SECTORS",0x0b;" | sfdisk $IMGFILE

 # of course, "one large partition" means minus the 2048 at the beginning
 ESP_SECTOR_START=2048
deitch commented 6 years ago

No objection to changing the partition type from the default linux, but do you mind explaining how it matters? What systems aren’t you seeing having issues booting?

medic15 commented 6 years ago

No problems booting but it was a point of confusion for me. I came across this while working on the field update functionality for my product. It took me a couple of hours to dig into this to determine if there was a good reason for the mismatch. The fix is straightforward and might save some future developer the same confusion.

deitch commented 6 years ago

The fix is straightforward and might save some future developer the same confusion

Yes, good point. Even if we decide to stick with Linux partition, we should be explicit about it, rather than relying on default.

My concern primarily is that we just use mkfs.vfat without specifying the FAT size (-F 12, -F 16, or -F 32), so if we have a small one and end up with FAT16 but a partition type of FAT32, it could be an issue. Then again, anyone using LinuxKis with the default is unlikely to be using FAT12. And, again, I prefer to explicitly use a partition type and then have an option to change it if needed, rather than have it switch based on sizes.

OK, let's get a PR in for this. Want to open it?