Add kernel module auto-loading #7287
Conversation
config/kernel-misc-minor.m4
Outdated
| AC_MSG_CHECKING([for available /dev/zfs minor]) | ||
|
|
||
| for i in $(seq 249 -1 200); do | ||
| if ! grep -q "#define.*_MINOR.*$i" \ |
There was a problem hiding this comment.
"^#define\s\+.*_MINOR\s\+.*$i" would be a bit more exact and prevent potential future false negatives (they currently match the same device minor numbers).
module/zfs/zfs_ioctl.c
Outdated
| * In this case the kernel modules must be manually loaded. | ||
| */ | ||
| zfs_misc.minor = MISC_DYNAMIC_MINOR; | ||
| error = misc_register(&zfs_misc); |
There was a problem hiding this comment.
keeping the existing printk (moved into a second if after the EBUSY one) and adding a second printk inside the if like this:
printk(KERN_INFO "ZFS: misc_register() with static minor %d failed %d, retrying with MISC_DYNAMIC_MINOR\n", ZFS_MINOR, error);
would make debugging issues here a lot easier I think (although they will probably trigger rarely if at all).
|
So, if I understand the udev rules. udev will load zfs modules if any block devices are part of a zpool. But on initial installation, we still have to worry about loading module prior to zed starting? |
|
Will this load the kernel-module (and start dependent services) when installing ZFS for the first time? |
|
@Fabian-Gruenbichler thanks for the review and keeping me honest! I've updated the PR and addressed your review feedback. I also did a little testing of the patch on CentOS 7 and discovered a few small issues which have been subsequently fixed upstream in Fedora. https://lists.freedesktop.org/archives/systemd-devel/2014-October/024673.html Anyway the take away is that for CentOS 7 for things to just work after installing the packages (without a reboot) we need to manually restart the @utopiabound @jgrund it would be great if you could verify this PR does work as intended on your sytstems.
Yes. It will proactively load the modules for you, although that really shouldn't been needed anymore with this change.
Nope, with this change starting the zed will trigger the module load. Effectively the zed service has been completely decoupled from the import step and it can run at any time. It is still the case that in accordance with the Fdora packaging guidelines the service won't be automatically started as part of the install, only enabled. |
Historically a dynamic misc minor number was registered for the /dev/zfs device in order to prevent minor number collisions. This was fine but it prevented us from being able to use the kernel module auto-loaded which requires a known reserved value. Resolve this issue by adding a configure test to find an available misc minor number which can then be used in MODULE_ALIAS_MISCDEV at build time. By adding this alias the zfs kmod is added to the list of known static-nodes and the systemd-tmpfiles-setup-dev service will create a /dev/zfs character device at boot time. This in turn allows us to update the 90-zfs.rules file to make it aware this is a static node. The upshot of this is that whenever a process (zpool, zfs, zed) opens the /dev/zfs the kmods will be automatic loaded. This even works for unprivileged users so there is no longer a need to manually load the modules at boot time. As an additional bonus the zed now no longer needs to start after the zfs-import.service since it will trigger the module load. In the unlikely event the minor number we selected conflicts with another out of tree unregistered minor number the code falls back to dynamically allocating it. In this case the modules again must be manually loaded. Note that due to the change in the method of registering the minor number the zimport.sh test case may incorrectly fail when the static node for the installed packages is created instead of the dynamic one. This issue will only transiently impact zimport.sh for this single commit when we transition and are mixing and matching methods. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> TEST_ZIMPORT_SKIP="yes"
Codecov Report
@@ Coverage Diff @@
## master #7287 +/- ##
==========================================
+ Coverage 76.33% 76.4% +0.06%
==========================================
Files 328 328
Lines 104014 104019 +5
==========================================
+ Hits 79403 79473 +70
+ Misses 24611 24546 -65
Continue to review full report at Codecov.
|
|
@behlendorf What release will this change be part of? |
|
@jgrund it will be part of the 0.8 release, and is under consideration for inclusion in a future 0.7.x point release. |
|
Thanks |
|
@kpande sorry! That case didn't get covered in the manual testing. I would have expected the updated udev rule to take care of it, but it sounds like that isn't the case. Did you settle on a solution? |
Resolves importing root pool during boot in dracut. This case was inadvertently broken with the module autoloading change in #7287. Reviewed-by: Matthew Thode <prometheanfire@gentoo.org> Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Kash Pande <kash@tripleback.net> Closes #7322
|
Also breaks NixOS with linux 4.16. Earlier kernels versions still work. |
|
So manually modprobing zfs is required now? |
|
@Mic92 it may be needed early in the boot before the normal kernel module autoloading infrastructure is in place. |
Resolves importing root pool during boot in dracut. This case was inadvertently broken with the module autoloading change in openzfs#7287. Reviewed-by: Matthew Thode <prometheanfire@gentoo.org> Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Kash Pande <kash@tripleback.net> Closes openzfs#7322
Resolves importing root pool during boot in dracut. This case was inadvertently broken with the module autoloading change in openzfs#7287. Reviewed-by: Matthew Thode <prometheanfire@gentoo.org> Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Kash Pande <kash@tripleback.net> Closes openzfs#7322
Resolves importing root pool during boot in dracut. This case was inadvertently broken with the module autoloading change in #7287. Reviewed-by: Matthew Thode <prometheanfire@gentoo.org> Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov> Signed-off-by: Kash Pande <kash@tripleback.net> Closes #7322
|
When no pool has been created at all，and all disks are free to use which have no metadata about zfs_member, manually modprobing zfs is required after reboot. I found this situation in CentOS 7.2. |
Historically a dynamic misc minor number was registered for the /dev/zfs device in order to prevent minor number collisions. This was fine but it prevented us from being able to use the kernel module auto-loaded which requires a known reserved value. Resolve this issue by adding a configure test to find an available misc minor number which can then be used in MODULE_ALIAS_MISCDEV at build time. By adding this alias the zfs kmod is added to the list of known static-nodes and the systemd-tmpfiles-setup-dev service will create a /dev/zfs character device at boot time. This in turn allows us to update the 90-zfs.rules file to make it aware this is a static node. The upshot of this is that whenever a process (zpool, zfs, zed) opens the /dev/zfs the kmods will be automatic loaded. This even works for unprivileged users so there is no longer a need to manually load the modules at boot time. As an additional bonus the zed now no longer needs to start after the zfs-import.service since it will trigger the module load. In the unlikely event the minor number we selected conflicts with another out of tree unregistered minor number the code falls back to dynamically allocating it. In this case the modules again must be manually loaded. Note that due to the change in the method of registering the minor number the zimport.sh test case may incorrectly fail when the static node for the installed packages is created instead of the dynamic one. This issue will only transiently impact zimport.sh for this single commit when we transition and are mixing and matching methods. Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> TEST_ZIMPORT_SKIP="yes" Closes openzfs#7287
Historically a dynamic misc minor number was registered for the /dev/zfs device in order to prevent minor number collisions. This was fine but it prevented us from being able to use the kernel module auto-loaded which requires a known reserved value. Resolve this issue by adding a configure test to find an available misc minor number which can then be used in MODULE_ALIAS_MISCDEV at build time. By adding this alias the zfs kmod is added to the list of known static-nodes and the systemd-tmpfiles-setup-dev service will create a /dev/zfs character device at boot time. This in turn allows us to update the 90-zfs.rules file to make it aware this is a static node. The upshot of this is that whenever a process (zpool, zfs, zed) opens the /dev/zfs the kmods will be automatic loaded. This even works for unprivileged users so there is no longer a need to manually load the modules at boot time. As an additional bonus the zed now no longer needs to start after the zfs-import.service since it will trigger the module load. In the unlikely event the minor number we selected conflicts with another out of tree unregistered minor number the code falls back to dynamically allocating it. In this case the modules again must be manually loaded. Note that due to the change in the method of registering the minor number the zimport.sh test case may incorrectly fail when the static node for the installed packages is created instead of the dynamic one. This issue will only transiently impact zimport.sh for this single commit when we transition and are mixing and matching methods. Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> TEST_ZIMPORT_SKIP="yes" Closes #7287
Description
Historically a dynamic misc minor number was registered for the
/dev/zfs device in order to prevent minor number collisions. This
was fine but it prevented us from being able to use the kernel
module auto-loaded which requires a known reserved value.
Resolve this issue by adding a configure test to find an available
misc minor number which can then be used in MODULE_ALIAS_MISCDEV at
build time. By adding this alias the zfs kmod is added to the list
of known static-nodes and the systemd-tmpfiles-setup-dev service
will create a /dev/zfs character device at boot time.
This in turn allows us to update the 90-zfs.rules file to make it
aware this is a static node. The upshot of this is that whenever
a process (zpool, zfs, zed) opens the /dev/zfs the kmods will be
automatic loaded. This even works for unprivileged users so there
is no longer a need to manually load the modules at boot time.
As an additional bonus the zed now no longer needs to start after
the zfs-import.service since it will trigger the module load.
In the unlikely event the minor number we selected conflicts with
another out of tree unregistered minor number the code falls back
to dynamically allocating it. In this case the modules again
must be manually loaded.
Motivation and Context
This is a long overdue bit of functionality. This way things will just work.
How Has This Been Tested?
Manually on Fedora 27.
Types of changes
Checklist:
Signed-off-by.