Bug 1556990

Summary: /boot/efi/EFI/redhat/grub.cfg does not get updated if /boot/grub2/grub.cfg exist on a uefi install.
Product: Red Hat Enterprise Linux 7 Reporter: Ryan Blakley <rblakley>
Component: tunedAssignee: Ondřej Lysoněk <olysonek>
Status: CLOSED ERRATA QA Contact: Dominik Rehák <drehak>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 7.4CC: aaron_wilk, drehak, jeder, jskarvad, olysonek, tcerna, thozza
Target Milestone: rcKeywords: Patch, Upstream
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: tuned-2.10.0-0.1.rc1.el7 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
: 1622646 (view as bug list) Environment:
Last Closed: 2018-10-30 10:50:19 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1622646    

Description Ryan Blakley 2018-03-15 17:10:44 UTC
Description of problem:When configuring a tuned profile to set cmdline parameters via the bootloader plugin on a uefi install. If for some reason the /boot/grub2/grub.cfg file exist, then the /boot/efi/EFI/redhat/grub.cfg does not get updated on a profile change, only the /boot/grub2/grub.cfg file gets updated. This is due to fact that the _get_grub2_cfg_file function loops through the GRUB2_CFG_FILES array and checks if one of the files exist, if it does it returns and doesn't continue. Since the /boot/grub2/grub.cfg file exist it doesn't continue on to check if both exist. It's very easy for someone to not recognize that the server is uefi, and accidentally run a grub2-mkconfig -o on the /etc/grub2.cfg symlink, which would cause this issue to happen.


Version-Release number of selected component (if applicable): tuned-2.8.0-5.el7_4.2


Steps to Reproduce:
1. On a uefi install setup a profile to set a cmdline parameter.
2. Touch /boot/grub2/grub.cfg
3. run # tuned-adm profile test-profile 


Actual results:
After running the above, you will see the /boot/efi/EFI/redhat/grub.cfg does not get updated. If a grub2-mkconfig -o /boot/grub2/grub.cfg is ran instead of the touch command above. Then you will see that on applying the profile it will update the /boot/grub2/grub.cfg file correctly.


Expected results:
Update only the /boot/efi/EFI/redhat/grub.cfg or update any of the files in the array if they exist just in case.


Additional info:
** I feel that an quick and easy fix would be to reorder the array entries, and place the efi path before the non efi path. 

** But I also threw together the below patch that changes the plugin to update all of the files in the GRUB2_CFG_FILES array if they exist. 

diff -up /usr/lib/python2.7/site-packages/tuned/plugins/plugin_bootloader.py.old /usr/lib/python2.7/site-packages/tuned/plugins/plugin_bootloader.py
--- /usr/lib/python2.7/site-packages/tuned/plugins/plugin_bootloader.py.old	2018-03-15 12:46:07.726685744 -0400
+++ /usr/lib/python2.7/site-packages/tuned/plugins/plugin_bootloader.py	2018-03-15 12:45:50.794161184 -0400
@@ -82,17 +82,22 @@ class BootloaderPlugin(base.Plugin):
 		return effective
 
 	def _get_grub2_cfg_file(self):
+		cfg_files = []
 		for f in consts.GRUB2_CFG_FILES:
 			if os.path.exists(f):
-				return f
-		return None
+				cfg_files.append(f)
+		if not cfg_files:
+			return None
+		else:
+			return cfg_files
 
 	def _patch_bootcmdline(self, d):
 		return self._cmd.add_modify_option_in_file(consts.BOOT_CMDLINE_FILE, d)
 
 	def _remove_grub2_tuning(self):
 		self._patch_bootcmdline({consts.BOOT_CMDLINE_TUNED_VAR : "", consts.BOOT_CMDLINE_INITRD_ADD_VAR : ""})
-		self._cmd.add_modify_option_in_file(self._grub2_cfg_file_name, {"set\s+" + consts.GRUB2_TUNED_VAR : "", "set\s+" + consts.GRUB2_TUNED_INITRD_VAR : ""}, add = False)
+		for f in self._grub2_cfg_file_name:
+			self._cmd.add_modify_option_in_file(f, {"set\s+" + consts.GRUB2_TUNED_VAR : "", "set\s+" + consts.GRUB2_TUNED_INITRD_VAR : ""}, add = False)
 		if self._initrd_dst_img_val is not None:
 			log.info("removing initrd image '%s'" % self._initrd_dst_img_val)
 			self._cmd.unlink(self._initrd_dst_img_val)
@@ -153,27 +158,28 @@ class BootloaderPlugin(base.Plugin):
 		if self._grub2_cfg_file_name is None:
 			log.error("cannot find grub.cfg to patch, you need to regenerate it by hand by grub2-mkconfig")
 			return False
-		grub2_cfg = self._cmd.read_file(self._grub2_cfg_file_name)
-		if len(grub2_cfg) <= 0:
-			log.error("error patching %s, you need to regenerate it by hand by grub2-mkconfig" % self._grub2_cfg_file_name)
-			return False
-		log.debug("adding boot command line parameters to '%s'" % self._grub2_cfg_file_name)
-		grub2_cfg_new = grub2_cfg
-		patch_initial = False
-		for opt in d:
-			(grub2_cfg_new, nsubs) = re.subn(r"\b(set\s+" + opt + "\s*=).*$", r"\1" + "\"" + d[opt] + "\"", grub2_cfg_new, flags = re.MULTILINE)
-			if nsubs < 1 or re.search(r"\$" + opt, grub2_cfg, flags = re.MULTILINE) is None:
-				patch_initial = True
-
-		# workaround for rhbz#1442117
-		if len(re.findall(r"\$" + consts.GRUB2_TUNED_VAR, grub2_cfg, flags = re.MULTILINE)) != \
-			len(re.findall(r"\$" + consts.GRUB2_TUNED_INITRD_VAR, grub2_cfg, flags = re.MULTILINE)):
-				patch_initial = True
-
-		if patch_initial:
-			grub2_cfg_new = self._grub2_cfg_patch_initial(self._grub2_cfg_unpatch(grub2_cfg), d)
-		self._cmd.write_to_file(self._grub2_cfg_file_name, grub2_cfg_new)
-		self._grub2_default_env_patch()
+		for f in self._grub2_cfg_file_name:
+			grub2_cfg = self._cmd.read_file(f)
+			if len(grub2_cfg) <= 0:
+				log.error("error patching %s, you need to regenerate it by hand by grub2-mkconfig" % f)
+				return False
+			log.debug("adding boot command line parameters to '%s'" % f)
+			grub2_cfg_new = grub2_cfg
+			patch_initial = False
+			for opt in d:
+				(grub2_cfg_new, nsubs) = re.subn(r"\b(set\s+" + opt + "\s*=).*$", r"\1" + "\"" + d[opt] + "\"", grub2_cfg_new, flags = re.MULTILINE)
+				if nsubs < 1 or re.search(r"\$" + opt, grub2_cfg, flags = re.MULTILINE) is None:
+					patch_initial = True
+
+			# workaround for rhbz#1442117
+			if len(re.findall(r"\$" + consts.GRUB2_TUNED_VAR, grub2_cfg, flags = re.MULTILINE)) != \
+				len(re.findall(r"\$" + consts.GRUB2_TUNED_INITRD_VAR, grub2_cfg, flags = re.MULTILINE)):
+					patch_initial = True
+
+			if patch_initial:
+				grub2_cfg_new = self._grub2_cfg_patch_initial(self._grub2_cfg_unpatch(grub2_cfg), d)
+			self._cmd.write_to_file(f, grub2_cfg_new)
+			self._grub2_default_env_patch()
 		return True
 
 	def _grub2_update(self):

Comment 2 Ondřej Lysoněk 2018-06-05 11:48:35 UTC
Thanks for the patch, Ryan. I fixed it up a bit and created an upstream pull request:
https://github.com/redhat-performance/tuned/pull/103

Comment 10 Jaroslav Škarvada 2018-08-28 08:54:33 UTC
Maybe we should check timestamps and use the latest one.

Comment 11 Jaroslav Škarvada 2018-08-28 08:57:35 UTC
(In reply to Jaroslav Škarvada from comment #10)
> Maybe we should check timestamps and use the latest one.

This will be addressed (somehow) in the bug 1622646.

Comment 13 Aaron 2018-08-30 17:33:58 UTC
Jaroslav, will the timestamp check in bug 1622646 be an additional check to the PR by Ryan that iterates thru all files in the GRUB2_CFG_FILES array? I noticed the PR was merged in.

https://github.com/redhat-performance/tuned/pull/103

Thanks,
Aaron

Comment 15 errata-xmlrpc 2018-10-30 10:50:19 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:3172