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

Jump to bottom

Enable building the DKMS package on Debian-based distributions #6731

Merged
merged 4 commits into from Oct 15, 2017
Merged

Enable building the DKMS package on Debian-based distributions #6731

merged 4 commits into from Oct 15, 2017

Conversation

Conan-Kudo
Copy link
Contributor

@Conan-Kudo Conan-Kudo commented Oct 7, 2017 •
edited

Description

This pull request adds the ability to generate DKMS packages that are compatible and installable for Debian and derivatives.

This depends on openzfs/spl/pull/657.

Motivation and Context

This change is intended to fix /issues/6044.

How Has This Been Tested?

I built it and installed the DKMS packages on Debian 9.0 (Stretch) for x86_64/amd64. The packages installed and did the right things (built the module and installed it).

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.

@Conan-Kudo Conan-Kudo mentioned this pull request Oct 7, 2017
Merged
behlendorf
behlendorf requested changes Oct 9, 2017
View reviewed changes
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.

Thanks! Functionally this looks good and worked as expected for me. There are just two minor additional tweaks I'd like to make mentioned inline.

config/deb.am Outdated
@@ -60,4 +71,4 @@ if CONFIG_USER
$$pkg8 $$pkg9;
endif

deb: deb-kmod deb-utils
deb: deb-kmod deb-dkms deb-utils
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 also add a target for this in the top-level Makefile.am to make it easy to build dkms packages without having to build kmods.

diff --git a/Makefile.am b/Makefile.am
index 4977448..05107cb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -50,5 +50,6 @@ etags:
 tags: ctags etags
 
 pkg: @DEFAULT_PACKAGE@
+pkg-dkms: @DEFAULT_PACKAGE@-dkms
 pkg-kmod: @DEFAULT_PACKAGE@-kmod
 pkg-utils: @DEFAULT_PACKAGE@-utils

config/deb.am Outdated
@@ -24,6 +24,17 @@ if CONFIG_KERNEL
$(RM) $$pkg1
endif


deb-dkms: deb-local rpm-dkms
if CONFIG_KERNEL
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove the CONFIG_KERNEL and CONFIG_USER guards. This happened in rpm.am quick a while ago was was overlooked in deb.am. This allows us to run ./configure --with-config=srpm which runs only enough of configure to generate Makefile's with targets capable of invoking the right machinery to build actual packages.

@behlendorf
Copy link
Contributor

@Conan-Kudo could you please add the following line to the ZFS commit comment. It will ensure this PR gets tested with its SPL counterpart.

Requires-spl: refs/pull/657/head

@Conan-Kudo
Copy link
Contributor Author

@behlendorf Does it matter which commit I put it in, or do I need to put it into both?

Also, do these requests also apply to spl, as I think I saw similar things there.

@behlendorf
Copy link
Contributor

You need only add it to the top ZFS commit, and yes please make the same changes to the SPL PR. You'll want to push the SPL updates first then the ZFS updates.

I also have a few tweaks to the buildbot I'll push for review which allow it to optionally use dkms packages I'll push once I'm sure they work.

@Conan-Kudo
config/deb.am: Enable building DKMS packages for Debian
fb951b7 
Signed-off-by: Neal Gompa <ngompa@datto.com>

Requires-spl: refs/pull/657/head
@Conan-Kudo
rpm/generic/zfs-dkms.spec.in: Adjust spec to be Debian-compatible
f656857 
This involved three things:
* Condition kernel-devel Req to RPM distros
* Adjust the DKMS Req to have a minimum of a version only
* Ensure that --rpm_safe_upgrade isn't used on non-RPM distros

Signed-off-by: Neal Gompa <ngompa@datto.com>
@codecov
Copy link

codecov bot commented Oct 13, 2017 •
edited

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6731      +/-   ##
==========================================
- Coverage   74.18%   74.14%   -0.04%     
==========================================
  Files         295      295              
  Lines       93905    93865      -40     
==========================================
- Hits        69659    69597      -62     
- Misses      24246    24268      +22
Flag Coverage Δ
#kernel 74.21% <ø> (-0.16%) ⬇️
#user 63.48% <ø> (+0.01%) ⬆️
Impacted Files Coverage Δ
module/icp/algs/skein/skein_port.h 0% <0%> (-100%) ⬇️
cmd/zed/agents/zfs_mod.c 59.29% <0%> (-8.08%) ⬇️
cmd/zed/zed_event.c 43.04% <0%> (-5.42%) ⬇️
cmd/zed/agents/zfs_diagnosis.c 34.31% <0%> (-4.7%) ⬇️
lib/libuutil/uu_list.c 62.75% <0%> (-4.7%) ⬇️
cmd/zed/agents/zfs_retire.c 52.79% <0%> (-2.49%) ⬇️
module/nvpair/nvpair.c 73.3% <0%> (-1.67%) ⬇️
module/zfs/zrlock.c 84.37% <0%> (-1.57%) ⬇️
module/zfs/zvol.c 84.16% <0%> (-1%) ⬇️
module/zfs/spa_errlog.c 93.07% <0%> (-0.77%) ⬇️
... and 44 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 cdc15a7...61ffb75. Read the comment docs.

@Conan-Kudo
Copy link
Contributor Author

@behlendorf The requested changes are done in SPL and ZFS PRs.

