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,101 +222,6 @@ 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))
-		if rc != 0:
-			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]
-
-	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))
-		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)
-		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
-
 	def _get_effective_options(self, options):
 		"""Merge provided options with plugin default options and merge all cmdline.* options."""
 		effective = self._get_config_options().copy()
@@ -352,6 +256,58 @@ def _get_effective_options(self, options):
 			effective["cmdline"] = cmdline
 		return effective
 
+	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
+		return out
+
+	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:
+			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("Error deleting rpm-ostree kargs: %s" % err)
+
+	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)
+		return appended[1].split() if appended else []
+
+	def _rpm_ostree_update(self):
+		"""Apply kernel parameter tuning in a rpm-ostree system."""
+		if self._get_appended_rpm_ostree_kargs():
+			return
+		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
+		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 _remove_rpm_ostree_tuning(self):
+		"""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 _get_grub2_cfg_files(self):
 		cfg_files = []
 		for f in consts.GRUB2_CFG_FILES:
@@ -382,19 +338,6 @@ 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):
-		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
-
-	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: ""})
-
 	def _instance_unapply_static(self, instance, rollback = consts.ROLLBACK_SOFT):
 		if rollback == consts.ROLLBACK_FULL and not self._skip_grub_config_val:
 			if self._rpm_ostree:
@@ -502,16 +445,6 @@ def _grub2_cfg_patch(self, d):
 			self._grub2_default_env_patch()
 		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:
-			return
-		self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR : self._cmdline_val, consts.BOOT_CMDLINE_KARGS_DELETED_VAR : self._dict_to_options(d)})
-
 	def _grub2_update(self):
 		self._grub2_cfg_patch({consts.GRUB2_TUNED_VAR : self._cmdline_val, consts.GRUB2_TUNED_INITRD_VAR : self._initrd_val})
 		self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR : self._cmdline_val, consts.BOOT_CMDLINE_INITRD_ADD_VAR : self._initrd_val})
@@ -651,8 +584,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: