The ZFS boot code seems to not even bother to load anything when it discovers an unsupported flag on a zpool. This makes perfect sense for a typical mount, but bootloader is special: when it fails one needs to physically get to the machine with a pen drive in hand to bring it up. Would it be possible to modify the boot loader ZFS support code to complain about the unsupported flags, but then try to proceed anyway? It's read-only, so it's not going to corrupt the pool, and in the worst case it will just fail to load, effectively falling back to current behaviour.
You are stating about loader, so I'll intentionally ignore here about features not supported by features unsupported after boot (not implemented in zfs.ko, used AFTER kernel starts). For features supported by zfs.ko but not by loader (zfsloader, loader.efi), unsupported featres are categorized into 2. 1. Read-only compatible feature 2. Features not in 1. For case 1, loader can (theoretically) sanely read the pool. So should run sanely even if they are enabled and active. For case 2, if the features are enabled but not yet set to active and actually used, theoretically loader can still read and boot the pool. But once any of the features are actually used, loader no longer can read it. For example, if new hash function which loader doesn't support is activated and used, loader cannot calculate the hash to check whether the file (/boot/loader.conl, kernel itself, ...) is broken or not. As broken files SHALL not be used to boot. The only solution for case 2 is to implement the unspported feature into loader. But if your complaints are about case 1, loader.should be fixed.
(In reply to Tomoaki AOKI from comment #1) I think the ask is simply make check_feature() and check_mos_features() (/stand/libsa/zfs/zfsimpl.c) to always return 0 instead of EIO even if the feature is not supported, so probably the boot code would emit a lot of errors, but eventually somehow able to read in the kernel (which is supposed to have the support, as the upgrade happened when the kernel is running). There may be some useful scenarios, for example, if a boot code that can't support zstd compression is used, and boot file system never had zstd compression, then the boot code should, in theory, be able to have sufficient ability to boot from the pool. Personally I don't really like this idea, though, because it is not guaranteed to work and may expose bugs of the read-only code (because we now broke the calling contract). It would be much better to prevent the upgrade from happening on the boot pool, and only allow it when boot environment is updated, instead. If I was tasked to implement something for this, I'd probably do it by extending zpool utility to ask an OS dependent binary about "Is feature X supported by the current boot environment" for each read-only incompatible features, if the pool have 'bootfs' set, and refuse to upgrade (unless -f is specified) these boot pools if any feature is not supported. A naive implementation of the checker can be done by doing something like: 1. Gather a list of all partitions that is "freebsd-boot" type or "efi" type (this is actually not right; the EFI partition should be mounted and the actual EFI/BOOT/BOOT${ARCH}.EFI should be checked instead), and append /boot/loader 2. Check if the feature string is present in the listed files, and return false if any of them do not have the string.
(In reply to Xin LI from comment #2) > I think the ask is simply make check_feature() and check_mos_features() > (/stand/libsa/zfs/zfsimpl.c) to always return 0 instead of EIO even if > the feature is not supported, so probably the boot code would emit a lot of > errors, but eventually somehow able to read in the kernel (which is supposed > to have the support, as the upgrade happened when the kernel is running). Basically disagree, but agreed if only allowing "unsupported, enabled but not active" features. At worst, broken (actually is in checksum mismatch and/or un-decompressed) kernel and modules are loaded, passed control and causing undefined behaviour, which can be fatal. > There may be some useful scenarios, for example, if a boot code that can't > support zstd compression is used, and boot file system never had zstd > compression, then the boot code should, in theory, be able to have sufficient > ability to boot from the pool. IIUC, it would be able to check whether the feature is active or not. Just to be enabled is not an actual problem, but once the feature is actually used, the state becomes forcibly active (usually on first write access after enabled). If the code sizes of legacy zfsboot and gptzfsboot allows, it could be possible. Not sure how much space are left and how much codes are needed to implement. Maybe imp@ would be the best person to make decision here. > Personally I don't really like this idea, though, because it is not guaranteed > to work and may expose bugs of the read-only code (because we now broke > the calling contract). It would be much better to prevent the upgrade from > happening on the boot pool, and only allow it when boot environment is updated, > instead. Agreed. > If I was tasked to implement something for this, I'd probably do it by extending > zpool utility to ask an OS dependent binary about "Is feature X supported by > the current boot environment" for each read-only incompatible features, > if the pool have 'bootfs' set, and refuse to upgrade (unless -f is specified) > these boot pools if any feature is not supported. Looks reasonable to me. But what's needed to be checked is NOT only the boot environment, but also the boot codes themselves, too. (As boot codes in freebsd-boot partition and/or ESP are not parts of ZFS BEs.) Still remaining problem is that there could be any 3rd party boot loaders supporting boot FreeBSD from ZFS. > A naive implementation of the checker can be done by doing something like: > 1. Gather a list of all partitions that is "freebsd-boot" type or "efi" type > (this is actually not right; the EFI partition should be mounted and the actual > EFI/BOOT/BOOT${ARCH}.EFI should be checked instead), and append /boot/loader /boot/loader should be /boot/zfsloader, unless they are already the things with different names for compatibility. Anyway, /boot/zfsloader should be preferred whenever it exists. Without it, upgrading from (could be non-supported) old versions which both were actually different, checking for /boot/loader clearly causes false-negative. > 2. Check if the feature string is present in the listed files, and return false > if any of them do not have the string. Maybe the easiest (but far from perfect) way would be to implement codes for it in zpool as FreeBSD-specific. Implementing for all supported OS'es by OpenZFS would be much harder and would be harder to be accepted by upstream. Checks for freebsd-boot pattition should be simply searching the whole partition. Possibly, gpart would be required to zero'ing whole partition before writing boot code to it, not to be confused by remnants of old contents. For ESP, yes, it should be mounted to check. And at least, the presense and contents of EFI/freebsd/loader.efi EFI/freebsd/boot1.efi (at least if EFI/freebsd/loader.efi does not exist) EFI/boot/boot${ARCH}.efi (including the check whether it's FreeBSD's one or not) should be checked. To be paranoid, possibly, EFI/freebsd/BOOT${ARCH}.efi, too? Oops...too many things to consider. And still, maybe I'm overlooking something fatal. But it's clearly worth considering to avoid unbootable OS'es.
efibootmgr makes it harder to find the right loader.efi... You'd need to look at the kenv vars we set to see what was actually booted... Though, to be honest, maybe we just take a page from that and have the boot loader export all the ZFS features in the read only set. We can use *THAT* to know whether or not to update. Nothing exported, then you gotta force maybe? Or we hard code the list the day the feature is committed (though it wouldn't save you from older code, so maybe the list of the 13.2 loader.efi). Having the boot loader export what it can do... Though if you *do* update loader.efi, then this won't work, so maybe not. Unless we update the list from install, though we don't know for sure what's going to boot...
Can we ignore the unsupported flags but still verify the checksums?
>Personally I don't really like this idea, though, because it is not guaranteed to work Well of course it's not. But the worst case we're risking here is what is currently the only case: a boot failure. >It would be much better to prevent the upgrade from happening on the boot pool, and only allow it when boot environment is updated, instead. Fully agree, but that's a significant effort, while what I'm talking about might be a one line change.
(In reply to Edward Tomasz Napierala from comment #6) > Well of course it's not. But the worst case we're risking here is what is currently the only case: a boot failure. I think it's a bad idea. It will transform the boot failure from a well known one (an error message saying it can't find loader.lua) to some random thing that may work for a while, but then randomly stop working in the future. Or some hang that's hard to notice, or something else entirely. It would be a support nightmare, so I'm very close to a hard no on doing it unconditionally because I'll be the one that has the extra work. However, there are other options. First, we could have a built-in command that sets a global flag to force the operation and retry. This isn't terrible to implement, but is somewhat of a pain because we'll need special code in every single driver to do this. And it can't work in boot1.efi, but I don't care about that because boot1.efi is deprecated. Second, we could prompt the user when we detect the problem whether or not to continue anyway. I think we always have a console, though it might not be the user's preferred console at this point (since that preference is set from the very filesystem we're trying to read). We do have conin for EFI, even boot1.efi. Third, for BIOS booting (and to a lessor extent EFI) we have a command line option we can bass in from boot0 that could force it. EFI could have an environment variable that controls it (for those systems that don't let you set a command line option). Fourth, and this is another of the modify all the boot loaders, would be to try what we do now, then test to see if we can read loader.lua (the 4th loader won't be modified: it's feature set is frozen and this is a new feature). If we can't read loader.lua, we know we're about to fail, so we can try again with a global force flag set (after a brief pause to announce we're doing this and all bets are off and we might reset or hit an assertion in the ZFS code). This I think I like best because it's the safest one that could be automatic. Plus we can set a kenv that communicates to an rc file to print a big, ugly warning on boot to say we had to do this to read the ZFS pool and you got lucky: next update or even next boot you might not be so lucky. Of course, 'don't update the zpool unless the boot blocks support it` is the best option.
Okay, I'm convinced. Indeed, having a box survive the reboot only to randomly fail to boot sometime afterwards would be pretty terrible.