search

opnsense / core

OPNsense GUI, API and systems backend
https://opnsense.org/
BSD 2-Clause "Simplified" License
3.11k stars 706 forks source link

zpool cache not respected by opnsense #7553

Open nt1 opened 1 month ago

nt1 commented 1 month ago

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

Describe the bug

zpools other than zroot present in zpool cache (/etc/zfs/zpool.cache or /boot/zfs/zpool.cache) are not imported at boot time in OPNsense 24.1.9.

To Reproduce

Steps to reproduce the behavior:

  1. Access command line of device and get a root shell.
  2. Create a new zpool.
  3. Export the zpool (zpool export )
  4. Import the zpool (zpool import )
  5. Verify the zpool exists in zpool cache (zdb -U /etc/zfs/zpool.cache)
  6. Reboot the system.
  7. When the system is back up, access the command line and check if the new zpool was imported (zpool list). Notice only zroot exists.

Expected behavior

Upon reboot, the newly created zpool was imported.

Describe alternatives you considered

It is possible to manually import the zpool using zpool import after the reboot, but this doesn't really scale.

I was able to get the pool to import as expected by adding zfs_enable="YES" to /etc/rc.conf which allowed /etc/rc.d/zfs to start at boot per expectations. I notice opnsense sets this variable in /usr/local/etc/rc.loader.d/20-zfs, perhaps this variable isn't making it to freebsd's rc.d?

Screenshots

If applicable, add screenshots to help explain your problem.

Relevant log files

If applicable, information from log files supporting your claim.

Additional context

Add any other context about the problem here.

Environment

Software version used and hardware type if relevant, e.g.:

OPNsense 24.1.9

fichtner commented 1 month ago

The code you may want to improve is this:

https://github.com/opnsense/core/blob/0f73da02ad205aba1be2d7928c72fb8805b34eb4/src/etc/rc#L153-L160

Do not confuse _load with _enable vars. We don’t use the RC subsystem that much as it tends to interfere with the boot sequence.

nt1 commented 1 month ago

Adding -c CACHE_FILE to the referenced zpool import and removing changes to /etc/rc.conf fixes my issue. However, I'm fairly certain that adding additional pools worked in a previous release of OPNSense with no changes.

The behavior of zpool import -a changed when OpenZFS was adopted in FreeBSD 13. In prior releases, -a would default to searching for pools in /dev: https://github.com/freebsd/freebsd-src/blob/b5ad6b488d9e62d820fe90fdce4aee4f4d3d7162/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c#L2634

This changes in OpenZFS, used in 13+, the environment variable ZPOOL_IMPORT_PATH is used as the search path: https://github.com/freebsd/freebsd-src/blob/5fe9c9de03ef3191d216964bc4d8e427d5ed5720/sys/contrib/openzfs/cmd/zpool/zpool_main.c#L3492

fichtner commented 1 month ago

Thanks for looking into this. Feel free to provide a PR, but I won't be able to review and commit in the next two weeks.

fichtner commented 1 month ago

(I'd say that ZPOOL_IMPORT_PATH might be the better path forward as I've avoided -c in the past for portability.)

nt1 commented 1 month ago

I'll open a PR to fix this this evening. I can work around this issue for now, so no rush on the review.

nt1 commented 1 month ago

I will propose a change that imports pools from zpool cache at boot (just like vanilla FreeBSD) rather than allowing the implementation of zpool import to dictate what's imported.

Pros of using zpool cache

  1. It's explicit, fstab like behavior.
  2. Consistency across reboots, the system boots with the same pools it had at shutdown.
  3. Prevents importing potentially problematic pools that are connected to the system, but have never been successfully imported.
  4. On appliances, may help deter/prevent tampering with root fs.

Cons of using zpool cache

  1. The existence and purpose of zpool cache does not appear to be widely known or extensively documented. This may surprise some people.
  2. Boot behavior is changing. The end user may see a change in available pools after booting this release (easily resolved by importing missing pools and exporting those that shouldn't be there).
  3. The cache file should be present in every existing install. If, for any reason, the cache file is missing, the system will not boot properly.
  4. The path of the cachefile could change in future FreeBSD/OpenZFS releases (which would require a fix to rc).