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

RPM: Fix regex so old versions of DKMS modules are removed on upgrade #10327

Merged
merged 1 commit into from
May 15, 2020

Conversation

ColMelvin
Copy link
Contributor

@ColMelvin ColMelvin commented May 14, 2020

Due to a mismatch between the text and a regex looking for that text,
the %preuninstall script would never run the dkms remove command
necessary to avoid corrupting the DKMS data configuration. Increase
regex specificity to avoid this issue.

Closes: #9891

Motivation and Context

Solves #9891

Description

Adds more specificity to regex, to ensure it matches against a line that has a double quote ("). This keeps awk from printing no output.

How Has This Been Tested?

  1. Ran on a test Fedora system with ZFS 0.8.3 already installed on kernel 5.4.20
  2. Updated system to latest version of packages with dnf update -y -x zfs\* -x kernel\*.
  3. Downloaded zfs-dkms-0.8.3-1.fc31.src.rpm source RPM
  4. Installed and modified .spec file to include changes seen in this commit
  5. Built zfs-dkms-0.8.3-1.fc31.noarch.rpm locally, with this change in the script
  6. Ran rpm --reinstall --noscripts ./zfs-dkms-0.8.3-1.fc31.noarch.rpm to install the new %preuninstall script
  7. Ran dkms status to verify DKMS tool still works
  8. Snapshot the VM
  9. Ran dnf update -y zfs-dkms to install v0.8.4
  10. Ran dkms status and verified DKMS tool still worked, with no mention of zfs 0.8.3, but with an install of zfs 0.8.4 available
  11. Rebooted machine
  12. Noticed zfs-import-cache.service failed -- fixed with systemctl restart zfs-import-cache
  13. Reverted the VM
  14. Ran dnf update -y to upgrade zfs-dkms to v0.8.4 and the kernel to 5.6.11
  15. Ran dkms status and verified DKMS tool still worked, with no mention of zfs 0.8.3, but with an install of zfs 0.8.4 available (for both 5.4.20 and 5.6.11)
  16. Rebooted machine
  17. Noticed zfs-import-cache.service failed -- fixed with systemctl restart zfs-import-cache

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.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

Due to a mismatch between the text and a regex looking for that text,
the `%preuninstall` script would never run the `dkms remove` command
necessary to avoid corrupting the DKMS data configuration.  Increase
regex specificity to avoid this issue.

Closes: openzfs#9891
Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
@ColMelvin ColMelvin force-pushed the fix.module-removal-on-upgrade branch from 4a77718 to 5df480a Compare May 14, 2020 02:15
@ColMelvin
Copy link
Contributor Author

When testing this change, for whatever reason, zfs-import-cache.service fails consistently when I boot; however, systemctl restart zfs-import-cache quickly fixes the issue. Hopefully, this problem describes another, independent issue and this change is unrelated.

Unfortunately, I can't verify that assertion because, without this fix, updating zfs-dkms to 0.8.4 will not create a zfs module - and I cannot manually create one with dkms install.

@codecov-io
Copy link

codecov-io commented May 14, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10327      +/-   ##
==========================================
+ Coverage   79.24%   79.28%   +0.03%     
==========================================
  Files         390      390              
  Lines      123336   123336              
==========================================
+ Hits        97737    97784      +47     
+ Misses      25599    25552      -47     
Flag Coverage Δ
#kernel 79.92% <ø> (+0.01%) ⬆️
#user 65.15% <ø> (+0.98%) ⬆️
Impacted Files Coverage Δ
module/zfs/vdev_raidz.c 88.88% <0.00%> (-4.91%) ⬇️
module/icp/algs/modes/gcm.c 77.38% <0.00%> (-4.43%) ⬇️
module/zfs/dsl_synctask.c 92.40% <0.00%> (-2.54%) ⬇️
cmd/ztest/ztest.c 73.67% <0.00%> (-2.41%) ⬇️
module/zfs/zrlock.c 89.23% <0.00%> (-1.54%) ⬇️
module/zfs/dmu_traverse.c 95.68% <0.00%> (-1.33%) ⬇️
module/zfs/vdev_trim.c 94.21% <0.00%> (-1.27%) ⬇️
module/zfs/spa_log_spacemap.c 93.40% <0.00%> (-1.15%) ⬇️
module/os/linux/zfs/zio_crypt.c 80.09% <0.00%> (-1.11%) ⬇️
module/os/linux/zfs/zfs_file_os.c 84.15% <0.00%> (-1.00%) ⬇️
... and 53 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 b29e31d...5df480a. Read the comment docs.

@behlendorf behlendorf added Component: Packaging custom packages Status: Code Review Needed Ready for review and testing labels May 14, 2020
@behlendorf behlendorf requested a review from tonyhutter May 14, 2020 16:32
Copy link
Contributor

@tonyhutter tonyhutter left a comment

Choose a reason for hiding this comment

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

I gave this a try by hand, and looks like it works:

# without patch
$ awk -F'"' '/META_ALIAS/ { print $2; exit 0 }' zfs_config.h 

# with patch
$ awk -F'"' '/META_ALIAS\s+"/ { print $2; exit 0 }' zfs_config.h 
zfs-0.8.4-1

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 15, 2020
@behlendorf behlendorf merged commit 4d6043f into openzfs:master May 15, 2020
as-com pushed a commit to as-com/zfs that referenced this pull request Jun 20, 2020
Due to a mismatch between the text and a regex looking for that text,
the `%preuninstall` script would never run the `dkms remove` command
necessary to avoid corrupting the DKMS data configuration.  Increase
regex specificity to avoid this issue.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
Closes: openzfs#9891
Closes openzfs#10327 
(cherry picked from commit 4d6043f)
tonyhutter added a commit to tonyhutter/zfs that referenced this pull request Sep 15, 2020
Due to a mismatch between the text and a regex looking for that text,
the `%preuninstall` script would never run the `dkms remove` command
necessary to avoid corrupting the DKMS data configuration.  Increase
regex specificity to avoid this issue.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
Closes: openzfs#9891
Closes openzfs#10327
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 15, 2020
Due to a mismatch between the text and a regex looking for that text,
the `%preuninstall` script would never run the `dkms remove` command
necessary to avoid corrupting the DKMS data configuration.  Increase
regex specificity to avoid this issue.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
Closes: openzfs#9891
Closes openzfs#10327
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Due to a mismatch between the text and a regex looking for that text,
the `%preuninstall` script would never run the `dkms remove` command
necessary to avoid corrupting the DKMS data configuration.  Increase
regex specificity to avoid this issue.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
Closes: openzfs#9891
Closes openzfs#10327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Packaging custom packages Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade of zfs to new version corrupts dkms kernel module configuration in Fedora
4 participants