#3025 PR `merged`: Fix create_grub2_cfg function usage¶

Labels: enhancement, bug, cleanup, fixed / solved / done

jsmeix opened issue at 2023-07-11 14:59:¶

Type: Bug Fix / Cleanup / Enhancement
Impact: Normal
Reference to related issue (URL):
https://github.com/rear/rear/issues/3024#issuecomment-1630963778
How was this pull request tested?
I did some basic tests, see
https://github.com/rear/rear/pull/3025#issuecomment-1634049686
and
https://github.com/rear/rear/pull/3025#issuecomment-1634118226
It even works reasonably well with Secure Boot for me, see
https://github.com/rear/rear/pull/3025#issuecomment-1635876186
I verified that OUTPUT=USB still works, see
https://github.com/rear/rear/pull/3025#issuecomment-1643774477
Brief description of the changes in this pull request:

Fix create_grub2_cfg function usage

jsmeix commented at 2023-07-11 15:07:¶

Found same kind of issue against "final power to the user"
in output/USB/Linux-i386/300_create_grub.sh
by hardcoded setting of GRUB2_SEARCH_ROOT_COMMAND
regardless what the user may have specified.

pcahyna commented at 2023-07-11 16:14:¶

The code being changed misses the intent of the code before #2662 . set root=cd0 and search --no-floppy --set=root --file /boot/efiboot.img should be both executed (set root=cd0 sets a sensible default that works in most cases and applies when search --no-floppy --set=root --file /boot/efiboot.img fails to do its job properly), but right now one executes only one of them ( with set root=cd0 taking precedence), which fails as soon as the DVD is not in the first DVD drive.

jsmeix commented at 2023-07-13 11:06:¶

With the current state of this pull request up to
https://github.com/rear/rear/pull/3025/commits/28fdbd8c7e1d5604ede1c7a73644e0e428c456ce
I works OK for my personal test case
(which does not mean much because I am not an expert in this area).

My original system is a KVM/QEMU VM
with OVMF "TianoCore" UEFI firmware:

# lsblk -ipo NAME,KNAME,PKNAME,TRAN,TYPE,FSTYPE,LABEL,SIZE,MOUNTPOINT
NAME        KNAME     PKNAME   TRAN   TYPE FSTYPE  LABEL                             SIZE MOUNTPOINT
/dev/sr0    /dev/sr0           sata   rom  iso9660 SLE-15-SP4-Full-x86_64151.11.001 12.1G 
/dev/vda    /dev/vda                  disk                                            15G 
|-/dev/vda1 /dev/vda1 /dev/vda        part vfat                                      512M /boot/efi
|-/dev/vda2 /dev/vda2 /dev/vda        part ext4                                     12.5G /
`-/dev/vda3 /dev/vda3 /dev/vda        part swap                                        2G [SWAP]

# grep -v '^$' etc/rear/local.conf 
FIRMWARE_FILES=( 'no' )
MODULES=( 'loaded_modules' )
PROGRESS_MODE="plain"
PROGRESS_WAIT_SECONDS="5"
SSH_ROOT_PASSWORD="rear"
USE_DHCLIENT="yes"
USE_SERIAL_CONSOLE="no"
OUTPUT=ISO
BACKUP=NETFS
BACKUP_OPTIONS="nfsvers=3,nolock"
BACKUP_URL=nfs://192.168.122.1/nfs

# usr/sbin/rear -D mkrescue
...
Found EFI system partition /dev/vda1 on /boot/efi type vfat
Using UEFI Boot Loader for Linux (USING_UEFI_BOOTLOADER=1)
Using '/usr/bin/xorrisofs' to create ISO filesystem images
Using autodetected kernel '/boot/vmlinuz-5.14.21-150400.22-default' as kernel in the recovery system
...
Trying to find what to use as UEFI bootloader...
Trying to find a 'well known file' to be used as UEFI bootloader...
Using '/boot/efi/EFI/sles/grubx64.efi' as UEFI bootloader file
...
Let GRUB2 load kernel /isolinux/kernel
Let GRUB2 load initrd /isolinux/initrd.cgz
Set GRUB2 default root device via 'set root=cd0'
Let GRUB2 search root device via 'search --no-floppy --set=root --file /boot/efiboot.img'
No serial console in GRUB2 (USE_SERIAL_CONSOLE is not true)
GRUB2 modules to load: ext2 fat part_gpt part_msdos
Making ISO image
Wrote ISO image: /root/rear/var/lib/rear/output/rear-localhost.iso (186M)
...

# mount -o loop /root/rear/var/lib/rear/output/rear-localhost.iso /tmp/iso

# mount
...
/root/rear/var/lib/rear/output/rear-localhost.iso on /tmp/iso type iso9660 (ro,relatime,nojoliet,check=s,map=n,blocksize=2048,iocharset=utf8)

# cat /tmp/iso/boot/grub/grub.cfg
set root=cd0
search --no-floppy --set=root --file /boot/efiboot.img
echo "Using root device $root from those available devices:"
ls
insmod all_video
set gfxpayload=keep
insmod part_gpt
insmod part_msdos
insmod ext2
echo 'No serial console (USE_SERIAL_CONSOLE was not true)'
set timeout="300"
set default="chainloader"
set fallback="chainloader"
echo 'Switching to GRUB boot menu...'
sleep --verbose --interruptible 30
menuentry "Relax-and-Recover (BIOS or UEFI without Secure Boot)" --id=rear {
    insmod gzio
    insmod xzio
    echo 'Loading kernel /isolinux/kernel ...'
    linux /isolinux/kernel root=UUID=7620c942-d8c0-45a8-9f8f-241e02f6f0c0  selinux=0
    echo 'Loading initial ramdisk /isolinux/initrd.cgz ...'
    initrd /isolinux/initrd.cgz
}
menuentry "Relax-and-Recover (UEFI and Secure Boot)" --id=rear_secure_boot {
    insmod gzio
    insmod xzio
    echo 'Loading kernel /isolinux/kernel ...'
    linuxefi /isolinux/kernel root=UUID=7620c942-d8c0-45a8-9f8f-241e02f6f0c0  selinux=0
    echo 'Loading initial ramdisk /isolinux/initrd.cgz ...'
    initrdefi /isolinux/initrd.cgz
}
menuentry "Boot next EFI" --id=chainloader {
    insmod chain
    search --fs-uuid --no-floppy --set=esp 8AD9-1260
    chainloader ($esp)/EFI/sles/grubx64.efi
}
menuentry "Reboot" --id=reboot {
    reboot
}
menuentry "Exit to EFI shell" --id=exit {
    exit
}

# find /tmp/iso/
/tmp/iso/
/tmp/iso/boot
/tmp/iso/boot/efiboot.img
/tmp/iso/boot/grub
/tmp/iso/boot/grub/grub.cfg
/tmp/iso/EFI
/tmp/iso/EFI/BOOT
/tmp/iso/EFI/BOOT/BOOTX64.efi
/tmp/iso/EFI/BOOT/fonts
/tmp/iso/EFI/BOOT/fonts/unicode.pf2
/tmp/iso/EFI/BOOT/grub.cfg
/tmp/iso/EFI/BOOT/locale
/tmp/iso/EFI/BOOT/locale/ast.mo
...
/tmp/iso/EFI/BOOT/locale/zh_TW.mo
/tmp/iso/isolinux
/tmp/iso/isolinux/boot.cat
/tmp/iso/isolinux/chain.c32
/tmp/iso/isolinux/hdt.c32
/tmp/iso/isolinux/initrd.cgz
/tmp/iso/isolinux/isolinux.bin
/tmp/iso/isolinux/isolinux.cfg
/tmp/iso/isolinux/kernel
/tmp/iso/isolinux/menu.c32
/tmp/iso/isolinux/message
/tmp/iso/isolinux/pci.ids
/tmp/iso/isolinux/poweroff.com
/tmp/iso/isolinux/rear.help
/tmp/iso/isolinux/reboot.c32
/tmp/iso/isolinux/vesamenu.c32

Excerpts from var/log/rear/rear-localhost.log

+ source /root/rear/usr/share/rear/output/ISO/Linux-i386/250_populate_efibootimg.sh
...
++ cp -v /boot/efi/EFI/sles/grubx64.efi /var/tmp/rear.SEIjjZqoarZFztm/tmp/mnt/EFI/BOOT/BOOTX64.efi
...
++ GRUB2_SET_ROOT_COMMAND='set root=cd0'
...
++ GRUB2_SEARCH_ROOT_COMMAND='search --no-floppy --set=root --file /boot/efiboot.img'
++ create_grub2_cfg /isolinux/kernel /isolinux/initrd.cgz
...
++ grub2-mkstandalone -v '--modules=ext2 fat part_gpt part_msdos' -O x86_64-efi -o /var/tmp/rear.SEIjjZqoarZFztm/tmp/mnt/EFI/BOOT/BOOTX64.efi /boot/grub/grub.cfg=/var/tmp/rear.SEIjjZqoarZFztm/tmp/mnt/EFI/BOOT/grub.cfg

Using the resulting ISO file in a virtual CDROM drive
on my replacement system which is another
same KVM/QEMU VM with OVMF "TianoCore" UEFI firmware
it boots the ReaR recovery system for me.

I only tried UEFI without secure boot.

Inside the ReaR recovery system:

RESCUE localhost:~ # lsblk -ipo NAME,KNAME,PKNAME,TRAN,TYPE,FSTYPE,LABEL,SIZE,MOUNTPOINT
NAME        KNAME     PKNAME   TRAN   TYPE FSTYPE  LABEL      SIZE MOUNTPOINT
/dev/sr0    /dev/sr0           sata   rom  iso9660 REAR-ISO 185.2M 
/dev/vda    /dev/vda                  disk                     15G 
|-/dev/vda1 /dev/vda1 /dev/vda        part vfat               512M 
|-/dev/vda2 /dev/vda2 /dev/vda        part ext4              12.5G 
`-/dev/vda3 /dev/vda3 /dev/vda        part swap                 2G

I also did a test with "rear mkbackup" and "rear recover"
which worked for me.

jsmeix commented at 2023-07-13 11:20:¶

Screenshot from 2023-07-13
13-11-20

What I personally like most is that now
GRUB2 shows which 'root' device it uses
and which devices are available.

Hopefully this avoids lengthy confusions as in
https://github.com/rear/rear/issues/3024
until finally (thanks to @pcahyna who found it out)
it became clear what the actual issue was, see
https://github.com/rear/rear/issues/3024#issuecomment-1630706633

jsmeix commented at 2023-07-13 11:59:¶

Tested how it behaves with two CDROM drives
on the replacement system using the same ISO as above
but now the ISO is in the second CDROM drive
(the first CDROM drive is empty):

Screenshot from 2023-07-13
14-28-58

GRUB2 now uses 'cd1' as 'root' device
and it boots well from it.

Inside the ReaR recovery system:

RESCUE localhost:~ # lsblk -ipo NAME,KNAME,PKNAME,TRAN,TYPE,FSTYPE,LABEL,SIZE,MOUNTPOINT
NAME        KNAME     PKNAME   TRAN   TYPE FSTYPE  LABEL      SIZE MOUNTPOINT
/dev/sr0    /dev/sr0           sata   rom                    1024M 
/dev/sr1    /dev/sr1           sata   rom  iso9660 REAR-ISO 185.2M 
/dev/vda    /dev/vda                  disk                     15G 
|-/dev/vda1 /dev/vda1 /dev/vda        part vfat               512M 
|-/dev/vda2 /dev/vda2 /dev/vda        part ext4              12.5G 
`-/dev/vda3 /dev/vda3 /dev/vda        part swap                 2G

Also rear recover worked well and
the recreated system boots normally.

jsmeix commented at 2023-07-14 06:22:¶

First things first:
I will not clean up the whole create_grub2_cfg() usage
via this pull request.
Via this pull request the create_grub2_cfg() usage
should only get fixed where it was actually wrong.
The implementation of the fix introduces the additional
GRUB2_SET_ROOT_COMMAND config variable to get consistency
regarding how GRUB2 sets and/or searches its 'root' device.
A general cleanup of the whole create_grub2_cfg() usage
should be done via a subsequent separated pull request.

jsmeix commented at 2023-07-14 13:35:¶

With my recent changes here up to
https://github.com/rear/rear/pull/3025/commits/b1d80997dcb083d632f2c8ec6db0e4cc306b41d9
it even works reasonably well with Secure Boot for me:

Same original system as above
but with Secure Boot enabled:

# grep -v '^$' etc/rear/local.conf 
FIRMWARE_FILES=( 'no' )
MODULES=( 'loaded_modules' )
PROGRESS_MODE="plain"
PROGRESS_WAIT_SECONDS="5"
SSH_ROOT_PASSWORD="rear"
USE_DHCLIENT="yes"
USE_SERIAL_CONSOLE="no"
SECURE_BOOT_BOOTLOADER="/boot/efi/EFI/sles/shim.efi"
OUTPUT=ISO
BACKUP=NETFS
BACKUP_OPTIONS="nfsvers=3,nolock"
BACKUP_URL=nfs://192.168.122.1/nfs

# usr/sbin/rear -D mkrescue
...
Found EFI system partition /dev/vda1 on /boot/efi type vfat
Using UEFI Boot Loader for Linux (USING_UEFI_BOOTLOADER=1)
Using '/usr/bin/xorrisofs' to create ISO filesystem images
...
Using '/boot/efi/EFI/sles/shim.efi' as UEFI Secure Boot bootloader file
...
Using Shim '/boot/efi/EFI/sles/shim.efi' as first stage UEFI bootloader BOOTX64.efi
Using second stage UEFI bootloader files for Shim: /boot/efi/EFI/sles/grub.efi /boot/efi/EFI/sles/grubx64.efi
Let GRUB2 load kernel /isolinux/kernel
Let GRUB2 load initrd /isolinux/initrd.cgz
Set GRUB2 default root device via 'set root=cd0'
Let GRUB2 search root device via 'search --no-floppy --set=root --file /boot/efiboot.img'
No serial console in GRUB2 (USE_SERIAL_CONSOLE is not true)
Making ISO image
Wrote ISO image: /root/rear/var/lib/rear/output/rear-localhost.iso (180M)
...

# mount -o loop /root/rear/var/lib/rear/output/rear-localhost.iso /tmp/iso
mount: /tmp/iso: WARNING: source write-protected, mounted read-only.

# mount /tmp/iso/boot/efiboot.img /tmp/fat

# find /tmp/iso /tmp/fat -ls | grep -v locale | grep '\.efi'
     1606    913 -rwx------   1 root     root        934680 Jul 14 14:52 /tmp/iso/EFI/BOOT/BOOTX64.efi
     1617   1206 -rwx------   1 root     root       1234944 Jul 14 14:52 /tmp/iso/EFI/BOOT/grub.efi
     1620    140 -rwx------   1 root     root        143360 Jul 14 14:52 /tmp/iso/EFI/BOOT/grubx64.efi
       85    914 -rwxr-xr-x   1 root     root        934680 Jul 14 14:52 /tmp/fat/EFI/BOOT/BOOTX64.efi
       86   1206 -rwxr-xr-x   1 root     root       1234944 Jul 14 14:52 /tmp/fat/EFI/BOOT/grub.efi
       87    140 -rwxr-xr-x   1 root     root        143360 Jul 14 14:52 /tmp/fat/EFI/BOOT/grubx64.efi

# ls -l /boot/efi/EFI/sles/shim.efi /boot/efi/EFI/sles/grub.efi /boot/efi/EFI/sles/grubx64.efi
-rwxr-xr-x 1 root root 1234944 Jul 12 15:12 /boot/efi/EFI/sles/grub.efi
-rwxr-xr-x 1 root root  143360 Jul 12 15:12 /boot/efi/EFI/sles/grubx64.efi
-rwxr-xr-x 1 root root  934680 Jul 12 15:12 /boot/efi/EFI/sles/shim.efi

I also did "rear mkbackup".

Same replacement system as above
but with Secure Boot enabled:

RESCUE localhost:~ # rear -D recover
...
Running 'finalize' stage ======================
...
Running mkinitrd...
Recreated initrd (/sbin/mkinitrd).
Creating EFI Boot Manager entries...
Creating  EFI Boot Manager entry 'SUSE_LINUX 15.4' for 'EFI\sles\shim.efi' (UEFI_BOOTLOADER='/boot/efi/EFI/sles/shim.efi') 
Installing secure boot loader (shim)...
...

When rebooting the recreated system
with the ReaR ISO still in its CDROM drive it booted from the ISO
but then via the default GRUB menu entry Boot next EFI
it loaded my SLES15 SP4 system but then that aborted to boot
with some "system is compromised" Secure Boot message
so I removed the ISO from the CDROM drive so that
it boots the recreated system directly from its harddisk
and that "just worked" for me (in Secure Boot mode).

jsmeix commented at 2023-07-17 13:33:¶

I did some generic cleanup of the create_grub2_cfg function usage
with my recent commits up to
https://github.com/rear/rear/pull/3025/commits/c5eb197aa6f3aa88cd2a98874e56bdb2277ee21b
in particular with
https://github.com/rear/rear/pull/3025/commits/e368c4200cce9bfb4bf60d0862497ff467282304
create_grub2_cfg() no longer accepts a third argument
(nowhere is create_grub2_cfg() called with a third argument)
that could be a "search GRUB2 'root' device command" string.
Now create_grub2_cfg() uses GRUB2_SET_ROOT_COMMAND and
GRUB2_SEARCH_ROOT_COMMAND directly as usual with other config variables.
Therefore the lower case variables grub2_set_root_command
and grub2_search_root_command are no longer needed, cf.
https://github.com/rear/rear/pull/3025#discussion_r1262376052

I still won't do a general cleanup via this pull request.

jsmeix commented at 2023-07-17 13:48:¶

@pcahyna @rear/contributors
as far as I can imagine currently I think I am now done
with fixing the create_grub2_cfg function usage
to the extent what I could reasonably do
within one single pull request.

If you like you may have a look at my current changes.
Perhaps you may spot some obvious mistakes.

Tomorrow I will redo my above tests with my latest changes.

Ideally - if all goes well - I would like to merge it
later this week (Thursday afternoon at the earliest).

jsmeix commented at 2023-07-18 08:35:¶

I did my above tests with my latest changes
and my tests worked again as before.

Two remarks:

Regarding
https://github.com/rear/rear/pull/3025#issuecomment-1635876186

When rebooting the recreated system
with the ReaR ISO still in its CDROM drive it booted from the ISO
but then via the default GRUB menu entry 'Boot next EFI'
it loaded my SLES15 SP4 system but then that aborted to boot
with some "system is compromised" Secure Boot message
so I removed the ISO from the CDROM drive so that
it boots the recreated system directly from its harddisk
and that "just worked" for me (in Secure Boot mode).

It also works to set in the OVMF "TianoCore" UEFI firmware
to use the harddisk as first boot device.

This time I made a screenshot how it looks when
it shows the "system is compromised" Secure Boot message:

Screenshot from 2023-07-18
10-19-06

jsmeix commented at 2023-07-18 08:45:¶

The second remark is about the
reinstalled bootloader during "rear recover":

What one gets after reboot after "rear recover"
are those two boot options

SLES 15-SP4
Advanced options for SLES 15-SP4

(see the above screenshot).

What is not there after "rear recover" is

UEFI Firmware Settings

which is normally there on SLES 15-SP4 which leads
into the OVMF "TianoCore" UEFI firmware setup menu.

To get the UEFI Firmware Settings entry
I did run the YaST bootloader module,
therein I changed an unimportant setting
(I changed in "bootloader options" the timeout)
so that YaST did actually reinstall the bootloader.
Now I get again the usual SLES 15-SP4 bootloader screen:

Screenshot from 2023-07-18
10-24-16

jsmeix commented at 2023-07-18 08:50:¶

@pcahyna
all looks OK now from my point of view with my tests.
I would much appreciate it if you could review my changes.

jsmeix commented at 2023-07-20 11:43:¶

I also tested

OUTPUT=USB
BACKUP=NETFS
BACKUP_URL=usb:///dev/disk/by-label/REAR-000
SECURE_BOOT_BOOTLOADER="/boot/efi/EFI/sles/shim.efi"
FIRMWARE_FILES=( 'no' )
MODULES=( 'loaded_modules' )
PROGRESS_MODE="plain"
PROGRESS_WAIT_SECONDS="5"
SSH_ROOT_PASSWORD="rear"
USE_DHCLIENT="yes"
USE_SERIAL_CONSOLE="no"

on my above original system (UEFI with Secure Boot)
with /dev/vdb as disk for "OUTPUT=USB"
and default hybrid formatting via

# usr/sbin/rear -D format /dev/vdb
...
Creating partition table of type gpt on /dev/vdb
Making a BIOS bootable device /dev/vdb
Creating BIOS boot partition /dev/vdb1
Setting 'bios_grub' flag on BIOS boot partition /dev/vdb1
Making an EFI bootable device /dev/vdb
Creating EFI system partition /dev/vdb2 with size 1024 MiB aligned at 8 MiB
Setting 'esp' flag on EFI partition /dev/vdb2
Creating ReaR data partition /dev/vdb3 up to 100% of /dev/vdb
Setting 'legacy_boot' flag on ReaR data partition /dev/vdb3
Creating vfat filesystem on EFI system partition on /dev/vdb2
Creating ext3 filesystem with label 'REAR-000' on ReaR data partition /dev/vdb3
Adjusting filesystem parameters on ReaR data partition /dev/vdb3

# lsblk -ipo NAME,TYPE,FSTYPE,LABEL,SIZE /dev/vdb
NAME        TYPE FSTYPE LABEL    SIZE
/dev/vdb    disk                   4G
|-/dev/vdb1 part                   8M
|-/dev/vdb2 part vfat   REAR-EFI   1G
`-/dev/vdb3 part ext3   REAR-000   3G

# parted -s /dev/vdb unit MiB print
Model: Virtio Block Device (virtblk)
Disk /dev/vdb: 4096MiB
Sector size (logical/physical): 512B/512B
Partition Table: gpt
Disk Flags: 
Number  Start    End      Size     File system  Name     Flags
 1      0.02MiB  8.00MiB  7.98MiB               primary  bios_grub
 2      8.00MiB  1032MiB  1024MiB  fat32        primary  boot, esp
 3      1032MiB  4095MiB  3063MiB  ext3         primary  legacy_boot

# usr/sbin/rear -D mkbackup
...
Found EFI system partition /dev/vda1 on /boot/efi type vfat
Using UEFI Boot Loader for Linux (USING_UEFI_BOOTLOADER=1)
...
Using '/boot/efi/EFI/sles/shim.efi' as UEFI Secure Boot bootloader file
...
Running 'output' stage ======================
Configuring EFI partition '/dev/disk/by-label/REAR-EFI' for EFI boot with 'shim.efi'
Configuring GRUB2 for EFI boot
Let GRUB2 load kernel /EFI/BOOT/kernel
Let GRUB2 load initrd /EFI/BOOT/initrd.cgz
Let GRUB2 search root device via 'search --no-floppy --set=root --label REAR-EFI'
No serial console in GRUB2 (USE_SERIAL_CONSOLE is not true)
GRUB2 modules to load: ext2 fat part_gpt part_msdos
...
Making /dev/vdb bootable with syslinux/extlinux
Writing syslinux MBR /usr/share/syslinux/gptmbr.bin of type gpt to /dev/vdb
Running 'backup' stage ======================
...

As far as I see Secure Boot of the ReaR recovery system
is not yet supported for OUTPUT=USB because it seems
for OUTPUT=USB it does not use Shim
(SECURE_BOOT_BOOTLOADER="/boot/efi/EFI/sles/shim.efi")
as first stage UEFI bootloader 'BOOTX64.efi'
and no second stage UEFI bootloader files for Shim like
/boot/efi/EFI/sles/grub.efi /boot/efi/EFI/sles/grubx64.efi
get copied into the directory where Shim (BOOTX64.efi) is
as done in output/ISO/Linux-i386/250_populate_efibootimg.sh
cf. https://github.com/rear/rear/pull/3025#issuecomment-1635876186

But UEFI without Secure Boot works for me.

So I had to switch off Secure Boot
on my replacement system (same as above)
to get the ReaR recovery system booted.

I did "rear recover" and rebooted to verify
that the recreated system works.

Then I shut down the replacement system and switched on again
Secure Boot in its OVMF TianoCore UEFI firmware setup menu
and booted it which also works, i.e. the recreated system
also boots with Secure Boot.

So "rear recover" did "the right thing" to also reinstall Shim,
cf. usr/share/rear/finalize/SUSE_LINUX/i386/675_install_shim.sh
in my case because I use SLES 15-SP4.
I didn't test if "rear recover" reinstalls Shim also
for other Linux distributions.

pcahyna commented at 2023-07-20 11:46:¶

@pcahyna all looks OK now from my point of view with my tests. I would much appreciate it if you could review my changes.

@jsmeix looking ...

pcahyna commented at 2023-07-20 15:21:¶

Looks good. The only potentially questionable part is the use of

contains_visible_char "$GRUB2_SET_ROOT_COMMAND"
contains_visible_char "$GRUB2_SEARCH_ROOT_COMMAND"

which, IIUC, prevents setting the variables to e.g. " " as a way to avoid using any set/search command at all.
As an example for another option, I have recently recommended the use of MKFS_XFS_OPTIONS=" " as a way to disable all XFS options (and thus using mkfs.xfs defaults), which is very useful in some situations.

jsmeix commented at 2023-07-21 11:33:¶

@pcahyna
thank you for your review!

Regarding questionable 'contains_visible_char' usage:

I introduced it because of
https://github.com/rear/rear/pull/3025#discussion_r1260972359

First I thought it would be against "final power to the user"
if the user could not specify to not use any set/search command.

But then I tested in etc/rear/local.conf

GRUB2_SET_ROOT_COMMAND="echo 'No set root command'"
GRUB2_SEARCH_ROOT_COMMAND="echo 'No search root command'"

which works for me to get the GRUB2 default 'memdisk'
but this cannot boot the recovery system
(because its kernel and initrd is not on 'memdisk').

Because it cannot work without any set/search command
I think the 'contains_visible_char' usage is OK here
to avoid that things don't work.

What is still questionable is if ReaR should
try to silently "correct" a user specified value
like "only blank characters" == "no characters",
cf. "sequence of zero bytes" == "end of file" in
https://github.com/rear/rear/pull/3027#issuecomment-1641940634
or
if ReaR should not try to "correct" user specified values?

jsmeix commented at 2023-07-21 11:34:¶

@rear/contributors
I will merge it in about one hour today
unless objections appear.

[Export of Github issue for rear/rear.]

#3025 PR merged: Fix create_grub2_cfg function usage¶