diskfs / go-diskfs

MIT License
516 stars 113 forks source link

ext4 partition create issues #247

Open Itxaka opened 2 months ago

Itxaka commented 2 months ago

Hey there!

as part of our hackweek, we are trying to move the Kairos installer (https://github.com/kairos-io/kairos-agent) to use pure go instead of shelling out and having to depend on external binaries.

Diskfs works wonderfully, it was so easy to create and partition a disk! great work there.

My main issue came when trying to create a partition in ext4 format. It looks like some of the blockgroups calculation is wrong as you get an error dividing by zero.

Minimum reproducible case:

package main

import (
    "github.com/diskfs/go-diskfs"
    "github.com/diskfs/go-diskfs/disk"
    "github.com/diskfs/go-diskfs/filesystem"
    "github.com/diskfs/go-diskfs/partition/gpt"
)

func main() {
    d, err := diskfs.Open("/dev/vda", diskfs.WithSectorSize(512))
    if err != nil {
        panic(err)
    }
    sizeFirst := uint64(10 * 1024 * 1024)
    startFirst := 1024 * 1024 / uint64(diskfs.SectorSize512)
    endFirst := sizeFirst/uint64(diskfs.SectorSize512) + startFirst - 1
    sizeSecond := uint64(20 * 1024 * 1024)
    startSecond := endFirst + 1
    endSecond := sizeSecond/uint64(diskfs.SectorSize512) + startSecond - 1

    table := &gpt.Table{
        LogicalSectorSize:  int(diskfs.SectorSize512),
        PhysicalSectorSize: int(diskfs.SectorSize512),
        ProtectiveMBR:      true,
        Partitions: []*gpt.Partition{
            {
                Start: startFirst,
                End:   endFirst,
                Size:  sizeFirst,
                Type:  gpt.EFISystemPartition,
                Name:  "part1",
            },
            {
                Start: startSecond,
                End:   endSecond,
                Size:  sizeSecond,
                Type:  gpt.LinuxFilesystem,
                Name:  "part2",
            },
        },
    }
    err = d.Partition(table)
    if err != nil {
        panic(err)
    }

    _, err = d.CreateFilesystem(disk.FilesystemSpec{
        Partition:   0,
        FSType:      filesystem.TypeFat32,
        VolumeLabel: "part1",
    })
    if err != nil {
        panic(err)
    }

    _, err = d.CreateFilesystem(disk.FilesystemSpec{
        Partition:   1,
        FSType:      filesystem.TypeExt4,
        VolumeLabel: "part1",
    })
    if err != nil {
        panic(err)
    }

}

This is a very simple reproducible case, you open /dev/vda, create a table with 2 partitions, and then create the filesystem. Disk and partition creation goes correctly with zero issues and fat32 filesystem creation as well, but ext4 creation fails.

panic: invalid sectors per block 0, must be between 2 and 128 sectors

goroutine 1 [running]:
main.main()
    /home/itxaka/projects/kairos-agent/main.go:124 +0x26a

Looks like because the disk.CreateFilesystem method calls the ext4.Create with no Params, the check on https://github.com/diskfs/go-diskfs/blob/master/filesystem/ext4/ext4.go#L158 will always fail, because the Params.SectorsPerBlock is gonna be always zero

Okay, no problem thats fixable, we can manually call the ext4.Create ourselves with proper params :)

so we substitute in our example this

_, err = d.CreateFilesystem(disk.FilesystemSpec{
        Partition:   1,
        FSType:      filesystem.TypeExt4,
        VolumeLabel: "part1",
    })

for this

params := ext4.Params{SectorsPerBlock: 16}
    _, err = ext4.Create(d.File, d.Table.GetPartitions()[1].GetSize(), d.Table.GetPartitions()[1].GetStart(), d.LogicalBlocksize, &params)

But that results in a panic as well:

panic: runtime error: integer divide by zero

goroutine 1 [running]:
github.com/diskfs/go-diskfs/filesystem/ext4.Create({0x5530f8, 0xc000052008}, 0x1400000, 0xb00000, 0x200?, 0x0?)
    /home/itxaka/projects/go-diskfs/filesystem/ext4/ext4.go:230 +0x188a
main.main()
    /home/itxaka/projects/kairos-agent/main.go:120 +0x305

This seems to point to https://github.com/diskfs/go-diskfs/blob/master/filesystem/ext4/ext4.go#L226 being the issue. I added some logging around and it kind of makes sense as at that point the values are:

// inodecount: 2560
// blockgroups: 0
inodesPerGroup := int64(inodeCount) / blockGroups

Which points to https://github.com/diskfs/go-diskfs/blob/master/filesystem/ext4/ext4.go#L194 in which the values are:

// numblocks: 2560
// blocksPerGroup: 65536
blockGroups := numblocks / int64(blocksPerGroup)

Which points to blocksPerGroup being too big to divide numblocks onto it. Again this comes from the Params having an empty value for BlocksPerGroup and it defaulting to blocksize * 8 at https://github.com/diskfs/go-diskfs/blob/master/filesystem/ext4/ext4.go#L184

