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

@@ -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"

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: