#2828 PR merged: Recognise -b/--bios options in format workflow

Labels: bug, fixed / solved / done

lzaoral opened issue at 2022-06-22 15:21:

Pull Request Details:

  • Type: Bug Fix

  • Impact: Normal

  • How was this pull request tested? Directly by invoking rear format -- -b/--bios <device>.

  • Brief description of the changes in this pull request:

Found while testing #2825.

According to rear format -- --help, this workflow should recognise both -b and --bios options. This PR fixes the following error message:

# rear format -- -b /dev/vdb
Use 'rear format -- --help' for more information.
rear format failed, check /var/log/rear/rear-<redacted>.log for details

jsmeix commented at 2022-06-23 08:44:

@lzaoral
thank you for testing it and for your fix!

It seems that was forgotten in
https://github.com/rear/rear/commit/9591fbf77c0c12329738625fcb83bb1d9b419b51
which links to
https://github.com/rear/rear/pull/2705

I guess it was not detected because the default hybrid boot
when neither --bios nor --efi is specified has worked so far.

jsmeix commented at 2022-06-23 08:44:

@rear/contributors
if there are no objections
I would like to merge it tomorrow afternoon.

lzaoral commented at 2022-06-23 08:50:

@jsmeix Actually, I tried the -b option because the efi or hybrid boot settings are broken on RHEL 7 at the moment. We are working on a fix with @pcahyna ASAP since RHEL 7 has older release of parted that does not support the esp keyword used here: https://github.com/rear/rear/blob/039b7b25566181bff8dd21648610e5625a33d036/usr/share/rear/format/USB/default/300_format_usb_disk.sh#L133

edit: better wording

jsmeix commented at 2022-06-23 10:10:

@lzaoral
if possible feel free to solve it by a simple fallback behaviour like

    # Set the right flag for the EFI partition:
    LogPrint "Setting 'esp' flag on EFI partition $RAW_USB_DEVICE$current_partition_number"
    if ! parted -s $RAW_USB_DEVICE set $current_partition_number esp on ; then
        LogPrintError "Failed to set 'esp' flag on EFI partition $RAW_USB_DEVICE$current_partition_number"
        # E.g. parted in RHEL 7 does not support to set the 'esp' flag so we try a fallback:
        if ! FALLBACK COMMAND ; then
            Error "..."
        fi
        LogPrintError "Did ... as fallback on EFI partition $RAW_USB_DEVICE$current_partition_number"
    fi

The LogPrintError() function should be used in general for
all important messages that should appear in the log file
and also on the user's terminal regardless whether or not
the user launched 'rear' in verbose mode, see
https://github.com/rear/rear/blob/master/usr/share/rear/lib/_input-output-functions.sh#L487

In particular when there was already a LogPrintError message
then subsequent messages that are related to that LogPrintError message
must also be shown to the user as LogPrintError messages
to ensure the user gets them on his terminal regardless
whether or not he launched 'rear' in verbose mode.

lzaoral commented at 2022-06-23 15:55:

@jsmeix, I'm sorry to be the bearer of bad news, but during testing of the fix for the esp keyword in parted on an EFI machine, I found out another regression :/.

The --efi option by default creates a msdos partition table unless I set USB_DEVICE_PARTED_LABEL=gpt in /etc/rear/local.confwhich was caused by 9591fbf77c0c12329738625fcb83bb1d9b419b51 and which is in clear contradiction with the documentation in usr/share/rear/conf/default.conf and is also a change in behaviour present in ReaR 2.6: https://github.com/rear/rear/blob/5e1ca54e119f016a366bbae6501ff48bec575e64/usr/share/rear/conf/default.conf#L971-L978

jsmeix commented at 2022-06-24 06:52:

@lzaoral
no need to be sorry.
I appreciate your news very much!
And actually it is good news because it is good to know bugs
before we release ReaR 2.7 so we can fix them which is good
for our users and also good for us because we get less issues
when many users use ReaR 2.7.

Could you please have a look at my
https://github.com/rear/rear/pull/2829
whether or not it makes things work for you
without the need to explictily specify the right
USB_DEVICE_PARTED_LABEL value in etc/rear/local.conf

jsmeix commented at 2022-06-24 13:20:

Because this pull request is merged I suggest to continue in
https://github.com/rear/rear/pull/2829
with what belongs to that subsequent issue.

jsmeix commented at 2022-06-24 13:22:

And for the other subsequent issue above
https://github.com/rear/rear/pull/2828#issuecomment-1164134153
please submit a new separated pull request.

[Export of Github issue for rear/rear.]