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

Merge SPL #7556

Closed
wants to merge 2 commits into from
Closed

Merge SPL #7556

wants to merge 2 commits into from

Conversation

behlendorf
Copy link
Contributor

@behlendorf behlendorf commented May 21, 2018

Description

This PR merges the zfsonlinux/spl repository in to the zfsonlinux/zfs respository and consists of two parts.

In the first part a merge-spl branch is created in the spl repository. The purpose of this branch is to slightly reorganize the spl source in order to allow it to merge cleanly with the zfs repository. Files which are no longer needed were removed and the rest were moved to non-conflicting locations. This branch was then merged in to the zfs repository to preserve the entire commit history.

The second portion of this PR makes the required changes to integrate the relocated spl kernel module in to the zfs repositories build system. After this point the master branch and 0.8.x and newer releases will not depend on an external spl repository.

NOTE: This PR has been squashed for the purposes of automated testing. The merge-spl branch which preserves all the history, but is otherwise identical, will be used for the real merge.

Motivation and Context

Having the spl source code split for the zfs source code is an unnecessary complication for developers, packagers, and end users. Bringing them together eliminates an entire class of issue which can result when using mismatched versions.

How Has This Been Tested?

Locally built and tested the following:

  • in-tree build and run of the ZTS
  • kmod-tracking RPM pacakges
  • kmods2 RPM packages

Pending results of the buildbot to indentify any missing build system issues.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@behlendorf behlendorf added Type: Building Indicates an issue related to building binaries Status: Work in Progress Not yet ready for general review labels May 21, 2018
@codecov
Copy link

codecov bot commented May 22, 2018

Codecov Report

Merging #7556 into master will increase coverage by 0.12%.
The diff coverage is 79.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7556      +/-   ##
==========================================
+ Coverage   77.61%   77.73%   +0.12%     
==========================================
  Files         336      362      +26     
  Lines      107608   110287    +2679     
==========================================
+ Hits        83522    85737    +2215     
- Misses      24086    24550     +464
Flag Coverage Δ
#kernel 78.37% <79.22%> (+0.24%) ⬆️
#user 66.83% <ø> (+0.18%) ⬆️

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 561ba8d...79e5b00. Read the comment docs.

@fling-
Copy link
Contributor

fling- commented May 22, 2018

Are the licensing issues expected?

@ryao
Copy link
Contributor

ryao commented May 22, 2018

@fling- I will probably get complaints from armchair experts after sys-fs/zfs-kmod is marked as both GPL and CDDL after this gets merged, but otherwise, no. Those complaints on sys-fs/zfs were the reason why I got the rc files relicensed under the BSD license several years ago. :/

@behlendorf
Copy link
Contributor Author

@fling- to be clear there is no licensing change. The SPL is still GPL licensed, it's only being relocated.

Copy link
Member

@ahrens ahrens left a comment

Choose a reason for hiding this comment

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

I looked at the changes to existing .c/.h files and they all looked reasonable. I assume that _BOOT was never used on Linux. Out of curiosity, how did you determine which header files were needed? Did you get compilation errors, or are most of the #include changes just code cleanup?

#else
#include <strings.h>
#endif
#include <sys/strings.h>
Copy link
Member

Choose a reason for hiding this comment

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

looks like we aren't really "determining" anything anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I'll drop the comment when I'm here.

@behlendorf
Copy link
Contributor Author

@ahrens thanks for taking a look. The issues with the headers were the result of me not bringing over all the headers from the SPL. There were quite a few which were entirely empty, or contained only a single
unrelated #include or misplaced #define. This seemed like as good a time as any to drop them. There's more refactoring/cleanup needed but I'm leaving that for a follow up PR.

@behlendorf behlendorf force-pushed the merge-spl-squashed branch 2 times, most recently from e2cea1d to cd73aef Compare May 23, 2018 17:25
@tonyhutter
Copy link
Contributor

@behlendorf are there any files in particular you want us to review?

@behlendorf
Copy link
Contributor Author

@tonyhutter I belive the most helpful thing would be if you could test the PR locally in your environment and make sure everything works as expected. The buildbot does provide a substanable amount of build/test coverage but there's still value in manually kicking the tires so to speak.

Merge a minimal version of the zfsonlinux/spl repository in to the
zfsonlinux/zfs repository.  Care was taken to prevent file conflicts
when merging and to preserve the spl repository history.  The spl
kernel module remains under the GPL license as documented by the
additional THIRDPARTYLICENSE.gplv2 file.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Minimal changes required to integrate the SPL sources in to the
ZFS repository build infrastructure.

Build system and packaging:
  * Renamed SPL_* autoconf m4 macros to ZFS_*.
  * Removed redundant SPL_* autoconf m4 macros.
  * Updated the RPM spec files to remove SPL package dependency.
  * The zfs package obsoletes the spl package, and the zfs-kmod
    package obsoletes the spl-kmod package.
  * The zfs-kmod-devel* packages were updated to add compatibility
    symlinks under /usr/src/spl-x.y.z until all dependent packages
    can be updated.  They will be removed in a future release.
  * Updated copy-builtin script for in-kernel builds.
  * Updated DKMS package to include the spl.ko.
  * Updated stale AUTHORS file to include all contributors.
  * Updated stale COPYRIGHT and included the SPL as an exception.

Required code changes:
  * Removed redundant HAVE_SPL macro.
  * Removed _BOOT from nvpairs since it doesn't apply for Linux.
  * Initial header cleanup (removal of empty headers, refactoring).
  * Remove SPL repository clone/build from zimport.sh.
  * Use of DEFINE_RATELIMIT_STATE and DEFINE_SPINLOCK removed due
    to build issues when forcing C99 compilation.
  * Replaced legacy ACCESS_ONCE with READ_ONCE.
  * Include needed headers for `current` and `EXPORT_SYMBOL`.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
TEST_ZIMPORT_SKIP="yes"
Build-ZFS: yes
Build-SPL: no
@tonyhutter
Copy link
Contributor

Kmem tracking still works (mem leaks added for testing):

[ 2792.253044] ZFS: Unloaded module v0.7.0-1401_g63fd8a3dc (DEBUG mode)
[ 2792.536228] kmem leaked 3300/8944212 bytes
[ 2792.536231] address          size  data             func:line
[ 2792.536248] 000000007505a1ef 100   03440b9200000000 zfs_ioc_pool_stats:1682
[ 2792.536252] 000000003c95065e 200   ee3b0dab00000000 zfs_ioc_pool_stats:1683
[ 2792.536254] 000000005559e127 100   83440b92736f2030 zfs_ioc_pool_stats:1682
[ 2792.536257] 00000000643ee585 200   ee3b0dabb0976aff zfs_ioc_pool_stats:1683

@behlendorf behlendorf deleted the merge-spl-squashed branch May 29, 2018 23:27
@behlendorf behlendorf mentioned this pull request May 29, 2018
@drescherjm
Copy link

This seems to have broken the gentoo 9999 ebuilds. I assume spl-9999 has to go away.

  • Preparing to run callback: "emerge --oneshot @module-rebuild sys-fs/zfs".....

  • IMPORTANT: 2 config files in '/etc/portage' need updating.
    Calculating dependencies

  • IMPORTANT: 1 news items need reading for repository 'gentoo'.

  • Use eselect news read to view new items.

  • See the CONFIGURATION FILES and CONFIGURATION FILES UPDATE TOOLS

  • sections of the emerge man page to learn how to update config files.
    ... done!

Verifying ebuild manifests
Jobs: 0 of 3 complete, 1 running Load avg: 4.59, 3.79, 2.51
Emerging (1 of 3) sys-kernel/spl-9999::gentoo
Jobs: 0 of 3 complete, 1 running Load avg: 4.59, 3.79, 2.51
Failed to emerge sys-kernel/spl-9999, Log file:
Jobs: 0 of 3 complete, 1 running Load avg: 4.30, 3.74, 2.50
'/var/tmp/portage/sys-kernel/spl-9999/temp/build.log'
Jobs: 0 of 3 complete, 1 running Load avg: 4.30, 3.74, 2.50
Jobs: 0 of 3 complete, 1 running, 1 failed Load avg: 4.30, 3.74, 2.50
Jobs: 0 of 3 complete, 1 failed Load avg: 4.30, 3.74, 2.50

Calculating dependencies *** Resuming merge...
.... done!

Jobs: 0 of 2 complete, 1 running Load avg: 4.30, 3.74, 2.50
Emerging (1 of 2) sys-fs/zfs-kmod-9999::gentoo
Jobs: 0 of 2 complete, 1 running Load avg: 4.30, 3.74, 2.50

@behlendorf
Copy link
Contributor Author

@drescherjm that's going to be a question for how @ryao want's to handle this.

@drescherjm
Copy link

I expected that..

@ryao
Copy link
Contributor

ryao commented May 31, 2018

@drescherjm @behlendorf It is fixed. Rebuilding sys-fs/zfs-kmod-9999 will now uninstall sys-kernel/spl. I am leaving sys-kernel/spl-9999 around for the time being. It is still useful if anyone wants to use it to build an old version by specifying the commit, but I plan to remove it when 0.8.0 is released.

@drescherjm
Copy link

Thanks. I am testing now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Work in Progress Not yet ready for general review Type: Building Indicates an issue related to building binaries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants