Bug 1614637

This bug appears to have been reported against 'rawhide' during the Fedora 29 development cycle.
Changing version to '29'.

*** Bug 1614756 has been marked as a duplicate of this bug. ***

(In reply to Petr Pisar from comment #0)
> # grub2-mkconfig >/boot/grub2/grub.cfg 
> Generating grub configuration file ...
> /etc/grub.d/01_fallback_counting: line 4: [: missing `]'
> Found linux image: /boot/vmlinuz-4.18.0-0.rc8.git1.1.fc29.x86_64
> Found initrd image: /boot/initramfs-4.18.0-0.rc8.git1.1.fc29.x86_64.img
> Found linux image: /boot/vmlinuz-4.18.0-0.rc8.git0.1.fc29.x86_64
> Found initrd image: /boot/initramfs-4.18.0-0.rc8.git0.1.fc29.x86_64.img
> Found linux image: /boot/vmlinuz-4.18.0-0.rc7.git1.1.fc29.x86_64
> Found initrd image: /boot/initramfs-4.18.0-0.rc7.git1.1.fc29.x86_64.img
> done
> 
> # rpm -qf /etc/grub.d/01_fallback_counting
> grub2-tools-2.02-49.fc29.x86_64
> 
> # cat -n /etc/grub.d/01_fallback_counting
>      1  #!/usr/bin/sh -e
>      2  
>      3  # Boot Counting
> →    4  if [ "\${boot_counter}" -a "\${boot_success}" = "0"]; then
>      5    if  [ "\${boot_counter}" = "0" -o "\${boot_counter}" = "-1" ]; then
>      6      set default=1
>      7      set boot_counter=-1
>      8    else
>      9      set boot_counter=$((\${boot_counter}-1))
>     10    fi
>     11    save_env boot_counter
>     12  fi
> 
> You are missing a space before the closing bracket, thus you compare
> boot_success variable against '0]' string.
> 
> BTW, escaping the dollar sigils looks suspicious. I think it does not do
> what you want.


I suppose this was supposed to create a grub.cfg snippet.  In that case it is missing a "cat << EOF" line and the corresponding "EOF" line.  As a shell script 
this does not make much sense.

grub2-2.02-60.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2018-d4d38d8885

(In reply to Fedora Update System from comment #4)
> grub2-2.02-60.fc29 has been submitted as an update to Fedora 29.
> https://bodhi.fedoraproject.org/updates/FEDORA-2018-d4d38d8885

Close; but no cigar:


[root@secondbux ~]# grub2-mkconfig -o /boot/grub2/grub.cfg
Generating grub configuration file ...
/etc/grub.d/01_fallback_counting: line 4: ${boot_counter}-1: syntax error: operand expected (error token is "${boot_counter}-1")
[root@secondbux ~]#

would that be fixed by another parenthesis around \${boot_counter}?

grub2-2.02-60.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-d4d38d8885

(In reply to Christian Glombek from comment #6)
> would that be fixed by another parenthesis around \${boot_counter}?

The fix is:


diff --git a/util/grub.d/01_fallback_counting.in b/util/grub.d/01_fallback_counting.in
index afe06199a..31e898188 100644
--- a/util/grub.d/01_fallback_counting.in
+++ b/util/grub.d/01_fallback_counting.in
@@ -7,8 +7,8 @@ if [ "\${boot_counter}" -a "\${boot_success}" = "0" ]; then
     set default=1
     set boot_counter=-1
   else
-    set boot_counter=$((\${boot_counter}-1))
+    set boot_counter=\$((\${boot_counter}-1))
   fi
   save_env boot_counter
 fi
-EOF
\ No newline at end of file
+EOF

This appears to break initial install. Some netinst tests failed in the Fedora-29-20181002.n.0 compose openQA testing, like this one:

https://openqa.fedoraproject.org/tests/287990

They failed with bootloader install error, program.log shows:

    01:42:30,298 INF program: Generating grub configuration file ...
    01:42:30,299 INF program: /etc/grub.d/01_fallback_counting: line 4: ${boot_counter}-1: syntax error: operand expected (error token is "${boot_counter}-1")
    01:42:30,300 DBG program: Return code: 1

Those tests seem to have installed this version of grub2 (as it was in updates-testing at the time they ran). In comparison, DVD install tests that deployed the previous build (-59) worked fine.

oh, that's the same bug villy saw, now I look closer at the previous comments. So, consider this confirmation!

grub2-2.02-61.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2018-d4d38d8885

Still broken.  Just now it is a grub syntax issue:

Script started on 2018-10-04 09:20:46+02:00
[root@secondbux ~]# grub2-mkconfig -o /boot/grub2/grub.cfg
Generating grub configuration file ...
Found linux image: /boot/vmlinuz-4.18.11-301.fc29.x86_64
Found initrd image: /boot/initramfs-4.18.11-301.fc29.x86_64.img
Found linux image: /boot/vmlinuz-4.18.11-300.fc29.x86_64
Found initrd image: /boot/initramfs-4.18.11-300.fc29.x86_64.img
Found linux image: /boot/vmlinuz-4.18.10-300.fc29.x86_64
Found initrd image: /boot/initramfs-4.18.10-300.fc29.x86_64.img
Found linux image: /boot/vmlinuz-0-rescue-9b6f90dd0fd94117aedaf2cc29d11cf5
Found initrd image: /boot/initramfs-0-rescue-9b6f90dd0fd94117aedaf2cc29d11cf5.img
error: ../grub-core/script/lexer.c:337:$.
error: ../grub-core/script/lexer.c:337:syntax error.
error: ../grub-core/script/lexer.c:337:Incorrect command.
error: ../grub-core/script/lexer.c:337:syntax error.
Syntax error at line 79
Syntax errors are detected in generated GRUB config file.
Ensure that there are no errors in /etc/default/grub
and /etc/grub.d/* files or please file a bug report with
/boot/grub2/grub.cfg.new file attached.
[root@secondbux ~]# exit
exit

Script done on 2018-10-04 09:21:27+02:00

Apparently grub does not like the "$((${boot_counter}-1))" construction.

At this point I'm just guessing, but maybe \$((\${boot_counter})-1) will work?

Nope. that does not help either

    set boot_counter=\$((\${boot_counter})-1)

$ sudo grub2-mkconfig -o /etc/grub2-efi.cfg
Generating grub configuration file ...
Found linux image: /boot/vmlinuz-4.18.11-301.fc29.x86_64
Found initrd image: /boot/initramfs-4.18.11-301.fc29.x86_64.img
Found linux image: /boot/vmlinuz-4.18.10-300.fc29.x86_64
Found initrd image: /boot/initramfs-4.18.10-300.fc29.x86_64.img
Found linux image: /boot/vmlinuz-4.18.10-200.fc28.x86_64
Found initrd image: /boot/initramfs-4.18.10-200.fc28.x86_64.img
Found linux image: /boot/vmlinuz-0-rescue-ee12a31aa29d41d6b9fd3d51df039fb2
Found initrd image: /boot/initramfs-0-rescue-ee12a31aa29d41d6b9fd3d51df039fb2.img
Adding boot menu entry for EFI firmware configuration
error: ../grub-core/script/lexer.c:337:$.
error: ../grub-core/script/lexer.c:337:syntax error.
error: ../grub-core/script/lexer.c:337:Incorrect command.
error: ../grub-core/script/lexer.c:337:syntax error.
Syntax error at line 84
Syntax errors are detected in generated GRUB config file.
Ensure that there are no errors in /etc/default/grub
and /etc/grub.d/* files or please file a bug report with
/etc/grub2-efi.cfg.new file attached.

In the grub documentation, there is not mention of arithmetic expression, so it may not be possible to take the value of boot_counter and increment it from within grub itself.  I must admit I don't understand the real purpose of this piece of code, but I suspect that someone must reconsider the solution strategy, as this looks like a dead end.

The purpose of this is to count down failing boot attempts.

If arithmetic ops aren't supported, maybe the right way to do this is to add a decrement function, analogous to https://github.com/rhboot/grub2/blob/fedora-30/util/grub-editenv.c#L270-L295 ?

grub2-2.02-61.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-d4d38d8885

Created attachment 1490686 [details]
Patch to clean this up.

(In reply to Christian Glombek from comment #16)
> The purpose of this is to count down failing boot attempts.
> 
> If arithmetic ops aren't supported, maybe the right way to do this is to add
> a decrement function, analogous to
> https://github.com/rhboot/grub2/blob/fedora-30/util/grub-editenv.c#L270-L295
> ?

That's probably the right idea; there's also a logic problem there where we're testing if it's -1 inside a test that it's 0.  Patch attached that should fix both issues.

I also made it increment "default" instead of setting it to 1, so that on the case where you've installed 2 kernels in a row that don't work, but never tried them, it will still (eventually) converge on the last working kernel.

(note that the patch as posted /replaces/ the existing patches.)

I still don't understand how this is actually getting used, though - what increments boot_counter?  What resets it when we get a new kernel?

pjones: there's some explanation on what he's trying to do here:

https://medium.com/@c.glombek/fedora-iot-gsoc-2018-final-report-c7a048dc8e80

It seems to be basically about automatically rolling back ostree upgrades on failed boot. Poking about in the links from that, the thing called 'greenboot' appears to be what's going to set the counter:

https://github.com/LorbusChris/greenboot/search?q=boot_counter&unscoped_q=boot_counter

The package for it recently passed review:

https://bugzilla.redhat.com/show_bug.cgi?id=1615895

grub2-2.02-62.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2018-d4d38d8885

Alright, so that seems to mean grub2-mkconfig needs to test that we're actually using that, or else we're just going to record a failure every time.  Is "systemctl -q is-enabled greenboot.service" sufficient to tell that?

Perhaps this script itself could be moved to that greenboot package, too?

(In reply to Peter Jones from comment #18)
> That's probably the right idea; there's also a logic problem there where
> we're testing if it's -1 inside a test that it's 0.  Patch attached that
> should fix both issues.
> 
> I also made it increment "default" instead of setting it to 1, so that on
> the case where you've installed 2 kernels in a row that don't work, but
> never tried them, it will still (eventually) converge on the last working
> kernel.

The logic was sound, it was checking for boot_counter=-1 inside a test for boot_success=0, not boot_counter=0.

Always setting default=1 is in that case also desired, as this is intended for ostree-based systems to choose the rollback deployment which is adressed this way.

boot_counter, same as boot_success which was introduced with menu auto-hide, need to be re-set from userspace.
The (now removed) test [ "\${boot_counter}" -a "\${boot_success}" = "0"] is supposed to be enough of an indicator for grub to activate this behavior. I don't think a test for that in mkconfig is necessary for this.

This patch changes the logic in a way not intended; there was actually a need for the nested logic, please revert this.

Just to make things clear: The boot_counter is a count down, and is re-set in userspace after installing an upgrade and before rebooting.

grub2-2.02-62.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-d4d38d8885

*** Bug 1628663 has been marked as a duplicate of this bug. ***

grub2-2.02-62.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.

this is still not fixed, new PR in https://github.com/rhboot/grub2/pull/42

The needinfo request[s] on this closed bug have been removed as they have been unresolved for 1000 days