Skip to content

refactor: generate GRUB images #12611

Merged
talos-bot merged 1 commit intosiderolabs:mainfrom
smira:fix/grub-boot
Jan 21, 2026
Merged

refactor: generate GRUB images #12611
talos-bot merged 1 commit intosiderolabs:mainfrom
smira:fix/grub-boot

Conversation

@smira
Copy link
Copy Markdown
Member

@smira smira commented Jan 16, 2026

Simplify the flow a bit by using live partition info,
avoid doing some calculations which are already done in the
partition code.

Remove some steps I believe we don't need to do.

return fmt.Errorf("failed to write back META partition data: %w", err)
}

return gptdev.Sync()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

both .Sync() calls were in the "image" mode, so they flush the pre-allocated file to disk which results in tons of useless I/O, as the file has gaps, and we're going to compress and throw it away in a moment.

So don't call fsync() on the disk image.

})
for idx, p := range pt.Partitions() {
if p.Name == constants.BIOSGrubPartitionLabel {
biosPartition = p
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

if we find the partition in the live partition table, we find the FirstLBA which is the first sector - this is what we need for patching, drop all the math later


partitionImageFile := filepath.Join(i.options.MountPrefix, biosPartitionInfo[0].Label+".img")

if err := utils.CreateRawDisk(i.options.Printf, partitionImageFile, int64(biosPartitionInfo[0].Size), true); err != nil {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we don't need an intermediate file to have an image of the future partition which contains core.img -> just write core.img directly

mbr := make([]byte, 446)

if _, err := bootImg.ReadAt(mbr, 0); err != nil {
if _, err := io.ReadFull(bootImg, mbr); err != nil {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Read() call is not guaranteed to read all bytes (vs. the Write call), so use ReadFull

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i thought newer go fixed the Read(), anyways let's be safe

Copy link
Copy Markdown
Member

@frezbo frezbo left a comment

Choose a reason for hiding this comment

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

🆒

Simplify the flow a bit by using live partition info,
avoid doing some calculations which are already done in the
partition code.

Remove some steps I believe we don't need to do.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
@smira
Copy link
Copy Markdown
Member Author

smira commented Jan 21, 2026

/m

@talos-bot talos-bot merged commit ddd6b18 into siderolabs:main Jan 21, 2026
58 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in Planning Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants