Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include dmsetup and dmeventd unconditionally #2748

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

lzaoral
Copy link
Contributor

@lzaoral lzaoral commented Jan 25, 2022

Pull Request Details:
  • Type: Bug Fix

  • Impact: High

  • Reference to related issue (URL): None

  • How was this pull request tested?

Successful recovery of a RHEL 8.5 VM with BIOS without encryption, LVM
or multipath with this patch applied and unsuccessful recovery of the same
machine without it. RHEL 8.5 has os-prober 1.74 and does not ship
grub-mount binary.

  • Brief description of the changes in this pull request:

Changes usr/share/rear/conf/GNU/Linux.conf

Older releases of os-prober (1.74 and below) use dmsetup as a fallback
solution for mounting when grub-mount is missing.

However, dmsetup was included in the rescue image if and only if LVM,
multipath or encryption were detected. Thus, BIOS machines that do
not use these but still have dmsetup present, would block indefinitely
on the "Installing GRUB2 boot loader..." step.

GRUB2 installation is performed in a chroot after the data have already
been recovered. ReaR would call grub-mkconfig which calls os-prober
which then executes dmsetup. However, it would never receive the response
trough host's udev as the rescue system would not have dmsetup present
to send it.

EDIT: reworded the description (thanks @pcahyna for suggestions!)
EDIT2: usr/share/rear/conf/GNU/Linux.conf is now being changed.

@lzaoral lzaoral force-pushed the always-include-dmsetup-ifpresent branch 2 times, most recently from 2eb44c6 to c726886 Compare January 25, 2022 17:01
@pcahyna
Copy link
Member

pcahyna commented Jan 25, 2022

Similar problem was reported for another case outside the context of ReaR where grub2-mkconfig is executed in a chroot with udev running outside the chroot: the Debian installer. The result is the same: it hangs forever when executing os-prober. See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=853927 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=860833 . The root cause of the problem has never been properly understood and the code in os-prober that executes dmsetup got eventually removed to fix this problem: https://bazaar.launchpad.net/~ubuntu-installer/os-prober/master/revision/419 . Some distributions still have the problematic os-prober though.

@pcahyna
Copy link
Member

pcahyna commented Jan 25, 2022

In order to show the problem the system must not use any LVM, thus the previous code did not include dmsetup in the rescue system. Note also that the old message "Device mapper found enabled. Including LVM tools." was a bit incorrect: LVM tools got included only if both device-mapper and LVM were found, finding device-mapper enabled was not enough.

@jsmeix
Copy link
Member

jsmeix commented Jan 26, 2022

In general when programs are useful in any case in the recovery system
shoudn't such programs be better added to PROGS in
the generic usr/share/rear/conf/GNU/Linux.conf ?

@jsmeix
Copy link
Member

jsmeix commented Jan 26, 2022

Only FYI:
The "Device mapper found enabled. Including LVM tools." message originated from
777133c

@jsmeix jsmeix self-assigned this Jan 26, 2022
@jsmeix jsmeix added bug The code does not do what it is meant to do cleanup labels Jan 26, 2022
@jsmeix jsmeix added this to the ReaR v2.7 milestone Jan 26, 2022
Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pcahyna
Copy link
Member

pcahyna commented Jan 31, 2022

In general when programs are useful in any case in the recovery system
shoudn't such programs be better added to PROGS in
the generic usr/share/rear/conf/GNU/Linux.conf ?

@jsmeix I agree. This case of dmsetup has nothing to do with LVM, really, although superficially it may appear related. So it belong to another location, and usr/share/rear/conf/GNU/Linux.conf is a good one. (Only a comment could be left here, explaining why dmsetup is not being added here despite being related to LVM, or perhaps the current code could be left as it is - I suppose that there is no problem with adding a tool to PROGS twice?)

@lzaoral lzaoral force-pushed the always-include-dmsetup-ifpresent branch 2 times, most recently from 2b9a1ec to d59a17d Compare January 31, 2022 15:46
@lzaoral
Copy link
Contributor Author

lzaoral commented Jan 31, 2022

@jsmeix @pcahyna I've added dmsetup and dmeventd with explanation for doing so to usr/share/rear/conf/GNU/Linux.conf.

@jsmeix
Copy link
Member

jsmeix commented Feb 1, 2022

There should be no problem adding something to PROGS or REQUIRED_PROGS several times
because in build/GNU/Linux/390_copy_binaries_libraries.sh
there is a sort -u

local all_binaries=( $( for bin in "${PROGS[@]}" "${REQUIRED_PROGS[@]}" ; do
                            ...
                        done 2>>/dev/$DISPENSABLE_OUTPUT_DEV | sort -u ) )
...
copy_binaries "$ROOTFS_DIR/bin" "${all_binaries[@]}"

and even without sort -u copying something several times should not cause harm.

@jsmeix
Copy link
Member

jsmeix commented Feb 1, 2022

When there are no objections I would like to merge it today afternoon.

@lzaoral lzaoral force-pushed the always-include-dmsetup-ifpresent branch from d59a17d to cf865b0 Compare February 1, 2022 13:40
# GRUB2 installation is performed in a chroot after the data have already
# been recovered. ReaR would call grub-mkconfig which calls os-prober
# which then executes dmsetup. However, it would never receive the response
# trough host's udev as the rescue system would not have dmsetup present
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*through, and while at it you can say that the expected response is in the form of releasing a SysV semaphore by dmsetup executed from udev and reference the related debian-installer bugs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking about it, the response is more "from udev processing" than "through udev" as it bypasses the udev control socket and is sent directly from one dmsetup to another via sysvipc.

Changes usr/share/rear/conf/GNU/Linux.conf

Older releases of os-prober (1.74 and below) use dmsetup as a fallback
solution for mounting when grub-mount is missing.

However, dmsetup was included in the rescue image if and only if LVM,
multipath or encryption were detected.  Thus, BIOS machines that do
not use these but still have dmsetup present, would block indefinitely
on the "Installing GRUB2 boot loader..." step.

GRUB2 installation is performed in a chroot after the data have already
been recovered.  ReaR would call grub-mkconfig which calls os-prober
which then executes dmsetup.  However, it would never receive the expected
response in the form of releasing a System V semaphore by dmsetup executed
by udevd outside the chroot as rescue system would not have dmsetup present.

related https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=853927
@lzaoral lzaoral force-pushed the always-include-dmsetup-ifpresent branch from cf865b0 to e5a84fd Compare February 2, 2022 10:52
@jsmeix
Copy link
Member

jsmeix commented Feb 2, 2022

I assume the description is OK now
so I would like to merge it today in about one hour
unless there are objections soon.

Copy link
Member

@pcahyna pcahyna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am satisfied with the explanation now, @jsmeix hope it is understandable - I participated in the debugging of the problem, so I am not a good outside observer.

@jsmeix jsmeix merged commit 39c79b3 into rear:master Feb 2, 2022
@lzaoral lzaoral deleted the always-include-dmsetup-ifpresent branch February 2, 2022 13:48
@jsmeix
Copy link
Member

jsmeix commented Feb 2, 2022

@lzaoral @pcahyna
thank you for your deep analysis of the problem and for your fix!

I think I understand "something" but the main point is that one understands
"without it things could block indefinitely at 'Installing GRUB2 boot loader...'"
so better safe than sorry and have sometimes needed things in the recovery system
because after "rear recover" failed it is a bit late to add missing stuff to the recovery system ;-)

@pcahyna
Copy link
Member

pcahyna commented Feb 2, 2022

so better safe than sorry and have sometimes needed things in the recovery system
because after "rear recover" failed it is a bit late to add missing stuff to the recovery system ;-)

Technically, this is not 100% true. You can copy dmsetup from the restored chroot to the running rescue ramdisk and restart rear recover, so this is one of the less disastrous errors that may happen (there are other ways to add missing stuff to the ramdisk, even a forgotten backup client can be added). But in this case the problem is really mysterious and it took us days to even figure out that dmsetup is what one needs (debian / ubuntu were never able to debug this problem in their installer and they rather requested removal of the problematic code from os-prober entirely), so it is very unlikely that one would find out the workaround.
Also, if you get a heart attack because the recovery of your broken $CRITICAL_SERVER failed, workarounds won't help you, as you are already dead :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The code does not do what it is meant to do cleanup fixed / solved / done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants