Bug 1219965 - rhts_partitions snippet should create /boot/efi for aarch64 systems
Summary: rhts_partitions snippet should create /boot/efi for aarch64 systems
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Beaker
Classification: Retired
Component: lab controller
Version: 20
Hardware: aarch64
OS: Linux
medium
medium
Target Milestone: 20.2
Assignee: matt jia
QA Contact: tools-bugs
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-05-08 20:26 UTC by Jeff Bastian
Modified: 2018-02-06 00:41 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-07-14 08:04:42 UTC
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1198881 0 high CLOSED allow a recipe to specify custom partitioning commands in <partitions/> 2021-02-22 00:41:40 UTC

Internal Links: 1198881

Description Jeff Bastian 2015-05-08 20:26:57 UTC
Description of problem:
Bug 1088761 enabled creating /boot/efi on x86 UEFI systems, but it does not work on aarch64 systems which also use UEFI.  I tried running a job with ks_meta="fstype=ext4" on an aarch64 system and the generated kickstart contained:

zerombr
clearpart --all --initlabel
part /boot --size 200 --recommended --asprimary --fstype ext4
part / --size 1024 --grow --fstype ext4
part swap --recommended


I also tried adding a key/value NETBOOT_METHOD=efigrub to the aarch64 system but it still did not create a /boot/efi partition.  From the comments, it seems the NETBOOT_METHOD=efigrub key/value pair only applies to x86 systems.

https://git.beaker-project.org/cgit/beaker/tree/Server/bkr/server/model/inventory.py?h=release-20#n711

Version-Release number of selected component (if applicable):
beaker 20

How reproducible:
every time

Steps to Reproduce:
1. run a job with ks_meta="fstype=ext4" on an aarch64 system

Actual results:
the kickstart file is missing /boot/efi

Expected results:
kickstart creates /boot/efi partition

Additional info:

Comment 2 Jeff Bastian 2015-05-08 20:41:57 UTC
Indeed, the has_efi check is only applied on i386 and x86_64 systems:

