Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partition table helper functions for adding root filesystem and boot partition #1009

Merged
merged 6 commits into from
Nov 6, 2024

Conversation

achilleas-k
Copy link
Member

This is part of #926 which I'm slowly splitting into smaller, bite-sized PRs.


The PR introduces two helper functions, EnsureRootFilesystem() and EnsureBootPartition(). These will add, respectively, a root filesystem and a boot partition to a partition table if they don't already exist.

mvo5
mvo5 previously approved these changes Nov 4, 2024
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Looks good, some ideas/suggestions inline but no blockers just my usual nitpicky self (but some might still be worthwhile).

pkg/disk/partition_table.go Show resolved Hide resolved
pkg/disk/partition_table.go Outdated Show resolved Hide resolved
pkg/disk/partition_table.go Outdated Show resolved Hide resolved
pkg/disk/partition_table.go Outdated Show resolved Hide resolved
pkg/disk/partition_table_test.go Show resolved Hide resolved
pkg/disk/partition_table_test.go Show resolved Hide resolved
pkg/disk/partition_table_test.go Show resolved Hide resolved
pkg/disk/partition_table.go Outdated Show resolved Hide resolved
pkg/disk/partition_table_test.go Outdated Show resolved Hide resolved
mvo5
mvo5 previously approved these changes Nov 5, 2024
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Thank you

ondrejbudai
ondrejbudai previously approved these changes Nov 5, 2024
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

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

I think I found a tiny issue regarding MBR partitions, but I believe that we can fix these in a follow-up since this code isn't actually used anywhere in production yet.

pkg/disk/partition_table.go Outdated Show resolved Hide resolved
pkg/disk/partition_table.go Outdated Show resolved Hide resolved
pkg/disk/partition_table.go Outdated Show resolved Hide resolved
Add a function that takes a partition table and adds a root filesystem,
but only if one does not already exist.  A root filesystem is any
mountable where the mountpoint is `/`.

The function decides whether to create a plain partition, a logical
volume, or a btrfs subvolume based on existing partitions and volumes in
the partition table.

This function is not currently used because our base partition tables
always have root filesystems, but it will be part of the new custom
partition table generator.
mvo5
mvo5 previously approved these changes Nov 6, 2024
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Thank you, looks good, there is lint error that probably needs fixing

pkg/disk/disk.go Outdated Show resolved Hide resolved
pkg/disk/partition_table_test.go Outdated Show resolved Hide resolved
@achilleas-k achilleas-k dismissed mvo5’s stale review November 6, 2024 08:49

The merge-base changed after approval.

Add a function that takes a partition table and adds a boot partition,
but only if one does not already exist.  A boot partition is any
mountable where the mountpoint is `/boot`.  Although the /boot
mountpoint must always be on a plain partition, this is not validated.

This function is not currently used because the NewPartitionTable()
function handles this automatically during layout conversions (in
ensureLVM() and ensureBtrfs()), but it will be part of the new custom
partition table generator.
Define a new const string: DosLinuxTypeID, which is used for Linux
filesystems on dos/mbr partition tables.

In the new helper functions, use the partition table type to determine
which ID to use.  This means that the functions will return an error
when the partition table type is not set (or is set to an unknown
value).

Update the tests to match the new behaviour and to test the correct ID
selection.
@ondrejbudai ondrejbudai added this pull request to the merge queue Nov 6, 2024
Merged via the queue into osbuild:main with commit 088e7a2 Nov 6, 2024
16 of 19 checks passed
@achilleas-k achilleas-k deleted the disk/pt/helper-functions branch November 6, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants