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

BLS entries consistency across architectures #1566

Open
dustymabe opened this issue Feb 1, 2024 · 7 comments
Open

BLS entries consistency across architectures #1566

dustymabe opened this issue Feb 1, 2024 · 7 comments

Comments

@dustymabe
Copy link
Contributor

In a recent PR where we are attempting to add CoreOS support to OSBuild for s390x we found the need to add some detection to the zip.inst stage for determining if /boot should be added to the file path for the discovered kernel and initrd from the BLS entries. This started a discussion about why this was needed.

The assumption I made up front is that all BLS config entries across all architectures would have a path for the kernel and initrd that was achored at the root of the filesystem for which the blscfg entries sat. i.e. if /boot is a separate filesystem then no /boot would be prepended to the entries. This makes sense because this is the only way GRUB would be able to find the kernel and initrd IIUC.

However s390x doesn't use GRUB and what we found (see the thread) is that the BLS config entries on s390x RHEL and Fedora images actually contained /boot in the path, even if there was a separate /boot filesystem:

# cat /mnt/loader/entries/ffffffffffffffffffffffffffffffff-6.6.8-200.fc39.s390x.conf
title  (6.6.8-200.fc39.s390x)
version 6.6.8-200.fc39.s390x
linux /boot/vmlinuz-6.6.8-200.fc39.s390x
initrd /boot/initramfs-6.6.8-200.fc39.s390x.img
options root=UUID=a4f79e2b-ce29-4e4f-b4b6-a9e4e66b07d0 ro no_timer_check console=ttyS0,115200n8 biosdevname=0 net.ifnames=0
id -20240131151940-6.6.8-200.fc39.s390x
grub_users $grub_users
grub_arg --unrestricted
grub_class kernel

This is actually by design (we think) as the script that creates this file on s390x hardcodes it:

mkbls() {
    local kernelver=$1 && shift
    local datetime=$1 && shift
    local kernelopts=$1 && shift

    local debugname=""
    local flavor=""

    if [[ "$kernelver" == *\+* ]] ; then
        local flavor=-"${kernelver##*+}"
        if [[ "${flavor}" == "-debug" ]]; then
            local debugname=" with debugging"
	    local debugid="-debug"
        fi
    fi

    cat <<EOF
title ${NAME} (${kernelver}) ${VERSION}${debugname}
version ${kernelver}${debugid}
linux /boot/vmlinuz-${kernelver}
initrd /boot/initramfs-${kernelver}.img
options ${kernelopts}
id ${ID}-${datetime}-${kernelver}${debugid}
grub_users \$grub_users
grub_arg --unrestricted
grub_class kernel${flavor}
EOF
}

I don't know if it is actually required for /boot/ to be prepended to the BLS entries on s390x ot not, but @achilleas-k and I did run a test where we modified the script to not have /boot on the front and did a kernel update and the system did at least update and reboot.

So. The reason we ended up here is because the BLS config entries on an s390x CoreOS machine actually don't have /boot prepended (presumably because OSTree is the one creating those entries and it does it consistently across all architectures).

Path forward:

  1. Long term: We should check with upstream to see if the hardcoded /boot in 20-zipl-kernel.install has meaning or not. If not maybe we should change it.
  2. Short term: @achilleas-k and I agreed we should modify the zipl.inst stage to handle either case. i.e. if it can find the file without boot in the file path then use it. If not then try with boot and use that if found, else error.
@cgwalters
Copy link
Contributor

This might relate to ostreedev/ostree@10e465c which we should probably turn on by default now (i did in the c9s-bootc images)

@dustymabe
Copy link
Contributor Author

This might relate to ostreedev/ostree@10e465c which we should probably turn on by default now (i did in the c9s-bootc images)

Hmm. I think there is something I am missing and you can help me understand. If we set sysroot.bootprefix to true (or OSTree changes it to default to true) then on systems with separate /boot/ filesystems that use GRUB, won't GRUB not be able to find the kernel/initrd? i.e. the path will be incorrect?

@cgwalters
Copy link
Contributor

won't GRUB not be able to find the kernel/initrd? i.e. the path will be incorrect?

ostreedev/ostree@0ced9fd

@dustymabe
Copy link
Contributor Author

dustymabe commented Feb 1, 2024

won't GRUB not be able to find the kernel/initrd? i.e. the path will be incorrect?

ostreedev/ostree@0ced9fd

Saving future me some time..

The relevant part here is that commit changes the deploy code to create a symlink in the /boot directory that points back to itself.

/boot/boot -> .

so effectively /boot/ and /boot/boot point to the same place.

so /boot/{path-to-kernel} will always work regardless of if the root of your filesystem is just the boot filesystem or the full root filesystem with boot mounted at /boot.

@dustymabe
Copy link
Contributor Author

dustymabe commented Feb 1, 2024

This means an alernative option (specifically for the CoreOS building with OSBuild efforts) is to set sysroot.bootprefix to true in our config and then the zipl.inst stage would be able to continue without issue.

@achilleas-k
Copy link
Member

This means an alernative option (specifically for the CoreOS building with OSBuild efforts) is to set sysroot.bootprefix to true in our config and then the zipl.inst stage would be able to continue without issue.

This sounds simple enough (is it?). Let me know what you decide and if we should change the zipl.inst stage.

@dustymabe
Copy link
Contributor Author

This means an alernative option (specifically for the CoreOS building with OSBuild efforts) is to set sysroot.bootprefix to true in our config and then the zipl.inst stage would be able to continue without issue.

This sounds simple enough (is it?). Let me know what you decide and if we should change the zipl.inst stage.

Yes. It should be simple. For now we have dropped the ("fix the prefix") commit from #1558

Once that merges we'll make the other changes on our side and report back here to continue the discussion.

dustymabe added a commit to dustymabe/coreos-assembler that referenced this issue Feb 6, 2024
This setting will make it so that BLS config entries get prepended
with /boot. OSTree already places a boot -> . symlink in the root
of the boot filesystem prepending with /boot will always just work.

For context see osbuild/osbuild#1566 (comment)

This also allows for dropping one of the upstream OSBuild zipl stage
patches.
dustymabe added a commit to dustymabe/osbuild that referenced this issue Feb 6, 2024
dustymabe added a commit to dustymabe/coreos-assembler that referenced this issue Feb 6, 2024
This setting will make it so that BLS config entries get prepended
with /boot. OSTree already places a boot -> . symlink in the root
of the boot filesystem prepending with /boot will always just work.

For context see osbuild/osbuild#1566 (comment)

This also allows for dropping one of the upstream OSBuild zipl stage
patches.
dustymabe added a commit to dustymabe/coreos-assembler that referenced this issue Feb 6, 2024
This setting will make it so that BLS config entries get prepended
with /boot. OSTree already places a boot -> . symlink in the root
of the boot filesystem prepending with /boot will always just work.

For context see osbuild/osbuild#1566 (comment)

This also allows for dropping one of the upstream OSBuild zipl stage
patches.
dustymabe added a commit to coreos/coreos-assembler that referenced this issue Feb 6, 2024
This setting will make it so that BLS config entries get prepended
with /boot. OSTree already places a boot -> . symlink in the root
of the boot filesystem prepending with /boot will always just work.

For context see osbuild/osbuild#1566 (comment)

This also allows for dropping one of the upstream OSBuild zipl stage
patches.
dustymabe added a commit to dustymabe/coreos-assembler that referenced this issue Feb 23, 2024
This is a second round of f5677a3 (reverted in db7bf08).

Now that coreos-installer in rawhide has
coreos/coreos-installer#1422
we can set sysroot.bootprefix to true again.

We can't do it in the non-OSBuild configs because not all streams
have the new coreos-installer yet.

Here is the comment from the original commit:

This setting will make it so that BLS config entries get prepended
with /boot. OSTree already places a boot -> . symlink in the root
of the boot filesystem prepending with /boot will always just work.

For context see osbuild/osbuild#1566 (comment)

This also allows for dropping one of the upstream OSBuild zipl stage
patches.
dustymabe added a commit to coreos/coreos-assembler that referenced this issue Feb 26, 2024
This is a second round of f5677a3 (reverted in db7bf08).

Now that coreos-installer in rawhide has
coreos/coreos-installer#1422
we can set sysroot.bootprefix to true again.

We can't do it in the non-OSBuild configs because not all streams
have the new coreos-installer yet.

Here is the comment from the original commit:

This setting will make it so that BLS config entries get prepended
with /boot. OSTree already places a boot -> . symlink in the root
of the boot filesystem prepending with /boot will always just work.

For context see osbuild/osbuild#1566 (comment)

This also allows for dropping one of the upstream OSBuild zipl stage
patches.
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

No branches or pull requests

3 participants