From f9f5e9e70d994a36f81c1b9d251088367d776eda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavol=20=C5=BD=C3=A1=C4=8Dik?= Date: Mon, 30 Sep 2024 10:55:31 +0200 Subject: [PATCH] bootloader: Simplify tuning of rpm-ostree kargs 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 --- tuned/consts.py | 1 - tuned/plugins/plugin_bootloader.py | 154 +++++++++-------------------- 2 files changed, 45 insertions(+), 110 deletions(-) diff --git a/tuned/consts.py b/tuned/consts.py index 5134684c..2f7f9b50 100644 --- a/tuned/consts.py +++ b/tuned/consts.py @@ -38,7 +38,6 @@ INITRD_IMAGE_DIR = "/boot" BOOT_CMDLINE_TUNED_VAR = "TUNED_BOOT_CMDLINE" BOOT_CMDLINE_INITRD_ADD_VAR = "TUNED_BOOT_INITRD_ADD" -BOOT_CMDLINE_KARGS_DELETED_VAR = "TUNED_BOOT_KARGS_DELETED" BOOT_CMDLINE_FILE = "/etc/tuned/bootcmdline" PETITBOOT_DETECT_DIR = "/sys/firmware/opal" MACHINE_ID_FILE = "/etc/machine-id" diff --git a/tuned/plugins/plugin_bootloader.py b/tuned/plugins/plugin_bootloader.py index 8a7621fb..550b7f66 100644 --- a/tuned/plugins/plugin_bootloader.py +++ b/tuned/plugins/plugin_bootloader.py @@ -8,7 +8,7 @@ import os import re import tempfile -from time import sleep +import subprocess log = tuned.logs.get() @@ -205,8 +205,7 @@ def _instance_init(self, instance): self._initrd_val = "" self._grub2_cfg_file_names = self._get_grub2_cfg_files() self._bls = self._bls_enabled() - - self._rpm_ostree = self._rpm_ostree_status() is not None + self._rpm_ostree = self._is_rpm_ostree_system() def _instance_cleanup(self, instance): pass @@ -223,100 +222,35 @@ def _get_config_options(cls): "skip_grub_config": None, } - @staticmethod - def _options_to_dict(options, omit=""): - """ - Returns dict created from options - e.g.: _options_to_dict("A=A A=B A B=A C=A", "A=B B=A B=B") returns {'A': ['A', None], 'C': ['A']} - """ - d = {} - omit = omit.split() - for o in options.split(): - if o not in omit: - arr = o.split('=', 1) - d.setdefault(arr[0], []).append(arr[1] if len(arr) > 1 else None) - return d - - @staticmethod - def _dict_to_options(d): - return " ".join([k + "=" + v1 if v1 is not None else k for k, v in d.items() for v1 in v]) - - def _rpm_ostree_status(self): - """ - Returns status of rpm-ostree transactions or None if not run on rpm-ostree system - """ - (rc, out, err) = self._cmd.execute(['rpm-ostree', 'status'], return_err=True) - log.debug("rpm-ostree status output stdout:\n%s\nstderr:\n%s" % (out, err)) + def _is_rpm_ostree_system(self): + """Check if the current system uses rpm-ostree.""" + try: + subprocess.check_call(["rpm-ostree", "status"]) + except subprocess.CalledProcessError: + return False + return True + + def _get_rpm_ostree_kargs(self): + """Retrieve the output of rpm-ostree kargs, i.e., current default kernel arguments.""" + (rc, out, err) = self._cmd.execute(["rpm-ostree", "kargs"], return_err=True) + if out: + log.debug("rpm-ostree kargs: %s" % out) if rc != 0: + log.error("Error getting rpm-ostree kargs: %s" % err) return None - splited = out.split() - if len(splited) < 2 or splited[0] != "State:": - log.warning("Exceptional format of rpm-ostree status result:\n%s" % out) - return None - return splited[1] + return out - def _wait_till_idle(self): - sleep_cycles = 10 - sleep_secs = 1.0 - for i in range(sleep_cycles): - if self._rpm_ostree_status() == "idle": - return True - sleep(sleep_secs) - if self._rpm_ostree_status() == "idle": - return True - return False - - def _rpm_ostree_kargs(self, append={}, delete={}): - """ - Method for appending or deleting rpm-ostree karg - returns None if rpm-ostree not present or is run on not ostree system - or tuple with new kargs, appended kargs and deleted kargs - """ - (rc, out, err) = self._cmd.execute(['rpm-ostree', 'kargs'], return_err=True) - log.debug("rpm-ostree output stdout:\n%s\nstderr:\n%s" % (out, err)) + def _append_rpm_ostree_kargs(self, kargs): + """Append a list of kernel arguments (in a rpm-ostree system).""" + (rc, _, err) = self._cmd.execute(["rpm-ostree", "kargs"] + ["--append=%s" % karg for karg in kargs], return_err=True) if rc != 0: - return None, None, None - kargs = self._options_to_dict(out) - - if not self._wait_till_idle(): - log.error("Cannot wait for transaction end") - return None, None, None - - deleted = {} - delete_params = self._dict_to_options(delete).split() - # Deleting kargs, e.g. deleting added kargs by profile - for k, val in delete.items(): - for v in val: - kargs[k].remove(v) - deleted[k] = val - - appended = {} - append_params = self._dict_to_options(append).split() - # Appending kargs, e.g. new kargs by profile or restoring kargs replaced by profile - for k, val in append.items(): - if kargs.get(k): - # If there is karg that we add with new value we want to delete it - # and store old value for restoring after profile unload - log.debug("adding rpm-ostree kargs %s: %s for delete" % (k, kargs[k])) - deleted.setdefault(k, []).extend(kargs[k]) - delete_params.extend([k + "=" + v if v is not None else k for v in kargs[k]]) - kargs[k] = [] - kargs.setdefault(k, []).extend(val) - appended[k] = val - - if append_params == delete_params: - log.info("skipping rpm-ostree kargs - append == deleting (%s)" % append_params) - return kargs, appended, deleted - - log.info("rpm-ostree kargs - appending: '%s'; deleting: '%s'" % (append_params, delete_params)) - (rc, _, err) = self._cmd.execute(['rpm-ostree', 'kargs'] + - ['--append=%s' % v for v in append_params] + - ['--delete=%s' % v for v in delete_params], return_err=True) + log.error("Error appending rpm-ostree kargs: %s" % err) + + def _delete_rpm_ostree_kargs(self, kargs): + """Delete a list of kernel arguments (in a rpm-ostree system).""" + (rc, _, err) = self._cmd.execute(["rpm-ostree", "kargs"] + ["--delete=%s" % karg for karg in kargs], return_err=True) if rc != 0: - log.error("Something went wrong with rpm-ostree kargs\n%s" % (err)) - return self._options_to_dict(out), None, None - else: - return kargs, appended, deleted + log.error("Error deleting rpm-ostree kargs: %s" % err) def _get_effective_options(self, options): """Merge provided options with plugin default options and merge all cmdline.* options.""" @@ -382,18 +316,16 @@ def _remove_grub2_tuning(self): log.info("removing initrd image '%s'" % self._initrd_dst_img_val) self._cmd.unlink(self._initrd_dst_img_val) - def _get_rpm_ostree_changes(self): + def _get_appended_rpm_ostree_kargs(self): + """Return the list of kernel arguments that were appended by this profile (in a rpm-ostree system).""" f = self._cmd.read_file(consts.BOOT_CMDLINE_FILE) appended = re.search(consts.BOOT_CMDLINE_TUNED_VAR + r"=\"(.*)\"", f, flags=re.MULTILINE) - appended = appended[1] if appended else "" - deleted = re.search(consts.BOOT_CMDLINE_KARGS_DELETED_VAR + r"=\"(.*)\"", f, flags=re.MULTILINE) - deleted = deleted[1] if deleted else "" - return appended, deleted + return appended[1].split() if appended else [] def _remove_rpm_ostree_tuning(self): - appended, deleted = self._get_rpm_ostree_changes() - self._rpm_ostree_kargs(append=self._options_to_dict(deleted), delete=self._options_to_dict(appended)) - self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR: "", consts.BOOT_CMDLINE_KARGS_DELETED_VAR: ""}) + """Remove kernel parameter tuning in a rpm-ostree system.""" + self._delete_rpm_ostree_kargs(self._get_appended_rpm_ostree_kargs()) + self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR: ""}) def _instance_unapply_static(self, instance, rollback = consts.ROLLBACK_SOFT): if rollback == consts.ROLLBACK_FULL and not self._skip_grub_config_val: @@ -503,14 +435,19 @@ def _grub2_cfg_patch(self, d): return True def _rpm_ostree_update(self): - appended, _ = self._get_rpm_ostree_changes() - _cmdline_dict = self._options_to_dict(self._cmdline_val, appended) - if not _cmdline_dict: - return None - (_, _, d) = self._rpm_ostree_kargs(append=_cmdline_dict) - if d is None: + """Apply kernel parameter tuning in a rpm-ostree system.""" + if self._get_appended_rpm_ostree_kargs(): + # The kargs are already set in /etc/tuned/bootcmldine, + # we are likely post-reboot and done. return - self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR : self._cmdline_val, consts.BOOT_CMDLINE_KARGS_DELETED_VAR : self._dict_to_options(d)}) + profile_kargs = self._cmdline_val.split() + active_kargs = self._get_rpm_ostree_kargs().split() + # Only append key=value pairs that do not yet appear in kernel parameters, + # otherwise we would not be able to restore the cmdline to the previous state + # via rpm-ostree kargs --delete. + kargs_to_append = [karg for karg in profile_kargs if karg not in active_kargs] + self._append_rpm_ostree_kargs(kargs_to_append) + self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR : " ".join(kargs_to_append)}) def _grub2_update(self): self._grub2_cfg_patch({consts.GRUB2_TUNED_VAR : self._cmdline_val, consts.GRUB2_TUNED_INITRD_VAR : self._initrd_val}) @@ -651,8 +588,7 @@ def _cmdline(self, enabling, value, verify, ignore_missing): v = self._variables.expand(self._cmd.unquote(value)) if verify: if self._rpm_ostree: - rpm_ostree_kargs = self._rpm_ostree_kargs()[0] - cmdline = self._dict_to_options(rpm_ostree_kargs) + cmdline = self._get_rpm_ostree_kargs() else: cmdline = self._cmd.read_file("/proc/cmdline") if len(cmdline) == 0: