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

Fix inclusion of libgcc_s.so on Void #6715

Merged
merged 3 commits into from Oct 9, 2017

Conversation

privb0x23
Copy link
Contributor

Description

Add a simple additional path check for "/usr/lib/libgcc_s.so*" and install it in the initramfs.

Motivation and Context

On Void Linux (x86_64 musl) libgcc_s.so is located in "/usr/lib" so it is not found by dracut (in the fallback case) and it produces an error.

How Has This Been Tested?

Changes result in the successful inclusion of "/usr/lib/libgcc_s.so" within the initramfs and no error is seen.

Void Linux x86_64 musl
Kernel 4.12.13_1
Zfs 0.7.1_1
Spl 0.7.1_1
Libgcc 6.3.0_5
Dracut 046_1

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have read the CONTRIBUTING document.

On Void Linux (x86_64 musl) libgcc_s.so is located in "/usr/lib" so it is not found by dracut and it produces an error.
- Add a simple additional path check for "/usr/lib/libgcc_s.so*" and install it in the initramfs.
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Please fix the commit style warnings, you can run make checkstyle locally before pushing. And then one minor suggested change.

@@ -46,6 +46,9 @@ install() {
# On systems with gcc-config (Gentoo, Funtoo, etc.):
# Use the current profile to resolve the appropriate path
dracut_install "/usr/lib/gcc/$(s=$(gcc-config -c); echo ${s%-*}/${s##*-})/libgcc_s.so.1"
elif [[ -n "$(ls /usr/lib/libgcc_s.so*)" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's redirect stderr to /dev/null to get rid of the error message when it doesn't exist.

elif [[ -n "$(ls /usr/lib/libgcc_s.so* 2>/dev/null)" ]]; then

@codecov
Copy link

codecov bot commented Oct 5, 2017

Codecov Report

Merging #6715 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6715      +/-   ##
==========================================
- Coverage   74.09%   74.07%   -0.02%     
==========================================
  Files         295      295              
  Lines       93882    93882              
==========================================
- Hits        69558    69540      -18     
- Misses      24324    24342      +18
Flag Coverage Δ
#kernel 74.35% <ø> (-0.05%) ⬇️
#user 63.33% <ø> (+0.03%) ⬆️
Impacted Files Coverage Δ
cmd/zed/agents/zfs_agents.c 75.78% <0%> (-15.63%) ⬇️
cmd/zed/agents/zfs_mod.c 57.54% <0%> (-4.92%) ⬇️
module/zfs/zpl_xattr.c 79.69% <0%> (-2.38%) ⬇️
cmd/zed/zed_disk_event.c 81.15% <0%> (-2.18%) ⬇️
module/zfs/zrlock.c 82.81% <0%> (-1.57%) ⬇️
module/zfs/zap_leaf.c 84.9% <0%> (-0.95%) ⬇️
module/zfs/spa_history.c 93.66% <0%> (-0.46%) ⬇️
module/zfs/zfs_rlock.c 95.49% <0%> (-0.41%) ⬇️
module/zfs/vdev_label.c 94.62% <0%> (-0.4%) ⬇️
module/zfs/dmu.c 82.91% <0%> (-0.35%) ⬇️
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39f5662...dbc8f23. Read the comment docs.

@behlendorf
Copy link
Contributor

@privb0x23 please add your signed-off-by to the commit as well so we can accept it, git commit -s. You can run make checkstyle locally to verify there aren't any style warning before pushing the change to the PR.

Only output was:

"Unescaped left brace in regex is deprecated here (and will be fatal in Perl 5.30), passed through in regex; marked by <-- HERE in m/(?:(?:\b(?:enum|struct|union)\s*[^\{]*)|(?:\s+=\s*)){ <-- HERE / at scripts/cstyle.pl line 830."

(Using Perl v5.26.0)

Signed-off-by: privb0x23 <privb0x23@users.noreply.github.com>
@privb0x23
Copy link
Contributor Author

Greetings,

I'm not sure why the automated "buildbot/Ubuntu 17.10 x86_64 (STYLE)" check is failing. Perhaps it has something to do with the commit message rather than the actual code/style, as the output includes:

error: commit message body contains line over 72 characters
Makefile:1425: recipe for target 'commitcheck' failed
make: *** [commitcheck] Error 1

My proposed changes are to a few lines in the dracut 90zfs module, so should not affect any failures in "real" ZFS code. The make checkstyle returned 0, with the only output related to a Perl warning.

The failure in the automated "buildbot/Fedora 26 x86_64 (TEST)" build seems to be related to "chattr", so also should not be affected by the dracut module.

Thanks

@behlendorf
Copy link
Contributor

@privb0x23 yes, the style warning is pertaining to the commit message. To keep the commits readable for everyone individual lines should be under 72 characters. I'll go ahead and fix this when I squash and merge it. The rest of the patch looks good, thanks!

@behlendorf behlendorf merged commit 4f23c5d into openzfs:master Oct 9, 2017
@behlendorf behlendorf added this to PR Needed in 0.7.3 Oct 9, 2017
aerusso pushed a commit to aerusso/zfs that referenced this pull request Oct 11, 2017
On Void Linux (x86_64 musl) libgcc_s.so is located in "/usr/lib"
so it is not found by dracut and it produces an error.

Add a simple additional path check for "/usr/lib/libgcc_s.so*"
and install it in the initramfs.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: privb0x23 <privb0x23@users.noreply.github.com>
Closes openzfs#6715
tonyhutter pushed a commit that referenced this pull request Oct 16, 2017
On Void Linux (x86_64 musl) libgcc_s.so is located in "/usr/lib"
so it is not found by dracut and it produces an error.

Add a simple additional path check for "/usr/lib/libgcc_s.so*"
and install it in the initramfs.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: privb0x23 <privb0x23@users.noreply.github.com>
Closes #6715
@tonyhutter tonyhutter moved this from PR Needed to Merged to 0.7.3 in 0.7.3 Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
0.7.3
Merged to 0.7.3
Development

Successfully merging this pull request may close these issues.

None yet

2 participants