@Conan-Kudo
config/deb.am: Drop CONFIG_KERNEL and CONFIG_USER guards
02af7a0 
Signed-off-by: Neal Gompa <ngompa@datto.com>
@Conan-Kudo
Makefile.am: Add pkg-dkms target
61ffb75 
Signed-off-by: Neal Gompa <ngompa@datto.com>
@behlendorf behlendorf merged commit 7670f72 into openzfs:master Oct 15, 2017
@Conan-Kudo Conan-Kudo deleted the deb-dkms branch October 15, 2017 20:00
@MasterCATZ
Copy link

Thank You, I have been dreading updating my kernel lately will see how 4.13 goes

@tonyhutter tonyhutter added this to PR Needed in 0.7.3 Oct 16, 2017
tonyhutter pushed a commit that referenced this pull request Oct 18, 2017
@Conan-Kudo @tonyhutter
Add DKMS package on Debian-based distributions
abe30b7 
* config/deb.am: Enable building DKMS packages for Debian
* rpm/generic/zfs-dkms.spec.in: Adjust spec to be Debian-compatible
  * Condition kernel-devel Req to RPM distros
  * Adjust the DKMS Req to have a minimum of a version only
  * Ensure that --rpm_safe_upgrade isn't used on non-RPM distros
* config/deb.am: Drop CONFIG_KERNEL and CONFIG_USER guards
* Makefile.am: Add pkg-dkms target

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Neal Gompa <ngompa@datto.com>
Closes #6044
Closes #6731
@tonyhutter tonyhutter moved this from PR Needed to Merged to 0.7.3 in 0.7.3 Oct 18, 2017
wli5 pushed a commit to wli5/zfs that referenced this pull request Oct 20, 2017
@Conan-Kudo @wli5
Add DKMS package on Debian-based distributions
0f722f6 
* config/deb.am: Enable building DKMS packages for Debian
* rpm/generic/zfs-dkms.spec.in: Adjust spec to be Debian-compatible
  * Condition kernel-devel Req to RPM distros
  * Adjust the DKMS Req to have a minimum of a version only
  * Ensure that --rpm_safe_upgrade isn't used on non-RPM distros
* config/deb.am: Drop CONFIG_KERNEL and CONFIG_USER guards
* Makefile.am: Add pkg-dkms target

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Neal Gompa <ngompa@datto.com>
Closes openzfs#6044
Closes openzfs#6731
Nasf-Fan pushed a commit to Nasf-Fan/zfs that referenced this pull request Nov 4, 2017
@Conan-Kudo @Nasf-Fan
Add DKMS package on Debian-based distributions
6ab853c 
* config/deb.am: Enable building DKMS packages for Debian
* rpm/generic/zfs-dkms.spec.in: Adjust spec to be Debian-compatible
  * Condition kernel-devel Req to RPM distros
  * Adjust the DKMS Req to have a minimum of a version only
  * Ensure that --rpm_safe_upgrade isn't used on non-RPM distros
* config/deb.am: Drop CONFIG_KERNEL and CONFIG_USER guards
* Makefile.am: Add pkg-dkms target

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Neal Gompa <ngompa@datto.com>
Closes openzfs#6044
Closes openzfs#6731
Nasf-Fan pushed a commit to Nasf-Fan/zfs that referenced this pull request Nov 6, 2017
@Conan-Kudo @Nasf-Fan
Add DKMS package on Debian-based distributions
3b4cf10 
* config/deb.am: Enable building DKMS packages for Debian
* rpm/generic/zfs-dkms.spec.in: Adjust spec to be Debian-compatible
  * Condition kernel-devel Req to RPM distros
  * Adjust the DKMS Req to have a minimum of a version only
  * Ensure that --rpm_safe_upgrade isn't used on non-RPM distros
* config/deb.am: Drop CONFIG_KERNEL and CONFIG_USER guards
* Makefile.am: Add pkg-dkms target

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Neal Gompa <ngompa@datto.com>
Closes openzfs#6044
Closes openzfs#6731
Nasf-Fan pushed a commit to Nasf-Fan/zfs that referenced this pull request Jan 29, 2018
@Conan-Kudo @Nasf-Fan
Add DKMS package on Debian-based distributions
db49bf7 
* config/deb.am: Enable building DKMS packages for Debian
* rpm/generic/zfs-dkms.spec.in: Adjust spec to be Debian-compatible
  * Condition kernel-devel Req to RPM distros
  * Adjust the DKMS Req to have a minimum of a version only
  * Ensure that --rpm_safe_upgrade isn't used on non-RPM distros
* config/deb.am: Drop CONFIG_KERNEL and CONFIG_USER guards
* Makefile.am: Add pkg-dkms target

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Neal Gompa <ngompa@datto.com>
Closes openzfs#6044
Closes openzfs#6731
Nasf-Fan pushed a commit to Nasf-Fan/zfs that referenced this pull request Feb 13, 2018
@Conan-Kudo @Nasf-Fan
Add DKMS package on Debian-based distributions
191c89a 
* config/deb.am: Enable building DKMS packages for Debian
* rpm/generic/zfs-dkms.spec.in: Adjust spec to be Debian-compatible
  * Condition kernel-devel Req to RPM distros
  * Adjust the DKMS Req to have a minimum of a version only
  * Ensure that --rpm_safe_upgrade isn't used on non-RPM distros
* config/deb.am: Drop CONFIG_KERNEL and CONFIG_USER guards
* Makefile.am: Add pkg-dkms target

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Neal Gompa <ngompa@datto.com>
Closes openzfs#6044
Closes openzfs#6731
@jwittlincohen jwittlincohen mentioned this pull request Feb 7, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
0.7.3
Merged to 0.7.3
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] Add support for building DKMS ZFS and SPL binaries for Debian/Ubuntu
3 participants
@Conan-Kudo @behlendorf @MasterCATZ