1986 ¶

Labels: bug, fixed / solved / done

Signum opened issue at 2018-11-28 15:56:¶

Relax-and-Recover (ReaR) Pull Request Template¶

Please fill in the following items before submitting a new pull request:

Pull Request Details:¶

Type: Bug Fix
Impact: High
Reference to related issue (URL):

https://github.com/rear/rear/issues/1986

How was this pull request tested?

I created a new ISO file on a laptop that had multiple interfaces and the issue was resolved by this diff.

Brief description of the changes in this pull request:

The DHCP client during recovery does not iterate correctly over all network interfaces that are found. It incorrectly takes the first found interface time and again. If the interface that is intended for recovery is not the first one then the automatic recovery will fail. The user can only work around the situation by running "dhclient" manually on the correct interface.

jsmeix commented at 2018-11-29 08:53:¶

@Signum
thank you for finding the bug and for your fix!

@gdha
I need your review here because I do not understand
how the variable DEVICE is meant to be used.

According to
https://github.com/rear/rear/wiki/Coding-Style
we use upper case variable names for global variables
i.e. for variables that are used in more than a single script
but in the scripts in skel/default/etc/scripts/system-setup.d/
and in usr/share/rear/skel/default/etc/scripts/
the variable DEVICE is only used in 58-start-dhclient.sh

Now I wonder if the variable DEVICE is perhaps in some
non-obvious way a global (config?) variable or if the variable
DEVICE is really only used locally in 58-start-dhclient.sh.

In the latter case that variable name should be in lower case
and according to https://github.com/rear/rear/wiki/Coding-Style
"For example dev is basically meaningless because
there are so many different kind of device-like thingies"
its meaningless name should be replaced by a meaningful name
for example something like

local network_interface
for network_interface in $( get_device_by_hwaddr ) ; do

(I also replaced the backticks for better readability)
because - strictly speaking - there are no network devices
(there are no device nodes like /dev/eth0 for networking hardware
instead the kernel has network interfaces for networking hardware plus
non-hardware network interfaces like lo for the 'loopback device' - argh!)
cf. the 'DESCRIPTION' in the traditional man ifconfig but even there
they also talk about devices - who cares - exact names do not matter,
everybody "just knows" what dev is - and DevOps are Device Operators
of course ;-)

jsmeix commented at 2018-12-04 15:09:¶

@gdha
if you have currently no time for a review,
I would like to merge it "as is" tomorrow.

jsmeix commented at 2018-12-05 13:12:¶

@Signum
thank you for fixing that non-standard use case for ReaR!

[Export of Github issue for rear/rear.]

#1987 PR merged: Fix for issue https://github.com/rear/rear/issues/1986¶