fix: support fat32 on 4k native disks#316
Conversation
deitch
left a comment
There was a problem hiding this comment.
This is well done, thank you.
Do you think it is possible to add a test for this? We don't need anything fancy at this point, I think even creating a 4k fat32-file in testdata/mkfat32.sh and then adding a test to fat32_test.go that opens the file would be plenty.
The only challenge, is how do you make a fat32 4k file without being root, which you cannot rely upon when running go test? Does mkfs.vfat -s 4096 cause the fail in the current one, and work with your fix?
unfortunately for testing this needs a loop block device, otherwise |
It does? I did a quick test, but I must have missed something? $ mkfs.vfat -F 32 -n TEST4K -S 4096 /tmp/vfat_4k.img
mkfs.fat 4.2 (2021-01-31)
$ file /tmp/vfat_4k.img
/tmp/vfat_4k.img: DOS/MBR boot sector, code offset 0x58+2, OEM-ID "mkfs.fat", Bytes/sector 4096, sectors 2560 (volumes <=32 MB), Media descriptor 0xf8, sectors/track 16, FAT (32 bit), sectors/FAT 3, se
rial number 0x52963158, label: "TEST4K "
$ fsck.fat -n -v /tmp/vfat_4k.img
fsck.fat 4.2 (2021-01-31)
Checking we can access the last sector of the filesystem
Warning: Filesystem is FAT32 according to fat_length and fat32_length fields,
but has only 2522 clusters, less than the required minimum of 65525.
This may lead to problems on some systems.
Boot sector contents:
System ID "mkfs.fat"
Media byte 0xf8 (hard disk)
4096 bytes per logical sector
4096 bytes per cluster
32 reserved sectors
First FAT starts at byte 131072 (sector 32)
2 FATs, 32 bit entries
12288 bytes per FAT (= 3 sectors)
Root directory start at cluster 2 (arbitrary size)
Data area starts at byte 155648 (sector 38)
2522 data clusters (10330112 bytes)
16 sectors/track, 2 heads
0 hidden sectors
2560 sectors total
Checking for unused clusters.
Checking free cluster summary.
/tmp/vfat_4k.img: 1 files, 1/2522 clusters
$ minfo -i /tmp/vfat_4k.img
device information:
===================
filename="/tmp/vfat_4k.img"
sectors per track: 16
heads: 2
cylinders: 80
media byte: f8
mformat command line:
mformat -t 80 -h 2 -s 16 -S 5 -r 0 -R 32 -L 3 -m 248 -K 6 -i "/tmp/vfat_4k.img" ::
bootsector information
======================
banner:"mkfs.fat"
sector size: 4096 bytes
cluster size: 1 sectors
reserved (boot) sectors: 32
fats: 2
max available root directory slots: 0
small size: 2560 sectors
media descriptor byte: 0xf8
sectors per fat: 0
sectors per track: 16
heads: 2
hidden sectors: 0
physical drive id: 0x80
reserved=0x0
dos4=0x29
serial number: 52963158
disk label="TEST4K "
disk type="FAT32 "
Big fatlen=3
Extended flags=0x0000
FS version=0x0000
rootCluster=2
infoSector location=1
backup boot sector=6
Infosector:
signature=0x41615252
free clusters=2521
last allocated cluster=2That looks like 4096? When I ran your sample program on the generated file, it failed (as expected). |
sorry it does, I must have missed something, pushing a test |
8da81a7 to
2e9a816
Compare
|
okay, pushed |
This fixes the issue that `GetFilesystem()` fails on vfat created on disks with 4k sector size.
`mkfs.vfat` uses 4k size when disk is native 4k.
This can be reproduced by creating a loopback device with 4k sectors and running mkfs.vfat on it.
```bash
truncate -s 10M /tmp/vfat_4k.img
sudo losetup -f --show -b 4096 /tmp/vfat_4k.img
sudo mkfs.vfat -F 32 -n TEST4K /dev/loop0
sudo losetup -d /dev/loop0
```
Then can be tested as:
```go
package main
import (
"github.com/diskfs/go-diskfs"
"github.com/diskfs/go-diskfs/backend/file"
"github.com/stretchr/testify/require"
)
func main() {
// Open the image file created with 4K sectors
bk, err := file.OpenFromPath("/tmp/vfat_4k.img", false)
require.NoError(err, "failed to open image file")
defer bk.Close()
// Try to open the disk
diskInfo, err := diskfs.OpenBackend(bk, diskfs.WithOpenMode(diskfs.ReadWriteExclusive))
require.NoError(err, "failed to open backend")
defer diskInfo.Close()
// This will fail with unpatched go-diskfs:
// "unknown filesystem on partition 0"
// Because go-diskfs rejects bytesPerSector != 512
fs, err := diskInfo.GetFilesystem(0)
require.NoError(err, "failed to get filesystem from partition 0")
defer fs.Close()
}
```
When mkfs.vfat formats a device with 4K logical sectors, it writes bytesPerSector=4096 in the BPB, which go-diskfs rejects.
Signed-off-by: Noel Georgi <git@frezbo.dev>
2e9a816 to
78e404b
Compare
|
I think the test failure is a just CI hiccup. I restarted it. |
When go-diskfs opens a 4k native block device, it detects the logical block size as 4096 from the OS and passes this to fat32.Read(). However, the validation was rejecting any blocksize other than 0 or 512, causing 'unknown filesystem' errors. This is a follow-up to PR diskfs#316 which added support for reading FAT32 filesystems with bytesPerSector=4096 from the BPB, but didn't update the blocksize validation. The blocksize parameter is only used for validation; the actual bytesPerSector is always read from the BPB. Fixes reading VFAT filesystems on 4k native disks when opened as block devices (e.g., /dev/vda1). Signed-off-by: Noel Georgi <git@frezbo.dev>
When go-diskfs opens a 4k native block device, it detects the logical block size as 4096 from the OS and passes this to fat32.Read(). However, the validation was rejecting any blocksize other than 0 or 512, causing 'unknown filesystem' errors. This is a follow-up to PR diskfs#316 which added support for reading FAT32 filesystems with bytesPerSector=4096 from the BPB, but didn't update the blocksize validation. The blocksize parameter is only used for validation; the actual bytesPerSector is always read from the BPB. Fixes reading VFAT filesystems on 4k native disks when opened as block devices (e.g., /dev/vda1). Also fix the write code for 4k disks. Signed-off-by: Noel Georgi <git@frezbo.dev>
…320) When go-diskfs opens a 4k native block device, it detects the logical block size as 4096 from the OS and passes this to fat32.Read(). However, the validation was rejecting any blocksize other than 0 or 512, causing 'unknown filesystem' errors. This is a follow-up to PR #316 which added support for reading FAT32 filesystems with bytesPerSector=4096 from the BPB, but didn't update the blocksize validation. The blocksize parameter is only used for validation; the actual bytesPerSector is always read from the BPB. Fixes reading VFAT filesystems on 4k native disks when opened as block devices (e.g., /dev/vda1). Also fix the write code for 4k disks. Signed-off-by: Noel Georgi <git@frezbo.dev>
This fixes the issue that
GetFilesystem()fails on vfat created on disks with 4k sector size.mkfs.vfatuses 4k size when disk is native 4k.This can be reproduced by creating a loopback device with 4k sectors and running mkfs.vfat on it.
Then can be tested as:
When mkfs.vfat formats a device with 4K logical sectors, it writes bytesPerSector=4096 in the BPB, which go-diskfs rejects.