so lets add those to params

params := ext4.Params{SectorsPerBlock: 2, BlocksPerGroup: 256}

Which causes another panic :D

panic: runtime error: integer divide by zero

goroutine 1 [running]:
github.com/diskfs/go-diskfs/filesystem/ext4.Create({0x5530f8, 0xc000052040}, 0x1400000, 0xb00000, 0x200?, 0x0?)
    /home/itxaka/projects/go-diskfs/filesystem/ext4/ext4.go:412 +0x189a
main.main()
    /home/itxaka/projects/kairos-agent/main.go:120 +0x305

This time its the https://github.com/diskfs/go-diskfs/blob/master/filesystem/ext4/ext4.go#L398 check as maxBlockGroups is 0. This is due to maxFilesystemSize64Bit resolving to 62 so the operation ends up being zero

// blocksize 1024
// blocksPerGroup 256
// maxFilesystemSize64Bit 62
maxBlockGroups = maxFilesystemSize64Bit / (uint64(blocksPerGroup) * uint64(blocksize))

Now here I have no idea how to continue troubleshooting this as I dont know what maxFilesystemSize64Bit refers to. Is this the maximum size of a filesystem in ext4 in bytes? is that missing a transformation to bytes or something? https://github.com/diskfs/go-diskfs/blob/master/filesystem/ext4/ext4.go#L59

Sorry about the big wall of text. I know enough to troubleshoot but not enough about ext4 to tackle this, otherwise I would have send a PR to try to fix it, but that maxFilesystemSize64Bit got me stumped :D

Running version v1.4.1

deitch commented 2 months ago

Hi @Itxaka

No, don't apologize for the wall of text. This is helpful, and much better than "I tried it, it didn't work." 😃

I'll try to dig in over the coming days.

deitch commented 2 months ago

Modified it slightly to enable passing parameters and creating a disk image if it does not exist:

package main

import (
        "os"

        "github.com/diskfs/go-diskfs"
        "github.com/diskfs/go-diskfs/disk"
        "github.com/diskfs/go-diskfs/filesystem"
        "github.com/diskfs/go-diskfs/partition/gpt"
)

func main() {
        filePath := os.Args[1]
        var fileSize int64 = 200*1024*1024
        if err := createFileIfNotExists(filePath, fileSize); err != nil {
                panic(err)
        }
        d, err := diskfs.Open(filePath, diskfs.WithSectorSize(512))
        if err != nil {
                panic(err)
        }
        sizeFirst := uint64(10 * 1024 * 1024)
        startFirst := 1024 * 1024 / uint64(diskfs.SectorSize512)
        endFirst := sizeFirst/uint64(diskfs.SectorSize512) + startFirst - 1
        sizeSecond := uint64(20 * 1024 * 1024)
        startSecond := endFirst + 1
        endSecond := sizeSecond/uint64(diskfs.SectorSize512) + startSecond - 1

        table := &gpt.Table{
                LogicalSectorSize:  int(diskfs.SectorSize512),
                PhysicalSectorSize: int(diskfs.SectorSize512),
                ProtectiveMBR:      true,
                Partitions: []*gpt.Partition{
                        {
                                Start: startFirst,
                                End:   endFirst,
                                Size:  sizeFirst,
                                Type:  gpt.EFISystemPartition,
                                Name:  "part1",
                        },
                        {
                                Start: startSecond,
                                End:   endSecond,
                                Size:  sizeSecond,
                                Type:  gpt.LinuxFilesystem,
                                Name:  "part2",
                        },
                },
        }
        err = d.Partition(table)
        if err != nil {
                panic(err)
        }

        _, err = d.CreateFilesystem(disk.FilesystemSpec{
                Partition:   0,
                FSType:      filesystem.TypeFat32,
                VolumeLabel: "part1",
        })
        if err != nil {
                panic(err)
        }

        _, err = d.CreateFilesystem(disk.FilesystemSpec{
                Partition:   1,
                FSType:      filesystem.TypeExt4,
                VolumeLabel: "part1",
        })
        if err != nil {
                panic(err)
        }

}

func createFileIfNotExists(filePath string, fileSize int64) error {
        // Check if the file exists
        _, err := os.Stat(filePath)
        if err == nil {
                // File exists, do nothing
                return nil
        }
        if !os.IsNotExist(err) {
                // If the error is not "file does not exist", return the error
                return err
        }

        // File does not exist, create it
        file, err := os.Create(filePath)
        if err != nil {
                return err
        }
        defer file.Close()

        // Set the file size
        err = file.Truncate(fileSize)
        if err != nil {
                return err
        }

        return nil
}
deitch commented 2 months ago

The whole Create() part is fairly new, just enabled a few weeks ago. I already am seeing some missing pieces. The maxfilesystem parts had some errors, those will be fixed shortly. It still is missing the gdt creation here. It just creates blank ones. Happy to take help on it.