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

small fix to the 58-start-dhclient.sh script #2038

Merged
merged 2 commits into from Feb 15, 2019
Merged

small fix to the 58-start-dhclient.sh script #2038

merged 2 commits into from Feb 15, 2019

Conversation

gdha
Copy link
Member

@gdha gdha commented Feb 11, 2019

Signed-off-by: Gratien D'haese gratien.dhaese@gmail.com

Pull Request Details: small fix to the 58-start-dhclient.sh script
  • Type: Bug Fix

  • Impact: Low

  • Reference to related issue (URL): Fail to acquire DHCP IP in 58-start-dhclient.sh #1274

  • How was this pull request tested? not tested yet

  • Brief description of the changes in this pull request: with multiple lan interface it might fail to acquire an IP address, with this little fix it should work.

Signed-off-by: Gratien D'haese <gratien.dhaese@gmail.com>
@gdha gdha added the minor bug An alternative or workaround exists label Feb 11, 2019
@gdha gdha added this to the ReaR v2.5 milestone Feb 11, 2019
@gdha gdha self-assigned this Feb 11, 2019
@gdha gdha requested a review from jsmeix February 11, 2019 16:56
@gerhard-tinned
Copy link

Would love to get this fix ... I ran exactly into this problem.

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.

Because dhclient.pid is nowhere else used in ReaR scripts
I assume changing the filename of the pid-file does not have
any unexpected bad side effects so that I approve this change.

@jsmeix
Copy link
Member

jsmeix commented Feb 14, 2019

I am not at all a DHCP expert but by plain looking at the code I wonder
why not the same fix is also needed for the IPv6 DHCP clients case in
usr/share/rear/skel/default/etc/scripts/system-setup.d/58-start-dhclient.sh

@jsmeix
Copy link
Member

jsmeix commented Feb 14, 2019

@gdha
if that fix is really only needed for the IPv4 case but not for the IPv6 case
could you then please add a comment at that code that explains why
both cases are different, cf. "Code should be easy to understand" at
https://github.com/rear/rear/wiki/Coding-Style

Signed-off-by: Gratien D'haese <gratien.dhaese@gmail.com>
@gdha
Copy link
Member Author

gdha commented Feb 15, 2019

@jsmeix sharp remark Johannes and right as usual 👍 fixed it too now

@gdha gdha merged commit 2e88514 into rear:master Feb 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed / solved / done minor bug An alternative or workaround exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants