after zpool upgrade, the need to update bootcode is not mentioned

[dlangille]

System information

Type	Version/Name
FreeBSD	13.0
amd64

After zpool upgrade, there is no notice to update the bootcode:

[dvl@r720-02:~] $ zpool status zroot
  pool: zroot
 state: ONLINE
status: Some supported features are not enabled on the pool. The pool can
	still be used, but some features are unavailable.
action: Enable all features using 'zpool upgrade'. Once this is done,
	the pool may no longer be accessible by software that does not support
	the features. See zpool-features(5) for details.
  scan: scrub repaired 0B in 00:01:35 with 0 errors on Thu May 20 03:32:17 2021
checkpoint: created Fri May 21 17:26:03 2021, consumes 648K
config:

	NAME        STATE     READ WRITE CKSUM
	zroot       ONLINE       0     0     0
	  mirror-0  ONLINE       0     0     0
	    ada0p3  ONLINE       0     0     0
	    ada1p3  ONLINE       0     0     0

errors: No known data errors
[dvl@r720-02:~] $ sudo zpool upgrade zroot
This system supports ZFS pool feature flags.

Enabled the following features on 'zroot':
  userobj_accounting
  encryption
  project_quota
  resilver_defer
  bookmark_v2
  redaction_bookmarks
  redacted_datasets
  bookmark_written
  log_spacemap
  livelist
  device_rebuild
  zstd_compress
  draid

[chungy]

What's required depends heavily on factors such as whether the pool is even required for booting (I know, your zroot example is one such), whether the partitions are in MBR or GPT format, and whether you use BIOS or UEFI to boot.

I feel like the bootloader is out of scope and it's a detail of system administration external to ZFS, especially when OpenZFS also covers Linux and its even more varied mechanisms for booting.

I'm assuming this particular instance follows an upgrade from FreeBSD 12.x to 13.0. FreeBSD probably should document their upgrade procedures to include updating the boot loader.

[allanjude]

Specifically, FreeBSD used to print a message here, and that went away as we migrated to OpenZFS. I think the idea would be to restore it, as before conditional on the pool property bootfs being set. Instead of providing specific instructions, I envision it just have a URL to the FreeBSD handbook chapter on bootcode or similar.

[grembo]

Specifically, FreeBSD used to print a message here, and that went away as we migrated to OpenZFS. I think the idea would be to restore it, as before conditional on the pool property bootfs being set. Instead of providing specific instructions, I envision it just have a URL to the FreeBSD handbook chapter on bootcode or similar.

@allanjude I thought about this too, but AFAIK bootfs doesn't necessarily need to be set in order to boot from a zpool.

Anyway, I opened a pull request in #12104 that mentions this in a generic enough way, so it applies to all operating systems, as I've been told by someone else who faced the same problem on Debian. I'm happy to change this pull request based on feedback of course. If the consensus is that Linux users don't want or don't need this, then it might be better to create a patch downstream.

[grahamperrin]

#12099 (comment)

… FreeBSD probably should document their upgrade procedures to include updating the boot loader.

FreeBSD bug 255318 – handbook: Document how to update the bootloader

[grembo]

@allanjude After some discussion, I updated the patch #12104 to only cover FreeBSD for now. The question is, if showing the reminder should be made conditional based on bootfs being set (like you suggested and I evaluated in the beginning, when I thought this will be cross platform). It's clearly not necessary to be set to boot from a pool (I built systems without it set in the past), but all out of the box FreeBSD setups we use these days have it set - so maybe a good trade off (and not being annoyed by the warning in case a non-bootable pool is updated). What do you think?

Code:

	if (modified_pool) {
#ifdef __FreeBSD__
		char bootfs[ZPOOL_MAXPROPLEN];
		if (zpool_get_prop(zhp, ZPOOL_PROP_BOOTFS, bootfs,
		    sizeof (bootfs), NULL, B_FALSE) == 0 &&
		    strcmp(bootfs, "-") != 0) {
			(void) printf(gettext("\nPool '%s' has the bootfs "
			    "property set, you might need to update\nthe boot "
			    "code. See gptzfsboot(8) and loader.efi(8) for "
			    "details.\n"), zpool_get_name(zhp));
		}
#else
		(void) printf("\n");
#endif
	}

Result (with bootfs set):

$ zpool upgrade testpool4
This system supports ZFS pool feature flags.

Enabled the following features on 'testpool4':
  userobj_accounting
  encryption
  project_quota
  allocation_classes
  resilver_defer
  bookmark_v2
  redaction_bookmarks
  redacted_datasets
  bookmark_written
  log_spacemap
  livelist
  device_rebuild
  zstd_compress
  draid

Pool 'testpool4' has the bootfs property set, you might need to update
the boot code. See gptzfsboot(8) and loader.efi(8) for details.

Result (without bootfs set):

This system supports ZFS pool feature flags.

Enabled the following features on 'testpool5':
  userobj_accounting
  encryption
  project_quota
  allocation_classes
  resilver_defer
  bookmark_v2
  redaction_bookmarks
  redacted_datasets
  bookmark_written
  log_spacemap
  livelist
  device_rebuild
  zstd_compress
  draid