{% elif distro_tree is arch('i386', 'x86_64') %}
{% if system and system.has_efi %}
{# x86 EFI #}
part /boot/efi --fstype vfat --size 200 --recommended
{%- if ondisk %} --ondisk={{ ondisk }}{% endif %}

https://git.beaker-project.org/cgit/beaker/tree/Server/bkr/server/snippets/rhts_partitions?h=release-20#n26

Comment 3 Jeff Bastian 2015-05-08 20:44:21 UTC
Rather than using system.has_efi, it would probably be better to treat aarch64 like ia64:

{% elif distro_tree is arch('aarch64') %}
part /boot/efi --fstype vfat --size 200 --recommended
{%- if ondisk %} --ondisk={{ ondisk }}{% endif %}

Comment 4 Dan Callaghan 2015-05-11 00:04:56 UTC
Thanks for the report Jeff.

(In reply to Jeff Bastian from comment #3)
> Rather than using system.has_efi, it would probably be better to treat
> aarch64 like ia64:
> 
> {% elif distro_tree is arch('aarch64') %}
> part /boot/efi --fstype vfat --size 200 --recommended
> {%- if ondisk %} --ondisk={{ ondisk }}{% endif %}

This is probably the right approach.

I think Fedora can be installed on some existing hardware using devicetree instead of EFI so in those cases /boot/efi may not be needed? However the future seems to be quickly converging on EFI for aarch64 so we probably don't need to worry about other possibilities.

Comment 5 Jeff Bastian 2015-05-11 17:46:11 UTC
Yes, Fedora can be run on U-boot systems, however, the ARM SBSA (Server Base System Architecture) requires UEFI + ACPI on server class 64-bit ARM systems, and my guess is Beaker instances are more likely to have server class hardware.  

However, to be friendly to U-boot systems, maybe we could add a flag to disable creating /boot/efi?  Something like this:

{% elif distro_tree is arch('aarch64') %}
{%- if uboot is not defined %}
part /boot/efi --fstype vfat --size 200 --recommended
{%- if ondisk %} --ondisk={{ ondisk }}{% endif %}
{% endif %}

So, it will create /boot/efi by default on aarch64 systems, unless you set ks_meta="uboot" which will disable it.

Comment 6 matt jia 2015-05-20 03:32:54 UTC
(In reply to Jeff Bastian from comment #5)
> Yes, Fedora can be run on U-boot systems, however, the ARM SBSA (Server Base
> System Architecture) requires UEFI + ACPI on server class 64-bit ARM
> systems, and my guess is Beaker instances are more likely to have server
> class hardware.  
> 
> However, to be friendly to U-boot systems, maybe we could add a flag to
> disable creating /boot/efi?  Something like this:
> 
> {% elif distro_tree is arch('aarch64') %}
> {%- if uboot is not defined %}
> part /boot/efi --fstype vfat --size 200 --recommended
> {%- if ondisk %} --ondisk={{ ondisk }}{% endif %}
> {% endif %}
> 
> So, it will create /boot/efi by default on aarch64 systems, unless you set
> ks_meta="uboot" which will disable it.

That sounds like a good idea. We can add uboot into the custom kickstart metadata variables.

https://beaker-project.org/docs-develop/user-guide/customizing-installation.html#kickstart-metadata

Comment 7 matt jia 2015-05-28 05:42:13 UTC
On Gerrit:

  http://gerrit.beaker-project.org/#/c/4226/

Comment 8 Dan Callaghan 2015-06-09 23:52:04 UTC
Do we really need to bother adding a uboot ksmeta variable? I am worried that it is adding complexity (not to mention, a somewhat confusingly named variable) for a very small corner case.

If someone has an aarch64 system using UBoot, won't the /boot/efi partition just go unused without causing any harm? Maybe we can just unconditionally define /boot/efi on aarch64 and that will be good enough.

Comment 9 Jeff Bastian 2015-06-10 19:47:14 UTC
Personally, I'd be ok with creating /boot/efi unconditionally.  Yes, it'll just be an unused partition on uboot systems.  Other than wasting a small bit (200M) of disk space, it won't hurt anything.

I was trying to be "friendly" to uboot systems by adding this variable, but I think this actually is a deeper issue (which will need a new BZ): you can add additional partitions with the <partitions> tag in a recipe, but you cannot change or override the base partitions (/ and /boot) unless you use <kickstart> which requires the full kickstart set of instructions (minus %packages and %pre/%post scripts).

If this is feasible, I'll open a new RFE.  The syntax could be something like
    <partitions type="additional">   <-- default if type is missing
    <partitions type="custom">       <-- must specify all including / and /boot

Comment 10 Dan Callaghan 2015-06-11 03:15:08 UTC
(In reply to Jeff Bastian from comment #9)
> I was trying to be "friendly" to uboot systems by adding this variable, but
> I think this actually is a deeper issue (which will need a new BZ): you can
> add additional partitions with the <partitions> tag in a recipe, but you
> cannot change or override the base partitions (/ and /boot) unless you use
> <kickstart> which requires the full kickstart set of instructions (minus
> %packages and %pre/%post scripts).

Yes that's a good point. We already have a very similar issue in bug 1198881, the latest idea there was to have a ksmeta "no_autopart" so that you can put custom partitioning commands in <ks_appends/>. Although I just remembered that won't work on older RHELs which lack the %end delimiter in kickstarts, hmm...

Comment 12 matt jia 2015-06-16 00:17:45 UTC
Verified steps:

1. create a job on an aarch64 system

2. check if kickstart creates /boot/efi partition

Comment 13 Dan Callaghan 2015-06-16 00:22:14 UTC
(In reply to matt jia from comment #12)
> 1. create a job on an aarch64 system

Specifically it has to use custom partitioning to trigger this bug, for example ks_meta="fstype=ext4".

Comment 17 Dan Callaghan 2015-07-14 08:04:42 UTC
Beaker 20.2 has been released.


Note You need to log in before you can comment on or make changes to this bug.