Bug 256024 - [zfs] `zpool upgrade' doesn't warn about updating bootloader anymore
Summary: [zfs] `zpool upgrade' doesn't warn about updating bootloader anymore
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.0-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-fs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-20 07:52 UTC by Michael Gmelin
Modified: 2021-06-03 14:01 UTC (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Gmelin freebsd_committer 2021-05-20 07:52:56 UTC
It seems like that while switching our upstream to OpenZFS, the following warning was lost after upgrading a zpool (e.g., `zpool upgrade zroot`):

  If you boot from pool 'zroot', don't forget to update boot code.
  Assuming you use GPT partitioning and da0 is your boot disk
  the following command will do it:

          gpart bootcode -b /boot/pmbr -p /boot/gptzfsboot -i 1 da0


I always liked that warning, as it reminded me not to forget this important step. At the same time I was also not so happy about the hardcoded example it gives (it only covers one specific setup).

Therefore I would propose to get this feature back, but using a more generic text:

  In case you're booting from <poolname>, please make sure to update
  the bootcode according to your partition layout.
  See zfsboot(8) (`man zfsboot') for details.


I didn't create a patch/code review yet, as I'm not sure how processes work when changing ZFS related code. I assume we try to keep the delta with OpenZFS as small as possible.

This would also address bug #229595.
Comment 1 Michael Gmelin freebsd_committer 2021-05-20 07:54:05 UTC
Adding Allan, as he might know more about the processes when updating ZFS related code.
Comment 2 Robert Clausecker 2021-05-20 12:12:07 UTC
Got bitten by this badly last weekend when I upgraded the root pool of my 12.2 -> 13.0 install.  Didn't wanna boot anymore.  Had to manually repartition to enlarge the 700 kB ESP to a size that suites the new boot loader.  Not fun at all.
Comment 3 Graham Perrin 2021-05-21 07:31:25 UTC
Parallel discussion: 

<https://lists.freebsd.org/archives/freebsd-current/2021-May/169385.html>

(In reply to Robert Clausecker from comment #2)

> 12.2 -> 13.0

Release notes were overlooked. 

<https://www.freebsd.org/releases/13.0R/relnotes/#upgrade> 
there's a red alert box referring to 
<https://www.freebsd.org/releases/13.0R/relnotes/#boot>
Comment 4 Michael Gmelin freebsd_committer 2021-05-21 07:46:04 UTC
(In reply to Graham Perrin from comment #3)


> Release notes were overlooked. 
> 
> <https://www.freebsd.org/releases/13.0R/relnotes/#upgrade> 
> there's a red alert box referring to 
> <https://www.freebsd.org/releases/13.0R/relnotes/#boot>

Just for clarity's sake, both of these locations don't contain anything specific about ZFS and/or zpool upgrade. They only and specifically talk about EFI System Partitions and efibootmgr(8). Therefore they don't relate to this PR beyond the fact that they both are about booting.
Comment 5 Graham Perrin 2021-05-21 07:52:05 UTC
Cross reference: documentation bug 255318 for the FreeBSD Handbook. 

----

(In reply to Michael Gmelin from comment #0)

If I'm not mistaken, neither 
<https://www.freebsd.org/cgi/man.cgi?query=zfsboot&sektion=8&manpath=FreeBSD+13.0-current> nor 
<https://www.freebsd.org/cgi/man.cgi?query=gptzfsboot&sektion=8&manpath=FreeBSD+13.0-current> covers EFI use cases. 

We have loader(8) 
<https://www.freebsd.org/cgi/man.cgi?query=loader&sektion=8&manpath=FreeBSD+13.0-current> 
and gpart (8) 
<https://www.freebsd.org/cgi/man.cgi?query=gpart&sektion=8&manpath=FreeBSD+13.0-current> 
however, these pages are long enough that some readers might not grasp the context and essence of what's required.
Comment 6 Michael Gmelin freebsd_committer 2021-05-21 08:01:54 UTC
(In reply to Graham Perrin from comment #5)

You wrote

> Release notes were overlooked. 
> 
> <https://www.freebsd.org/releases/13.0R/relnotes/#upgrade> 
> there's a red alert box referring to 
> <https://www.freebsd.org/releases/13.0R/relnotes/#boot>

And neither of these two locations talks about anything else than EFI.

This PR is not about claiming that there is no documentation about how to install the boot loader.

It's about the warning after "zpool upgrade" that disappeared and if

  1. We could add an errata note warning about that (so people are aware).
  2. If we could bring back a similar warning back.

Someone reading the gpart man page is probably already aware that there's something to do (and depending on how experienced they are, might read that man page on a different device while trying to figure out how to make that machine they just updated boot again).
Comment 7 Robert Clausecker 2021-05-21 08:10:03 UTC
(In reply to Graham Perrin from comment #3)

Curiously enough I did read the release notes, just forgot about them when upgrading the server.  And it only failed once I had upgraded the root pool a few weeks later and then rebooted the server.  A warning on zpool upgrade would have helped me a great deal.
Comment 8 Graham Perrin 2021-05-21 08:27:20 UTC
(In reply to Michael Gmelin from comment #6)

For what it's worth: I'm one of a number of people who read the release notes, but misinterpreted some part(s). I sort of read between the lines and drew false conclusions. 

----

A big +1 to having a concise alert at pool upgrade time.

Ideally (if possible), have the alert _prior_ to completion of the command, although … 

zpool-upgrade(8) <https://man.freebsd.org/zpool-upgrade%288%29>

… I'm not aware of anything like dry-run in this context and (sorry) I don't have handy a pool with which I can experiment.
Comment 9 Kurt Jaeger freebsd_committer 2021-05-21 09:06:36 UTC
(In reply to Graham Perrin from comment #8)
I'm also one of those who really miss the warning and would appreciate if we
had some guide for the not-so-technical user-base.

Let me assure you, debian/Linux has the same problem as I currently
have one server that runs, but will probably fail to reboot, because,
they, too, had no safety / warnings on zpool upgrade 8-}
Comment 10 martin 2021-05-21 09:46:30 UTC
Why not just update the boot loader at the same time as you update the OS?  It might contain useful bug fixes.
Comment 11 Ronald Klop 2021-05-21 10:18:40 UTC
My upgrade script does something like this: (pseudocode for brevity)

PART=/dev/ada0p1
BOOTFILE=/boot/gptzfsboot
FILESIZE=$( stat -f "%z" $BOOTFILE )

CURBOOT_MD5=`head -c $FILESIZE $PART | md5 -q /dev/stdin`
NEWBOOT_MD5=`md5 -q $BOOTFILE`
if CURBOOT_MD5 != NEWBOOT_MD5; then
    echo "Upgrade your bootblocks."
    echo "gpart bootcode -b /boot/pmbr -p /boot/gptzfsboot -i 1 $PART"
fi

This can probably made more general. Just my 2 cents.
Comment 12 Robert Clausecker 2021-05-21 10:49:52 UTC
(In reply to martin from comment #10)

I don't understand either why freebsd-update(8) doesn't have provisions for that.  It doesn't even print an excerpt of the release notes warning people to do that themselves.
Comment 13 Michael Gmelin freebsd_committer 2021-05-21 11:02:36 UTC
(In reply to Robert Clausecker from comment #12)

Because freebsd-update doesn't upgrade your zpool. So reinstalling the boot loader isn't necessary after running freebsd-update.

Typical scheme for me:
1. Update FreeBSD.
2. Keep pools as they are, to make sure the release works well and existing rescue images, other machines etc., can mount them easily.
3. A few weeks or even months later, run "zpool upgrade".

That last step is the point where the warning makes sense (and that's why it was there).
Comment 14 stephan 2021-05-21 12:10:00 UTC
But it doesn't hurt to (optionally) upgrade the bootloader directly after the update, does it? 

Even if somebody upgrades the zpool only later on (or never) a new bootloader ensures that *if* the upgrade happens at some point, the system still boots at all times.
Comment 15 Michael Gmelin freebsd_committer 2021-05-21 12:21:17 UTC
(In reply to stephan from comment #14)

Assuming the boot loader stays compatible with previous zpool versions it wouldn't hurt - at least if freebsd-update can actually reliably determine the bootloader setup used (e.g., label the boot partitions of *all* driver containing the boot pool, not just the first one and then not be able to boot in case one of the drives dies, and also not drives not used for booting, including removable media).

Being able to do it in cases where it's clear cut and warn in other cases that it might need to be done manually, might be a start - and of course realize all the cases in which freebsd-update is run on something that doesn't involve the bootloader, e.g., updating jails.

There's a lot of potential for breaking things in the process though.
Comment 16 Michael Gmelin freebsd_committer 2021-05-22 13:12:33 UTC
Note that this has been reported upstream now.

I just created a pull request, let's see where this goes: https://github.com/openzfs/zfs/pull/12104
Comment 17 Michael Gmelin freebsd_committer 2021-06-03 14:01:41 UTC
The pull request has been merged upstream.

https://github.com/openzfs/zfs/commit/65d9212aeeb531e9f987bb41a1ee11b526d2cdad

It would be nice if the change could somehow be brought into releng/13.