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.

Sign up for GitHub

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

Jump to bottom

bootloader: Simplify tuning of rpm-ostree kargs #690

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

bootloader: Simplify tuning of rpm-ostree kargs #690

wants to merge 1 commit into from

Conversation

zacikpa
Copy link
Contributor

@zacikpa zacikpa commented Sep 27, 2024

This PR aims to simplify the tuning of kernel parameters in rpm-ostree systems, which has some major flaws:

  1. It does not respect the claim that existing kernel parameters are left untouched.
  2. It does not append new kernel parameters in the order specified in the profile.

The new implementation is much simpler, but it also restricts the user more, since it disallows replacing existing kernel parameters.

Setting to WIP, since I'm still doing testing on Fedora CoreOS virtual machines.

@zacikpa zacikpa marked this pull request as ready for review September 30, 2024 09:09
@zacikpa zacikpa requested a review from yarda September 30, 2024 09:09
@yarda
Copy link
Contributor

yarda commented Dec 11, 2024

The new implementation is much simpler, but it also restricts the user more, since it disallows replacing existing kernel parameters.

There is an older grub convention that TuneD cannot change/delete any karg it didn't add itself, so I think this restriction is OK.

One question, what happen if there is running deployment or the rpm-ostree is busy. IMHO previously, it waited until it's idle. Is the request processed later or is there failure in such case?

@zacikpa
Copy link
Contributor Author

zacikpa commented Dec 11, 2024

One question, what happen if there is running deployment or the rpm-ostree is busy. IMHO previously, it waited until it's idle. Is the request processed later or is there failure in such case?

I will re-check that, can't remember if I tested such cases or not, thanks.

@yarda
Copy link
Contributor

yarda commented Jan 16, 2025

One question, what happen if there is running deployment or the rpm-ostree is busy. IMHO previously, it waited until it's idle. Is the request processed later or is there failure in such case?

I will re-check that, can't remember if I tested such cases or not, thanks.

Is the behaviour the same?

@zacikpa
Copy link
Contributor Author

zacikpa commented Jan 17, 2025 •
edited
Loading

One question, what happen if there is running deployment or the rpm-ostree is busy. IMHO previously, it waited until it's idle. Is the request processed later or is there failure in such case?

I will re-check that, can't remember if I tested such cases or not, thanks.

Is the behaviour the same?

Looking at it now, I likely factored out that code by accident. I will add it back.
Edit: Or I rather misunderstood what the code did. If there is a new active deployment, it's ok to proceed, but if there is a transaction in progress (like creating new deployment), then we should wait.

@zacikpa
bootloader: Simplify tuning of rpm-ostree kargs
879323d 
The bootloader plugin claims that it does not mess
with existing kernel parameters; make sure we adhere
to that also for rpm-ostree systems:

1. Only append a new karg if the same key=value pair
does not already appear in kernel parameters. If we would
duplicate it, it would not be possible to determine which
one to delete when unapplying the profile.

2. Do not delete existing key=value pairs when the profile
adds a karg with another value for the key. A single key can
appear multiple times in the kernel parameter list with different
values.

Also make sure new kargs are added exactly in the order in which
they appear in the profile. This is especially important for kargs
related to hugepages.

Resolves: RHEL-45836
@zacikpa
Copy link
Contributor Author

zacikpa commented Jan 31, 2025

Updated the code to handle errors better and re-added the wait until rpm-ostree becomes idle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yarda yarda Awaiting requested review from yarda

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zacikpa @yarda