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

aarch64 uses dos format partitions #551

Closed
cgwalters opened this issue Jul 18, 2024 · 4 comments · Fixed by #582
Closed

aarch64 uses dos format partitions #551

cgwalters opened this issue Jul 18, 2024 · 4 comments · Fixed by #582

Comments

@cgwalters
Copy link
Contributor

https://github.com/osbuild/bootc-image-builder/blame/babb61b0112a7b55b4d1137f200074a86ff25f61/bib/cmd/bootc-image-builder/partition_tables.go#L79

Why?? git annotate points to 344a1c4 cc @achilleas-k

@cgwalters
Copy link
Contributor Author

While we're here, I think we should fix ppc64le and s390x in osbuild to use gpt.

For reference it looks like the coreos pipeline is doing this, e.g. https://github.com/coreos/coreos-assembler/blob/aea950cfadfae9eae9b92c6bb22bfaafe84c092c/src/osbuild-manifests/coreos.osbuild.s390x.mpp.yaml#L59

@cgwalters
Copy link
Contributor Author

@yoheiueda (in case you know)...is there any reason not to use gpt for s390x? AFAICS it seems today Anaconda in RHEL9.4 defaults to dos...which, just skimming the code looks like it's probably this https://github.com/rhinstaller/anaconda/blob/19df67bad509b4cb65ff23c24a7d54ee586b5e68/pyanaconda/modules/storage/initialization.py#L95 just because anaconda treats zipl as "mbr"?

And I'm guessing that the osbuild Go reimplementation of this just inherited that logic. But ISTM that we should really change Anaconda too to just default to GPT unconditionally nowadays. I guess there's maybe a vague concern about old x86_64 BIOS systems, but that's clearly not relevant for s390x.

(Random aside, searching for "anaconda gpt" is now dominated by AI-related search results because "gpt" acquired a much more popular expansion in recent years, and "anaconda" was previously also a separate python installer...)

@yoheiueda
Copy link
Contributor

@cgwalters zipl supports GPT, so I can't come up with any ideas to use dos partition table format.

@achilleas-k
Copy link
Member

https://github.com/osbuild/bootc-image-builder/blame/babb61b0112a7b55b4d1137f200074a86ff25f61/bib/cmd/bootc-image-builder/partition_tables.go#L79

Why?? git annotate points to 344a1c4 cc @achilleas-k

We should switch to gpt wherever possible, I agree. For some history about this:

The original switch for the aarch64 image happened in osbuild/images@92cdb0f for the Fedora IoT images, which were the basis for the bootc disk configurations. The change was made to add support for Raspberry Pi boards and to match the Fedora ImageFactory configurations.

As for the s390x, I don't know. My guess it's been dos since forever and no one requested a change for it when most image definitions moved to gpt by default.

cgwalters added a commit to cgwalters/bootc-image-builder that referenced this issue Aug 1, 2024
Closes: osbuild#551

There was a fair amount of copy-paste even when we just had
aarch64+x86_64, the addition of duplicate bootfs/rootfs definitions
for ppc64le+s390x is a strong motivation to finally define shared
constants.

I also factored out a shared `const` with some comments
for the disk UUID that will save someone some time wondering
about it later (this textually but not logically conflicts
with osbuild#568
but I think this makes sense to land first).

Signed-off-by: Colin Walters <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Aug 2, 2024
Closes: #551

There was a fair amount of copy-paste even when we just had
aarch64+x86_64, the addition of duplicate bootfs/rootfs definitions
for ppc64le+s390x is a strong motivation to finally define shared
constants.

I also factored out a shared `const` with some comments
for the disk UUID that will save someone some time wondering
about it later (this textually but not logically conflicts
with #568
but I think this makes sense to land first).

Signed-off-by: Colin Walters <[email protected]>
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 a pull request may close this issue.

3 participants